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.
Thanks,
Arvind Prabhakar
