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

[email protected] commented on FLUME-935:
-----------------------------------------------------


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

(Updated 2012-02-06 06:37:40.844119)


Review request for Flume.


Changes
-------

This patch incorporates suggested changes and fixes, an additional fix or two, 
and fairly comprehensive unit tests, though not fully reusable ones as I 
earlier suggested might be nice to have.

One fix had to do with handling Errors as well as Exceptions--I ran across the 
need for this in practice, when a codec was not installed properly.  This fix 
required modifying ChannelException to accept a Throwable as the cause, not 
just an Exception.  I hope that's OK.

With regard to the reference counting question regarding the support for nested 
transactions: I was doing the reference counting there in order to limit the 
nesting behavior to between begin() and commit()/rollback(), so that one would 
not necessarily have to call begin() [or getTransaction()] and close() in 
perfect nested pairs--I was considering close() a resource release, not a 
fundamental part of the transaction process.

However, as I thought about nested transactions, I realized that they seem to 
be a non-requirement.  I thought that they were at least a nice-to-have based 
on the JDBC channel implementation, but nothing else seems to expect them.  I 
therefore removed them entirely pending identification of need and/or further 
work and discussion to figure out how they should work.


Summary
-------

Implementation of FLUME-935 as new classes BasicChannelSemantics, 
BasicTransactionSemantics, and ChannelUtils.  It might be better to fold 
BasicChannelSemantics into AbstractChannel and rename BasicTransactionSemantics 
to AbstractTransaction, but doing that would require refactoring of existing 
classes that extend AbstractChannel.


This addresses bug FLUME-935.
    https://issues.apache.org/jira/browse/FLUME-935


Diffs (updated)
-----

  
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java
 PRE-CREATION 
  
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
 PRE-CREATION 
  
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelUtils.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/3516/diff


Testing (updated)
-------

I am using these in production code, and they have survived significant 
integration testing there, including failure modes.  Note also that these 
classes are largely error handling and precondition testing code designed to 
test the correctness of the code around them.

A fairly comprehensive set of unit tests around BasicChannelSemantics and 
BasicTransactionSemantics is included.


Thanks,

Peter


                
> 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

        

Reply via email to