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

nkeywal commented on HBASE-3855:
--------------------------------

I saw some points related to numIterReseek, used in the MemStoreScanner#getNext 
 (line 690):

{noformat}679       protected KeyValue getNext(Iterator it) {
680           KeyValue ret = null;
681           long readPoint = ReadWriteConsistencyControl.getThreadReadPoint();
682           //DebugPrint.println( " MS@" + hashCode() + ": threadpoint = " + 
readPoint);
683      
684           while (ret == null && it.hasNext()) {
685             KeyValue v = it.next();
686             if (v.getMemstoreTS() <= readPoint) {
687               // keep it.
688               ret = v;
689             }
690             numIterReseek--;
691             if (numIterReseek == 0) {
692               break;
693              }
694           }
695           return ret;
696         }{noformat} 

This function is called by seek, reseek, and next. The numIterReseek  is only 
usefull for reseek.


There are some issues, I am not totally sure it's the root cause of the test 
case error, but it could explain partly the randomness of the error, and one 
point is for sure a bug.

1) In getNext, numIterReseek is decreased, then compared to zero. The seek 
function sets numIterReseek to zero before calling getNext. It means that the 
value will be actually negative, hence the test will always fail, and the loop 
will continue. It is the expected behaviour, but it's quite smart.

2) In "reseek", numIterReseek is not set between the loops on the two 
iterators. If the numIterReseek is equals to zero after the loop on the first 
one, the loop on the second one will never call seek, as numIterReseek will be 
negative. 

3) Still in "reseek", the test to call "seek" is (kvsetNextRow == null && 
numIterReseek == 0). In other words, if kvsetNextRow is not null when 
numIterReseek equals zero, numIterReseek will start to be negative at the next 
iteration and seek will never be called.

4) You can have side effects if reseek ends with a numIterReseek > 0: the 
following calls to the "next" function will decrease numIterReseek to zero, and 
getNext will break instead of continuing the loop. As a result, later calls to 
next() may return null or not depending on how is configured the default value 
for numIterReseek.


To check if the issue comes from point 4, you can set the numIterReseek to zero 
before returning in reseek:
{noformat}      numIterReseek = 0;
      return (kvsetNextRow != null || snapshotNextRow != null);
    }{noformat}


On my env, on trunk, it seems to work, but as it's random I am not really sure. 
I also had to modify the test (I added a loop) to make it fails more often, the 
original test was working quite well here. 


> Performance degradation of memstore because reseek is linear
> ------------------------------------------------------------
>
>                 Key: HBASE-3855
>                 URL: https://issues.apache.org/jira/browse/HBASE-3855
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: dhruba borthakur
>            Priority: Blocker
>             Fix For: 0.92.0
>
>         Attachments: memstoreReseek.txt, memstoreReseek2.txt
>
>
> The scanner use reseek to find the next row (or next column) as part of a 
> scan. The reseek code iterates over a Set to position itself at the right 
> place. If there are many thousands of kvs that need to be skipped over, then 
> the time-cost is very high. In this case, a seek would be far lesser in cost 
> than a reseek.

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

        

Reply via email to