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

HBase Review Board commented on HBASE-3082:
-------------------------------------------

Message from: "Jonathan Gray" <jg...@apache.org>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1053/#review1585
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/regionserver/ControlledScan.java
<http://review.cloudera.org/r/1053/#comment5382>

    Need to add apache licenses



src/main/java/org/apache/hadoop/hbase/regionserver/ControlledScan.java
<http://review.cloudera.org/r/1053/#comment5383>

    Maybe a little more description here.  Like, why do we need this.
    
    Also, is ControlledScan the best name for this class?  Not clear to me that 
this is "controlled" and thus the other scans are "uncontrolled"?
    
    If this was to be used for more later and even for this use, it's really 
additional configuration/options for Scans.  So is an InternalScan or 
ModifiedScan?



src/main/java/org/apache/hadoop/hbase/regionserver/ControlledScan.java
<http://review.cloudera.org/r/1053/#comment5385>

    Rather than these two public booleans, we should have getters/setters.
    
    And even rather than just straight set/get on each boolean, these two 
booleans are usually one or the other, right?  So I think it would be better to 
hide implementation detail and instead expose methods like setMemStoreOnly() 
and setFilesOnly() which would deal with the two booleans appropriately.  Then 
you'd have get methods like checkMemStore() and checkFiles() or something like 
that.



src/main/java/org/apache/hadoop/hbase/regionserver/ControlledScan.java
<http://review.cloudera.org/r/1053/#comment5384>

    Whitespace



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.cloudera.org/r/1053/#comment5386>

    Need to cleanup whitespace here.
    
    Maybe a little bit of javadoc for get_last_increment which describes what 
this does (and why it's still "correct" for increments)


- Jonathan





> For ICV gets, first look in MemStore before reading StoreFiles
> --------------------------------------------------------------
>
>                 Key: HBASE-3082
>                 URL: https://issues.apache.org/jira/browse/HBASE-3082
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>            Reporter: Jonathan Gray
>            Assignee: Prakash Khemani
>
> For incrementColumnValue operations, it is possible to check MemStore for the 
> column being incremented without sacrificing correctness.  If the column is 
> not found in MemStore, we would then have to do a normal Get that 
> opens/checks all StoreFiles for the given Store.
> In practice, this makes increment operations significantly faster for 
> recently/frequently incremented columns.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to