hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: Increment multiple columns in a row at once
Date Mon, 25 Oct 2010 21:26:13 GMT


> On 2010-10-24 21:41:48, khemani wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/client/Increment.java, line 45
> > <http://review.cloudera.org/r/1088/diff/2/?file=15904#file15904line45>
> >
> >     setWriteToWal() is missing?

yup.  will add.


> On 2010-10-24 21:41:48, khemani wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/client/Increment.java, lines 46-47
> > <http://review.cloudera.org/r/1088/diff/2/?file=15904#file15904line46>
> >
> >     why a navigable map? why not just a map?

u can do things like tailMap[() with it.


> On 2010-10-24 21:41:48, khemani wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 3012
> > <http://review.cloudera.org/r/1088/diff/2/?file=15907#file15907line3012>
> >
> >     I am not sure how it is ensured that the order of iteration over the columns
in family.getValue.entrySet() is same as the order of results returned?
> >     
> >     Also, if get finds multiple matches then will it return all of them? If yes
then this will not work.

familyMap and the map of columns to amounts are both TreeMaps ordered with Bytes.BYTES_COMPARATOR.
 Results are also guaranteed to be in order.  And our Get has maxVersions=1 so we will not
get multiple matches per column.


On 2010-10-24 21:41:48, Jonathan Gray wrote:
> > public long upsert(List<KeyValue> kvs) {
> > 
> > Should this be private? If it were private then would you have removed the this.readLock.lock()
locking?

it's called from outside MemStore so needs to be public


- Jonathan


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


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