> On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 103 > > <https://reviews.apache.org/r/5073/diff/3/?file=108613#file108613line103> > > > > There are some missing @Override's here, correct?
Yep. Adding it. > On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 201 > > <https://reviews.apache.org/r/5073/diff/3/?file=108613#file108613line201> > > > > Shouldn't we flush the table after batchSize has been processed? Actually, it is ok not to. This is because we use Htable.batch(), which flushes immediately. Even the increment is supposed to send individual increments immediately to HBase. Only puts are affected by flushes. > On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 164 > > <https://reviews.apache.org/r/5073/diff/3/?file=108613#file108613line164> > > > > cf.getBytes(Charsets.UTF8) doesn't throw a checked exception. Done. > On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 216 > > <https://reviews.apache.org/r/5073/diff/3/?file=108613#file108613line216> > > > > Since we have a pending exception to throw, how about logging any > > Exception from rollback? That is the tack I took in the ChannelProcessor. Done. > On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleHbaseEventSerializer.java, > > line 96 > > <https://reviews.apache.org/r/5073/diff/3/?file=108615#file108615line96> > > > > Same as above, an alternative getBytes() method does not throw the > > checked exception. Done. > On 2012-05-12 15:42:20, Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 87 > > <https://reviews.apache.org/r/5073/diff/3/?file=108613#file108613line87> > > > > Doesn't look like you can set this? Seems like the last update, I removed the configurability of this - will put it back. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5073/#review7828 ----------------------------------------------------------- On 2012-05-11 20:29:38, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5073/ > ----------------------------------------------------------- > > (Updated 2012-05-11 20:29:38) > > > Review request for Flume. > > > Summary > ------- > > Hbase sink. > > > This addresses bug FLUME-1183. > https://issues.apache.org/jira/browse/FLUME-1183 > > > Diffs > ----- > > bin/flume-ng 0108997 > flume-ng-dist/pom.xml 5bdcfe7 > flume-ng-sinks/flume-ng-hbase-sink/pom.xml PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HbaseEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleHbaseEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleRowKeyGenerator.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestHBaseSink.java > PRE-CREATION > flume-ng-sinks/pom.xml acb3087 > pom.xml 8c11a2d > > Diff: https://reviews.apache.org/r/5073/diff > > > Testing > ------- > > Unit tests added > > > Thanks, > > Hari > >
