hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ryan Rawson" <ryano...@gmail.com>
Subject Re: Review Request: Increment multiple columns in a row at once
Date Mon, 25 Oct 2010 03:05:54 GMT

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



trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
<http://review.cloudera.org/r/1088/#comment5507>

    adding trailing spaces
    



trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
<http://review.cloudera.org/r/1088/#comment5508>

    adding more trailing spaces, your ide should have a feature to strip these



trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
<http://review.cloudera.org/r/1088/#comment5511>

    I'm not sure how this code is optional for your new 'upsert', here are some use cases
that I found painful:
    
    - when the KV is in snapshot we can create new KVs in memstore with the same TS.  This
means you have a dup and before we had this new 'dup' code it ruined counts badly.
    - the Math.max() bit it to ensure the new KV isnt being created in the past a bit and
accidently duplicating  a timestamp inside the snapshot.



trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
<http://review.cloudera.org/r/1088/#comment5509>

    im not sure i like the name upsert, it is too rdbms-y for me.
    
    I need to poke more at this, but i prever the matchingRow() call, it encapsulates the
getRowOffset junk, which leaks wayyy too much all over the place.
    
    



trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
<http://review.cloudera.org/r/1088/#comment5510>

    yeah you cant compare against memstoreTS because if you have this in here you wont be
able to ever increment values that were inserted into the future. you'd just leave them there
and continually see it in the 'get' part and then in this code bit leave it in place and create
a new KV that is masked by the future KV.
    
    It won't be possible for that insert to be part of an uncommitted change because of the
rowlock however. So no atomic-rules will have been broken.


- Ryan


On 2010-10-24 19:29:07, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1088/
> -----------------------------------------------------------
> 
> (Updated 2010-10-24 19:29:07)
> 
> 
> Review request for hbase, stack and khemani.
> 
> 
> Summary
> -------
> 
> Adds a new Increment class that allows multiple columns (each w/ own increment amount)
in a single row being incremented in one call.
> 
> The big wins here are being able to do multiple columns in a row in a single RPC and
having it be appended/synced to the WAL in a single call.
> 
> The current trade-off is that you lose atomicity to readers (ie. this does not currently
use RWCC).  Eventually it could but for the current use case I am building this for, it's
okay like this.
> 
> 
> This addresses bug HBASE-2946.
>     http://issues.apache.org/jira/browse/HBASE-2946
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/Increment.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1026930

>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1026930 
>   trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1026930 
>   trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1026930

> 
> Diff: http://review.cloudera.org/r/1088/diff
> 
> 
> Testing
> -------
> 
> Added TestFromClientSide.testIncrement() which adds some client-side tests of Increment
(and mixing w/ original icv call).  That passes and most the way through a test suite run.
> 
> 
> Thanks,
> 
> Jonathan
> 
>


Mime
View raw message