Hi, On Fri, Mar 30, 2012 at 4:18 PM, Arvind Prabhakar <[email protected]> wrote: > On Fri, Mar 30, 2012 at 11:25 AM, Prasad Mujumdar <[email protected]> > wrote: >> On Fri, Mar 30, 2012 at 10:09 AM, Arvind Prabhakar <[email protected]>wrote: >> >>> On Fri, Mar 30, 2012 at 8:56 AM, Brock Noland <[email protected]> wrote: >>> > I have been thinking about Channel.getTransaction and >>> > BasicTransactionSemantics. In the case of an error occurring, I think >>> > users would expect close to throw away the current transaction and >>> > start fresh. Similar to a JDBC object or file, most code catches and >>> > logs the checked exception the close might throw. >>> > >>> > However, in our close check the state of the current transaction and >>> > throw a runtime exception if it's in the wrong state, as such it >>> > cannot be closed. Additionally, Channel.getTransaction will return the >>> > same transaction over and over again if it's not closed. >>> > >>> > >>> https://github.com/apache/flume/blob/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java#L104 >>> > >>> https://github.com/apache/flume/blob/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java#L176 >>> > >>> > As such, in a source or sink, the only way to "start afresh" is to >>> > have a method as follows: >>> > >>> > try { tran.begin()} } catch(Exception e) {} >>> > try { tran.rollback()} } catch(Exception e) {} >>> > try { tran.close()} } catch(Exception e) {} >>> > >>> > Thoughts? >>> >>> In my opinion, tx.close() should be a safe operation that never throws >>> any exception and cleans up the transaction no matter what. If it so >>> happens that the system is in an inconsistent state, the other methods >>> should throw exceptions to indicate that. The reason for this is that >>> without a safe tx.close() we can never recover from a failed state as >>> you have pointed out. >>> >>> >> +1 >> Though I think we should have a free() which does what close() does today. >> Then a close() would try to silently rollback the transaction if its open >> before freeing it. > > I believe that close() and free() are equivalent. The current idiom > for tx use is as follows: > > Channel ch = ... > Transaction tx = ch.getTransaction(); > try { > tx.begin(); > ... > // ch.put(event) or ch.take() > ... > tx.commit(); > } catch (ChannelException ex) { > tx.rollback(); > ... > } finally { > tx.close(); > } > > As you can see, there is a clear place for close in order to ensure > that the transaction resources are released. Adding a free() here > would not add any value than what is already provided by this idiom.
I created a JIRA for this. https://issues.apache.org/jira/browse/FLUME-1089 Cheers, Brock -- Apache MRUnit - Unit testing MapReduce - http://incubator.apache.org/mrunit/
