> On 2012-05-02 17:05:30, Arvind Prabhakar wrote: > > flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelProcessor.java, > > line 1 > > <https://reviews.apache.org/r/4837/diff/2/?file=103887#file103887line1> > > > > Missing the license header.
fixed > On 2012-05-02 17:05:30, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java, > > lines 121-145 > > <https://reviews.apache.org/r/4837/diff/2/?file=103886#file103886line121> > > > > Perhaps this is equivalent to: > > > > ... > > } catch (Throwable th) { > > if (tx != null) { > > try { > > tx.rollback(); > > } catch (Exception ex) { > > LOG.warn("exception...", ex); > > } > > } > > Throwables.propagate(th); > > } finally { > > if (tx != null) { > > tx.close(); > > } > > } > > > > If you agree, I would request that we follow this logic instead in > > other places as well below. Agreed, updated in the latest patch. - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4837/#review7474 ----------------------------------------------------------- On 2012-05-05 15:44:22, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4837/ > ----------------------------------------------------------- > > (Updated 2012-05-05 15:44:22) > > > Review request for Flume. > > > Summary > ------- > > ChannelProcessor did not handle transactions well, in some cases if anything > but ChannelException was thrown, the close in the finally would throw an > exception as rollback was not called. In other cases if a subclass of error > was thrown the same would occurred. Additionally, if getTransaction threw an > exception the null transaction value was not handled and an NPE would be > thrown. > > > This addresses bug FLUME-1131. > https://issues.apache.org/jira/browse/FLUME-1131 > > > Diffs > ----- > > flume-ng-core/pom.xml b798b34 > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java > eb6460e > > flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelProcessor.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/4837/diff > > > Testing > ------- > > Tests added and tests pass. > > > Thanks, > > Brock > >
