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()


> 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.
>
> Roshan Naik wrote:
>     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 ?

Yeah I think we'd have to call rollback, catching and logging any exception. This behavior would be similar to the db where you can close a connection without closing a statement and you can close a statement without closing a result set.

I think we should hear from some other committers before making the change I describe above.
- Brock
-----------------------------------------------------------
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
>
>