[jira] [Commented] (CASSANDRA-10082) Transactional classes shouldn't also implement streams, channels, etc

["Benedict (JIRA)" <ji...@apache.org>]

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

Benedict commented on CASSANDRA-10082:
--------------------------------------

I'm not totally convinced. {{AutoCloseable}} (and, in fact, {{OutputStream}}) both explicitly refer only to the release of associated resources, and say nothing about what should happen to the object state on such a release. Assuming they must always commit any state changes is not necessarily a reasonable assumption. It so happens that many do, but it is not a part of the standard contract, and the contract of the API you are using should always be consulted.

That said, I don't think there's anything wrong with introducing a general purpose wrapper that accepts a {{Transactional & AutoCloseable}}, and reroutes close to finish. But I don't think this should live in {{SequentialWriter}}, as it seems a confusingly arbitrary method call for changing the semantics.

> Transactional classes shouldn't also implement streams, channels, etc
> ---------------------------------------------------------------------
>
>                 Key: CASSANDRA-10082
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10082
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Blake Eggleston
>            Assignee: Blake Eggleston
>         Attachments: 0001-replacing-SequentialWriter-OutputStream-extension-wi.patch
>
>
> Since the close method on the Transactional interface means "abort if commit hasn't been called", mixing Transactional and AutoCloseable interfaces where close means "we're done here" is pretty much never the right thing to do. 
> The only class that does this is SequentialWriter. It's not used in a way that causes a problem, but it's still a potential hazard for future development.
> The attached patch replaces the SequentialWriter OutputStream implementation with a wrapper class that implements the expected behavior on close, and adds a warning to the Transactional interface. It also adds a unit test that demonstrates the problem without the fix.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)