> On Dec. 31, 2013, 6:43 a.m., Bruno MAHE wrote: > > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSink.java, > > lines 181-182 > > <https://reviews.apache.org/r/16415/diff/1/?file=401611#file401611line181> > > > > Naive question: > > If your serializer fails to serialize an event for any reason (ex: > > event is not well formed), then currently you would rollback the > > transaction and try again forever. > > Matt Molek wrote: > Split the committing the transaction off into its own method, and changed > the exception handling so the transaction is only rolled back for an > exception in the commit itself. Failure in the serializer won't cause a > rollback now. This follows the way the hbase sink is written. > > Hari Shreedharan wrote: > Frankly, I think the approach in this class is fine. If the event is not > well-formed, there is nothing that can be done (A bad event should cause > issues that ops should notice). If the user is ok with dropping ill-formed > events, they should catch any exceptions within the serializer and return an > empty list of mutations. You need to throw an exception and retry if the > serializer throws. So the code in this patch does look good to me - there is > no real need to change it. > > We recently fixed the HBaseSink code to do this - earlier it had issues > with transactions not being closed if the serializer threw - which would > happen if you don't catch exceptions thrown by the serializer
I think my changes follow the way the HBaseSink is currently set up. Though now that you mention it, I see how an exception in the serializer would cause the transaction to be left open. Am I looking at the wrong branch? I'm working on the trunk branch from 'git clone http://git-wip-us.apache.org/repos/asf/flume.git'. Or did you mean the AsyncHBaseSink? In AsyncHBaseSink I see there is an effort to always rollback and close the transaction if there are exceptions pretty much anywhere the process method. So, I should keep the logic the way it was before, where any exception in the process method leads to a transaction rollback? Would this lead to bad transactions being rolled back and tried again forever? - Matt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16415/#review30996 ----------------------------------------------------------- On Dec. 27, 2013, 6 p.m., Matt Molek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16415/ > ----------------------------------------------------------- > > (Updated Dec. 27, 2013, 6 p.m.) > > > Review request for Flume. > > > Repository: flume-git > > > Description > ------- > > This patch is identical to FLUME-2234-0.patch except I had to regenerate it > with different git options to get review board to accept it. > > > Diffs > ----- > > flume-ng-sinks/flume-ng-accumulo-sink/pom.xml PRE-CREATION > > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSink.java > PRE-CREATION > > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSinkConfigurationConstants.java > PRE-CREATION > > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/SimpleAccumuloEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-accumulo-sink/src/test/java/org/apache/flume/sink/accumulo/TestAccumuloSink.java > PRE-CREATION > flume-ng-sinks/pom.xml d03576b > > Diff: https://reviews.apache.org/r/16415/diff/ > > > Testing > ------- > > > Thanks, > > Matt Molek > >
