You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Brock Noland <br...@cloudera.com> on 2012/03/30 17:56:16 UTC
Channel/Transaction States
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?
Brock
--
Apache MRUnit - Unit testing MapReduce - http://incubator.apache.org/mrunit/
Re: Channel/Transaction States
Posted by Brock Noland <br...@cloudera.com>.
Hi,
On Fri, Mar 30, 2012 at 4:18 PM, Arvind Prabhakar <ar...@apache.org> wrote:
> On Fri, Mar 30, 2012 at 11:25 AM, Prasad Mujumdar <pr...@cloudera.com> wrote:
>> On Fri, Mar 30, 2012 at 10:09 AM, Arvind Prabhakar <ar...@apache.org>wrote:
>>
>>> On Fri, Mar 30, 2012 at 8:56 AM, Brock Noland <br...@cloudera.com> 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/
Re: Channel/Transaction States
Posted by Arvind Prabhakar <ar...@apache.org>.
On Fri, Mar 30, 2012 at 11:25 AM, Prasad Mujumdar <pr...@cloudera.com> wrote:
> On Fri, Mar 30, 2012 at 10:09 AM, Arvind Prabhakar <ar...@apache.org>wrote:
>
>> On Fri, Mar 30, 2012 at 8:56 AM, Brock Noland <br...@cloudera.com> 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
Re: Channel/Transaction States
Posted by Prasad Mujumdar <pr...@cloudera.com>.
On Fri, Mar 30, 2012 at 10:09 AM, Arvind Prabhakar <ar...@apache.org>wrote:
> On Fri, Mar 30, 2012 at 8:56 AM, Brock Noland <br...@cloudera.com> 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.
thanks
Prasad
> Thanks,
> Arvind
>
>
> >
> > Brock
> >
> > --
> > Apache MRUnit - Unit testing MapReduce -
> http://incubator.apache.org/mrunit/
>
Re: Channel/Transaction States
Posted by Arvind Prabhakar <ar...@apache.org>.
On Fri, Mar 30, 2012 at 8:56 AM, Brock Noland <br...@cloudera.com> 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.
Thanks,
Arvind
>
> Brock
>
> --
> Apache MRUnit - Unit testing MapReduce - http://incubator.apache.org/mrunit/