Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Flume, mail # dev - Review Request: BasicTransactionSemantics should avoid throw-ing from close()


Copy link to this message
-
Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()
Roshan Naik 2013-04-10, 18:28


> On April 9, 2013, 8:46 p.m., Hari Shreedharan wrote:
> > flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java, line 182
> > <https://reviews.apache.org/r/10362/diff/1/?file=279141#file279141line182>
> >
> >     I agree with Brock. This should not be the case. The file channel can lose data if we don't rollback (unless there is a replay)
>
> Brock Noland wrote:
>     Yeah that is a good point. The file channel needs rollback to be called....
>
> Roshan Naik wrote:
>     Hari, Brock,
>        Not clear to me what the implied suggestion for change is. Are you suggesting rollback() should be called from within close() ? Or ...  leave it as it originally was ?
>
> Brock Noland wrote:
>     Hi,
>    
>     My first comment was that the code looks incorrect to me in that it prints an error message when the transaction is NEW or COMPLETED. Hari brings up a good point though. File channel requires a rollback otherwise data will be stuck in limbo. Memory channel may have this as well, not sure.  Therefore, I think if the transaction has not been rolledback/committed we need to execute a rollback.

Ok. If we add an automatic rollback inside close() ..
- It will have a significant impact to the programming model for transactions. Essentially  there will be no need to call rollback() and then close() .. folks can directly call close and never bother calling rollback(). Today close() is only serving one purpose as far as i can tell.. perform some validations to ensure programmers are using the transactions correctly.

- close() will  continue to throw and this jira's intent will not be addressed.

It seems to me that this jira should be dropped.  Thoughts ?
- Roshan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review18884
-----------------------------------------------------------
On April 9, 2013, 3:21 a.m., Roshan Naik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10362/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 3:21 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Loggin error instead of throwing exception in cases where close() is called w/o rollback or abort being called.
>
>
> This addresses bug FLUME-1321.
>     https://issues.apache.org/jira/browse/FLUME-1321
>
>
> Diffs
> -----
>
>   flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java 403cbca
>
> Diff: https://reviews.apache.org/r/10362/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Roshan Naik
>
>