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

nkeywal commented on HBASE-4195:
--------------------------------

With the current implementation, it can fails in TestHRegion at assert 
"assertEquals("i=" + i, expectedCount, result.size());" when there is a mess up 
on the lists. It should not occur with the new implementation.

The issue in the flush is shown by the assert just next to this one. It's (log 
added in the new patch)
{noformat}
              LOG.warn("These two KV should have the same value." +
                " Previous KV:" +
                previousKV + "(memStoreTS:" + previousKV.getMemstoreTS() + ")" +
                ", New KV: " +
                kv + "(memStoreTS:" + kv.getMemstoreTS() + ")"
              );
              assertEquals(previousKV.getValue(), thisValue);
{noformat}

With these modified values on testWritesWhileGetting:
{noformat}
    int testCount = 1000; // more iterations. Increase it more if necessary.
    // [...]
    int flushInterval = 2; // more flush
    int compactInterval = 1000 * flushInterval; // no compact (should have no 
impact, but...)
{noformat}

I produced this error on the trunk + patch proposed:

{noformat}
[...]
put iteration = 1927
put iteration = 2021
These two KV should have the same value. Previous 
KV:row0/family4:qual99/1942/Put/vlen=4(memStoreTS:993), New KV: 
row0/family5:qual0/1944/Put/vlen=4(memStoreTS:0)
E
Time: 19.051
There was 1 failure:
1) testWritesWhileGetting(org.apache.hadoop.hbase.regionserver.TestHRegion)
junit.framework.AssertionFailedError: expected:<1942> but was:<1944>
        at 
org.apache.hadoop.hbase.HBaseTestCase.assertEquals(HBaseTestCase.java:704)
        at 
org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:2781)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
[...]
{noformat}


May be there are more memory allocations on 0.90 and as a consequence more 
random interruptions caused by the GC? Note that I am doing the tests outside 
of Maven, but with JUnit, without specific java parameters. 

If worth checking that on .90 it is as well an error when the family changes 
(qualifier equals qual0 or qual1). May be it's something different on .90.

Unfortunatly  (or not :-)) I am on vacations for a week, so I won't be able to 
give a hand the next days, but I will be back middle of next week.





> Possible inconsistency in a memstore read after a reseek, possible 
> performance improvement
> ------------------------------------------------------------------------------------------
>
>                 Key: HBASE-4195
>                 URL: https://issues.apache.org/jira/browse/HBASE-4195
>             Project: HBase
>          Issue Type: Bug
>          Components: regionserver
>    Affects Versions: 0.90.4
>         Environment: all
>            Reporter: nkeywal
>            Assignee: nkeywal
>            Priority: Critical
>             Fix For: 0.90.5
>
>         Attachments: 20110824_4195_MemStore.patch, 
> 20110824_4195_TestHRegion.patch
>
>
> This follows the dicussion around HBASE-3855, and the random errors (20% 
> failure on trunk) on the unit test 
> org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting
> 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.
> It has to be confirmed that this totally fix (it could be partial or 
> unrelated) 
> org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting 
> before implementing a complete solution.

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

        

Reply via email to