> On 2012-04-10 00:01:17, Arvind Prabhakar wrote: > > Thanks for the patch Brock. I think what this patch does is forces a state > > transition on close no matter what. This has the potential of covering up > > for programmatic problems that could lead to resource/tx leaks in the > > system which I feel should not happen. If a component is buggy, the other > > components around it should not try to coverup. > > > > Another way to look at it is - the close() method should not throw an > > exception ever. This can be further reinforced by having a thread local > > transaction that is discarded on close. > > Brock Noland wrote: > I can agree with that. > > The new code would do the state transition (which means a new transaction > is gotten on getTransaction()) and then call doClose(). Correct? > > Arvind Prabhakar wrote: > My view on it is that there are two parts to this problem: > > 1. If someone calls close() when the tx is not in the correct state, that > should fail with an exception. This signals a bad/buggy implementation that > should be identified aggressively and fixed. > > 2. If someone calls close() when the tx is in the correct state, that > should never fail. This will ensure that good code is not penalized for > implementation issues of the tx provider. > >
In my understanding from the email chain "Channel/Transaction States" was that like a DB statement, you should be able to call close() should be safe to call at any point in time. If work is uncommitted that work is thrown away. If we require rollback or commit to be called before close, then every source/sink needs to catch Throwable, call rollback and rethrow so that close can be called in the finally block. Thoughts? - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4655/#review6810 ----------------------------------------------------------- On 2012-04-05 03:05:51, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4655/ > ----------------------------------------------------------- > > (Updated 2012-04-05 03:05:51) > > > Review request for Flume. > > > Summary > ------- > > Allowing the calling of transaction.close() at any point of time. > > > This addresses bug FLUME-1089. > https://issues.apache.org/jira/browse/FLUME-1089 > > > Diffs > ----- > > > flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java > 403cbca > > flume-ng-core/src/test/java/org/apache/flume/channel/TestBasicChannelSemantics.java > 80020fc > > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java > bc81f26 > > Diff: https://reviews.apache.org/r/4655/diff > > > Testing > ------- > > Unit tests pass. > > > Thanks, > > Brock > >
