You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Gary Helmling <gh...@gmail.com> on 2010/11/16 08:09:52 UTC

Review Request: HBASE-3235: Intermittent incrementColumnValue failure in TestHRegion

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

Review request for hbase and Ryan Rawson.


Summary
-------

Fix for MemStore.upsert(KeyValue) to start the kvset.tailSet() of potential KVs to remove at the beginning of entries for the row/family/qualifier combination, ignoring timestamp to prevent Puts being skipped based on timestamp alone and masking the ICV.


This addresses bug HBASE-3235.
    http://issues.apache.org/jira/browse/HBASE-3235


Diffs
-----

  src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java b7409b0 
  src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 7640997 

Diff: http://review.cloudera.org/r/1224/diff


Testing
-------

Added a new test: TestHRegion.testIncrementColumnValue_UpdatingInPlace_TimestampClobber() to recreate the existing failure condition: 1) put to a row/family/qualifier, 2) ICV to the same row/family/qualifier with the same timestamp.  This test fails consistently without the patch to MemStore.

With the patch to MemStore, the new test case consistently passes.  I also ran TestHRegion 15+ times and saw no more intermittent failures of testIncrementColumnValue_UpdatingInPlace().  Previously this was failing every 5 or so test runs, so this seems a pretty good indication it's fixed.

I also ran through the full test suite on 0.90 and all passed except for an error in TestHLog...


Thanks,

Gary


Re: Review Request: HBASE-3235: Intermittent incrementColumnValue failure in TestHRegion

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1224/#review1937
-----------------------------------------------------------

Ship it!


I believe the TestHLog failure noted was on our Hudson. If so it is unrelated to this change. 

- Andrew


On 2010-11-15 23:09:52, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1224/
> -----------------------------------------------------------
> 
> (Updated 2010-11-15 23:09:52)
> 
> 
> Review request for hbase and Ryan Rawson.
> 
> 
> Summary
> -------
> 
> Fix for MemStore.upsert(KeyValue) to start the kvset.tailSet() of potential KVs to remove at the beginning of entries for the row/family/qualifier combination, ignoring timestamp to prevent Puts being skipped based on timestamp alone and masking the ICV.
> 
> 
> This addresses bug HBASE-3235.
>     http://issues.apache.org/jira/browse/HBASE-3235
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java b7409b0 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 7640997 
> 
> Diff: http://review.cloudera.org/r/1224/diff
> 
> 
> Testing
> -------
> 
> Added a new test: TestHRegion.testIncrementColumnValue_UpdatingInPlace_TimestampClobber() to recreate the existing failure condition: 1) put to a row/family/qualifier, 2) ICV to the same row/family/qualifier with the same timestamp.  This test fails consistently without the patch to MemStore.
> 
> With the patch to MemStore, the new test case consistently passes.  I also ran TestHRegion 15+ times and saw no more intermittent failures of testIncrementColumnValue_UpdatingInPlace().  Previously this was failing every 5 or so test runs, so this seems a pretty good indication it's fixed.
> 
> I also ran through the full test suite on 0.90 and all passed except for an error in TestHLog...
> 
> 
> Thanks,
> 
> Gary
> 
>