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

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



bq.  On 2011-08-25 04:29:19, Ted Yu wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java,
 line 490
bq.  > <https://reviews.apache.org/r/1650/diff/2/?file=35429#file35429line490>
bq.  >
bq.  >     Line wrap is needed for this long line.

Will do.


bq.  On 2011-08-25 04:29:19, Ted Yu wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java,
 line 491
bq.  > <https://reviews.apache.org/r/1650/diff/2/?file=35429#file35429line491>
bq.  >
bq.  >     This should be false.

This definitely needs to be true. Deletes need to be retained when the cache is 
flushed.
True, means to retain deletes.

(I had this the wrong way in my first attempt as well and then wondered why all 
the deleted rows were not removed)


bq.  On 2011-08-25 04:29:19, Ted Yu wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CollectionBackedScanner.java,
 line 64
bq.  > <https://reviews.apache.org/r/1650/diff/2/?file=35430#file35430line64>
bq.  >
bq.  >     Minor comment:
bq.  >     This ctor is used by KeyValueScanFixture below. Putting comparator 
as first parameter would make it more readable.

Agreed, I'll change the order.


- Lars


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


On 2011-08-25 01:28:34, Lars Hofhansl wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1650/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-25 01:28:34)
bq.  
bq.  
bq.  Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This avoids flushing row versions to disk that are known to be GC'd by the 
next compaction anyway.
bq.  This covers two scenarios:
bq.  1. maxVersions=N and we find at least N versions in the memstore. We can 
safely avoid flushing any further versions to disk.
bq.  2. similarly minVersions=N and we find at least N versions in the 
memstore. Now we can safely avoid flushing any further *expired* versions to 
disk.
bq.  
bq.  This changes the Store flush to use the same mechanism that used for 
compactions.
bq.  I borrowed some code from the tests and refactored the test code to use a 
new utility class that wraps a sorted collection and then behaves like 
KeyValueScanner. The same class is used to create scanner over the memstore's 
snapshot.
bq.  
bq.  
bq.  This addresses bug HBASE-4241.
bq.      https://issues.apache.org/jira/browse/HBASE-4241
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
 1161347 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CollectionBackedScanner.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/KeyValueScanFixture.java
 1161347 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java
 1161347 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java
 1161347 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java
 1161347 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
 1161347 
bq.  
bq.  Diff: https://reviews.apache.org/r/1650/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Ran all tests. TestHTablePool and TestDistributedLogSplitting error out 
(with or without my change).
bq.  I had to change three tests that incorrectly relied on old rows hanging 
around after a flush (or were otherwise incorrect).
bq.  
bq.  No new test, as this should cause no functional change.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Lars
bq.  
bq.



> Optimize flushing of the Store cache for max versions and (new) min versions
> ----------------------------------------------------------------------------
>
>                 Key: HBASE-4241
>                 URL: https://issues.apache.org/jira/browse/HBASE-4241
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>    Affects Versions: 0.92.0
>            Reporter: Lars Hofhansl
>            Assignee: Lars Hofhansl
>         Attachments: 4241-v2.txt, 4241.txt
>
>
> As discussed with with Jon, there is room for improvement in how the memstore 
> is flushed to disk.
> Currently only expired KVs are pruned before flushing, but we can also prune 
> versions if we find at least maxVersions versions in the memstore.
> The same holds for the new minversion feature: If we find at least minVersion 
> versions in the store we can remove all further versions that are expired.
> Generally we should use the same mechanism here that is used for Compaction. 
> I.e. StoreScanner. We only need to add a scanner to Memstore that can scan 
> along the current snapshot.

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

        

Reply via email to