Review Board 1.7.22


HBASE-3584 We need to atomically put/delete/increment in one call

Review Request #3481 - Created Jan. 12, 2012 and submitted

Lars Hofhansl
trunk
HBASE-3584
Reviewers
hbase
hbase
Add API for atomic row mutations to HBase (currently Put and Delete).

Client API would look like this:
    Delete d = new Delete(ROW);
    Put p = new Put(ROW);
    ...
    AtomicRowMutation arm = new AtomicRowMutation(ROW);
    arm.add(p);
    arm.add(d);
    myHtable.mutateAtomically(arm);
* Simple functional test: TestFromClientSide.testAtomicRowMutation
* Multithreaded stress test: TestAtomicOperation.testAtomicMutationMultiThreads
* coprocessor test: TestRegionObserverInterface.testAtomicRowMutation
* manual testing
Review request changed
Updated (Jan. 13, 2012, 5:11 p.m.)
This version passes all tests.
Fixed some Java doc comments.

This is a good candidate for commit.
Posted (Jan. 13, 2012, 8:01 p.m.)
+1 with some comments below.
You think this class needs to be AtomicRowMutation?  (makes me want to 'duck and cover').   All mutations on a row in hbase are 'atomic' so the prefix strikes me as redundant
Yeah, its kinda crazy all this duplication of say the row byte array.. it'll be here and then up in Put and Delete.  Its like we need a shorthand of some kind.... but for another time.
Should be 0?  (No biggie)
Why have an add for Put and one for Delete and not just an add Mutation?
Important!
Usually we add space around operators.  Next time.
Good
Call it mutate?
mutate?
Duplicated code?  We should fix this up some day
Posted (Jan. 13, 2012, 8:17 p.m.)

   

  
Yep... The same that way that all KVs in Put/Delete's familyMap replicate the same rowkey... Should tackle that generally in a separate jira.
Versions are 0-based? :)
Append is also a Mutation and is not supported (neither is Increment, but that's not a Mutation).
If/when we support all Mutation we can have a general add(Mutation) mutation (nice thing that this won't change the visible client api)
Yep!
Will change
Hmm... Torn about this.
The main point of this is that it is atomic. We already have multiAction (which can take Mutations), but it is not atomic.
  1. Ok
    
    MultiAction is x-row, right?   RowMutation has the row prefix.
    
    But if you think it makes it clearer, go for it.
Posted (Jan. 18, 2012, 9:18 p.m.)

   

  
@Lars: Sorry about the late question. I would like to understand the following case.

What if we crash in between this loop?

Is it possible that we have persisted half the WALEdits, but not gotten to the rest?

I am concerned, that if that were the case (hopefully not) then on RS restart, we will replay, a partial list of mutations from "arm".



  1. Yes, which is why I filed, implemented, and committed HBASE-5203, which fixes the problem for real :)