[ 
https://issues.apache.org/jira/browse/HBASE-4071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13089087#comment-13089087
 ] 

jirapos...@reviews.apache.org commented on HBASE-4071:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1582/#review1591
-----------------------------------------------------------


Nice work!  Some formatting things (didn't comment on some lines > 80 chars) 
and need to update some of the javadocs for the new arguments.  Will look 
through the patch one more time before I ask questions.


http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<https://reviews.apache.org/r/1582/#comment3577>

    line > 80 chars



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<https://reviews.apache.org/r/1582/#comment3578>

    javadoc



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3582>

    seems like this change actually changes what the functionality / lines are 
between ColumnTracker vs. QueryMatcher.  should you update the javadoc here to 
describe what this is not responsible for?



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3579>

    update the javadoc here... maybe explain why we pass ttl?  seems odd that 
this removed timestamp from the API



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3581>

    update javadoc



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3580>

    long line



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
<https://reviews.apache.org/r/1582/#comment3583>

    spaces between arguments:  (minVersions, maxVersions, ttl)



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3584>

    update javadoc



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java
<https://reviews.apache.org/r/1582/#comment3585>

    i'm a little confused about this.  this doesn't need to look at the number 
of included versions?  you are only ever done if minVersions is explicitly set 
to 0 (disabled)?



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<https://reviews.apache.org/r/1582/#comment3587>

    this is a nice comment, but is this true?
    
    can't i safely delete anything that is expired once i have hit my min for 
this file?  we may not evict _enough_ but we can at least evict something?
    
    unfortunately we don't use the query matcher here, but should be easy to 
add it here (someone else has added it here as part of some other changes, but 
they aren't committed yet and won't be for some time)



http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<https://reviews.apache.org/r/1582/#comment3588>

    nice explanation


- Jonathan


On 2011-08-21 00:32:00, Lars Hofhansl wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1582/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-21 00:32:00)
bq.  
bq.  
bq.  Review request for hbase, Todd Lipcon, Michael Stack, and Ian Varley.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Allow enforcing a minimum number of versions when TTL is enable for a 
store.
bq.  The GC logic for both versions and TTL is unified inside the 
ColumnTrackers.
bq.  
bq.  
bq.  This addresses bug HBASE-4071.
bq.      https://issues.apache.org/jira/browse/HBASE-4071
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
 1159836 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/ruby/hbase/admin.rb 
1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
 1159836 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java
 1159836 
bq.  
bq.  Diff: https://reviews.apache.org/r/1582/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Ran all tests. I get error (not failures) from two: 
TestDistributedLogSplitting and TestHTablePool. Both fail with or without my 
changes.
bq.  New tests: TestMinVersions.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Lars
bq.  
bq.



> Data GC: Remove all versions > TTL EXCEPT the last written version
> ------------------------------------------------------------------
>
>                 Key: HBASE-4071
>                 URL: https://issues.apache.org/jira/browse/HBASE-4071
>             Project: HBase
>          Issue Type: New Feature
>            Reporter: stack
>         Attachments: MinVersions.diff
>
>
> We were chatting today about our backup cluster.  What we want is to be able 
> to restore the dataset from any point of time but only within a limited 
> timeframe -- say one week.  Thereafter, if the versions are older than one 
> week, rather than as we do with TTL where we let go of all versions older 
> than TTL, instead, let go of all versions EXCEPT the last one written.  So, 
> its like versions==1 when TTL > one week.  We want to allow that if an error 
> is caught within a week of its happening -- user mistakenly removes a 
> critical table -- then we'll be able to restore up the the moment just before 
> catastrophe hit otherwise, we keep one version only.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to