> On 2012-03-30 08:33:50, Mike Percy wrote: > > Since we are catching Throwable here, we may want to do it for FLUME-1074 > > too. Shouldn't we do this in all places where Transaction.commit() is > > called, if for nothing else than consistency? > > Brock Noland wrote: > +1 > > Prasad Mujumdar wrote: > Perhaps we should have a cleanup method in transaction interface that > rolls back the transaction if its still open and then close it. This way we > can consolidate the transaction error handling in a single finally block > instead of multiple catch blocks. > > Brock Noland wrote: > I mailed flume-dev on a similar issue "Channel/Transaction States" > > Brock Noland wrote: > I would like to note that I think catching Throwable in 1074 and or > creating this idea of a cleanup shouldn't hold up either 1074 or 1075. These > are both good incremental changes.
@Brock, sorry I didn't see the thread earlier. I will log a new ticket once we have a consensus on the dev list thread. @Mike, if we are going to change the close() logic, then we probably won't need to change all the exception handling code. - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4569/ > ----------------------------------------------------------- > > (Updated 2012-03-30 05:38:44) > > > Review request for Flume, Arvind Prabhakar and Hari Shreedharan. > > > Summary > ------- > > Unhandled thorwable in HDFS sink can leave transaction open. > > > This addresses bug FLUME-1075. > https://issues.apache.org/jira/browse/FLUME-1075 > > > Diffs > ----- > > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > eee9221 > > Diff: https://reviews.apache.org/r/4569/diff > > > Testing > ------- > > regression tests > > > Thanks, > > Prasad > >
