[jira] [Commented] (FLUME-935) Create abstract implementations of basic channel/transaction semantics

["Juhani Connolly (Commented) (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLUME-935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13197563#comment-13197563 ] 

Juhani Connolly commented on FLUME-935:
---------------------------------------

A problem came up when unit testing Flume-936 which I'm basing around your framework:

- When I throw a ChannelException from MemoryChannel in doCommit, it is caught by commit in the RuntimeException clause where it is then further propagated. If in the higher up code I attempt to catch a ChannelException, it does not get caught, going straight to the finally clause.

For example:
    try {
      transaction.begin();
      channel.put(EventBuilder.withBody("overflow event".getBytes()));
      transaction.commit();  //  <------------------------------------ This causes  a  ChannelException in doCommit
    } catch (ChannelException e) {
      overflowed = true;       // <-------------------------------------- Never goes here
      transaction.rollback();
    } catch (Exception e) {
      overflowed = true;       // <-------------------------------------- Goes right here
      transaction.rollback();
    } finally {
      transaction.close();
    }

(tbh, I have no clue why this works like this but I confirmed this in the debugger... must be a detail of exception handling I had never come across before)

I do not think that allowing doCommit and such to throw anything other than a ChannelException(which is unchecked anyway) is a good idea, and making the definitions as throws Exception is definitely far too loose. If things are left as is,  people will need to write code like this when working with any channel that throws ChannelException from doXYZ

try {
  transaction.xyz();
catch (RuntimException e) {
  if(e instanceof ChannelException) {
     transaction.rollback();
  } else {
    throw e;
  }
}
                
> Create abstract implementations of basic channel/transaction semantics
> ----------------------------------------------------------------------
>
>                 Key: FLUME-935
>                 URL: https://issues.apache.org/jira/browse/FLUME-935
>             Project: Flume
>          Issue Type: Improvement
>          Components: Channel
>    Affects Versions: v1.0.0
>            Reporter: Peter Newcomb
>            Assignee: Peter Newcomb
>            Priority: Minor
>             Fix For: v1.1.0
>
>
> Correctly executing or checking the state transitions for channels and transactions is nontrivial.  It would be helpful to have a correct implementation of each that can be used either directly or as a reference when creating new channels or clients of channels.
> Specifically, on the client side it would be nice to package the try { begin() ... commit() } catch { rollback() } finally { close() } code, with all the appropriate exception propagation and logging code, so that it need not be repeated constantly.
> On the channel side, it'd be nice to have a packaged implementation of the implied ThreadLocal semantics of the Transaction class, along with Preconditions checking to make sure that clients follow the try { begin() ... commit() } catch { rollback() } finally { close() } pattern.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira