[
https://issues.apache.org/jira/browse/FLUME-935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13196640#comment-13196640
]
[email protected] commented on FLUME-935:
-----------------------------------------------------
bq. On 2012-01-31 01:36:50, Arvind Prabhakar wrote:
bq. > Thanks for the patch Peter. The framework looks good to me. A couple of
questions/comments:
bq. >
bq. > 1. For reference counting the transaction, it is probably better to do
decrement the nesting count within close() instead of commit/rollback. This is
because the idiom for transaction use requires the close call to be called in
finally block which provides for a stronger guarantee.
bq. >
bq. > 2. You mention in the description regarding test cases. Since you have
done the majority of the work already, would you mind adding a few test cases
to it to assert its workings? Apart from validating that the framework works
well, these test cases will also help preserve the correctness from inadvertent
changes that may indirectly impact its functioning.
bq. >
bq. > Thanks,
bq. > Arvind
I'll think about (1) some more (I think I had a reason to do it that way), and
will either do as you suggest or document why I did it that way (in which case
we can discuss it further).
I'd be happy to do (2), and based on other testing I've done have already made
a couple of minor fixes/tweaks to the patch anyway.
Thanks for the review!
-peter
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3516/#review4697
-----------------------------------------------------------
On 2012-01-18 00:05:21, Peter Newcomb wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3516/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-01-18 00:05:21)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. 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.
bq.
bq.
bq. This addresses bug FLUME-935.
bq. https://issues.apache.org/jira/browse/FLUME-935
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java
PRE-CREATION
bq.
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
PRE-CREATION
bq.
/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelUtils.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/3516/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. 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.
bq.
bq. All that said, it wouldn't be a bad idea to create unit tests around
these, and ideally reusable test classes to test the basic use cases for any
Channel implementation or client.
bq.
bq.
bq. Thanks,
bq.
bq. Peter
bq.
bq.
> 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
> 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
