> 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
> 
>

Reply via email to