----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7657/#review14116 -----------------------------------------------------------
Hari, looks like a good patch! I have a few comments and questions below. flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java <https://reviews.apache.org/r/7657/#comment30185> Why do we batch the puts into one runnable but not the increments? flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java <https://reviews.apache.org/r/7657/#comment30189> The formatting is kind of off because of the number of changes here so maybe I am off, but we also call tnx.commit() and tnx.close() below. Are we sure that the future.get() will throw an exception everytime and as such we won't hit those lines? flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java <https://reviews.apache.org/r/7657/#comment30186> Curious as to why we removed this behavior? flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java <https://reviews.apache.org/r/7657/#comment30184> I wonder what the performance impact will be of flushing every increment? - Brock Noland On Oct. 19, 2012, 12:14 a.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7657/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2012, 12:14 a.m.) > > > Review request for Flume. > > > Description > ------- > > Modify the Hbase sink to use multiple threads to wait on RPC calls. Since the > threads are blocked often, this will not cause too many running threads. > > > This addresses bug FLUME-1649. > https://issues.apache.org/jira/browse/FLUME-1649 > > > Diffs > ----- > > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java > 021ecd0 > > Diff: https://reviews.apache.org/r/7657/diff/ > > > Testing > ------- > > All current unit tests pass. Did functional testing. > > > Thanks, > > Hari Shreedharan > >
