You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Roshan Naik <ro...@hortonworks.com> on 2013/04/09 05:21:37 UTC

Review Request: BasicTransactionSemantics should avoid throw-ing from close()

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/
-----------------------------------------------------------

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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Roshan Naik <ro...@hortonworks.com>.

> On April 16, 2013, 10:51 p.m., Mike Percy wrote:
> > I think it'd be great to preserve or enable the Lock-like idiom as a best-practice, i.e.
> > 
> > txn.begin()
> > try {
> >   // do stuff, puts, whatever
> >   ch.put(...);
> >   ch.commit();
> > } catch (FlumeException ex) {
> >   ch.rollback();
> > } finally {
> >   ch.close();
> > }
> > 
> > So whatever gets us there (right now we have to catch Throwable which is pretty horrible) I would support.
> > 
> > May require changes to the channel implementations which is fine. After mulling it over a bit, I think we should encourage channel implementations to do their own rollback-on-close and log an ERROR if the txn is still open at close time, because it will give them more ability to react to implementation specific details, like if rollback throws in the middle of a close.
> 
> Mike Percy wrote:
>     BTW, I meant to write txn.close() in the finally block above

So ..  
- BasicTransactionSemanitcs.doClose() will be marked abstract and all the channels will implement this in their transaction class 
- BasicTransactionSemanitcs.close()  method's task will henceforth be delegated down to the channel's   doClose()  

is that right ?


- Roshan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review19278
-----------------------------------------------------------


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Mike Percy <mp...@apache.org>.

> On April 16, 2013, 10:51 p.m., Mike Percy wrote:
> > I think it'd be great to preserve or enable the Lock-like idiom as a best-practice, i.e.
> > 
> > txn.begin()
> > try {
> >   // do stuff, puts, whatever
> >   ch.put(...);
> >   ch.commit();
> > } catch (FlumeException ex) {
> >   ch.rollback();
> > } finally {
> >   ch.close();
> > }
> > 
> > So whatever gets us there (right now we have to catch Throwable which is pretty horrible) I would support.
> > 
> > May require changes to the channel implementations which is fine. After mulling it over a bit, I think we should encourage channel implementations to do their own rollback-on-close and log an ERROR if the txn is still open at close time, because it will give them more ability to react to implementation specific details, like if rollback throws in the middle of a close.

BTW, I meant to write txn.close() in the finally block above


- Mike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review19278
-----------------------------------------------------------


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Mike Percy <mp...@apache.org>.

> On April 16, 2013, 10:51 p.m., Mike Percy wrote:
> > I think it'd be great to preserve or enable the Lock-like idiom as a best-practice, i.e.
> > 
> > txn.begin()
> > try {
> >   // do stuff, puts, whatever
> >   ch.put(...);
> >   ch.commit();
> > } catch (FlumeException ex) {
> >   ch.rollback();
> > } finally {
> >   ch.close();
> > }
> > 
> > So whatever gets us there (right now we have to catch Throwable which is pretty horrible) I would support.
> > 
> > May require changes to the channel implementations which is fine. After mulling it over a bit, I think we should encourage channel implementations to do their own rollback-on-close and log an ERROR if the txn is still open at close time, because it will give them more ability to react to implementation specific details, like if rollback throws in the middle of a close.
> 
> Mike Percy wrote:
>     BTW, I meant to write txn.close() in the finally block above
> 
> Roshan Naik wrote:
>     So ..  
>     - BasicTransactionSemanitcs.doClose() will be marked abstract and all the channels will implement this in their transaction class 
>     - BasicTransactionSemanitcs.close()  method's task will henceforth be delegated down to the channel's   doClose()  
>     
>     is that right ?

I think we need to wait for Flume 2.0 to do that since it's an API break. Probably what we should do instead is simply document the API contract for Channel.close() implementations and fix any broken implementations in the core.


- Mike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review19278
-----------------------------------------------------------


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review19278
-----------------------------------------------------------


I think it'd be great to preserve or enable the Lock-like idiom as a best-practice, i.e.

txn.begin()
try {
  // do stuff, puts, whatever
  ch.put(...);
  ch.commit();
} catch (FlumeException ex) {
  ch.rollback();
} finally {
  ch.close();
}

So whatever gets us there (right now we have to catch Throwable which is pretty horrible) I would support.

May require changes to the channel implementations which is fine. After mulling it over a bit, I think we should encourage channel implementations to do their own rollback-on-close and log an ERROR if the txn is still open at close time, because it will give them more ability to react to implementation specific details, like if rollback throws in the middle of a close.

- Mike Percy


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Brock Noland <br...@cloudera.com>.

> 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 ?

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.


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Brock Noland <br...@cloudera.com>.

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

Yeah that is a good point. The file channel needs rollback to be called.... 


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Roshan Naik <ro...@hortonworks.com>.

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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Roshan Naik <ro...@hortonworks.com>.

> 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....

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 ?


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Brock Noland <br...@cloudera.com>.

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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Brock Noland <br...@cloudera.com>.

> 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 ?
> 
> Brock Noland wrote:
>     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.

It'd still be "polite" and recommended to rollback the transaction if not technically required. We could also log and error if we had to log rollback the transaction.


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review18884
-----------------------------------------------------------



flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
<https://reviews.apache.org/r/10362/#comment39340>

    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)


- Hari Shreedharan


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


Re: Review Request: BasicTransactionSemantics should avoid throw-ing from close()

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10362/#review18881
-----------------------------------------------------------



flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
<https://reviews.apache.org/r/10362/#comment39337>

    Is this what we want?  Can we have a test for this?


- Brock Noland


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