cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Blake Eggleston (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-10082) Transactional classes shouldn't also implement streams, channels, etc
Date Mon, 17 Aug 2015 19:49:47 GMT

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

Blake Eggleston commented on CASSANDRA-10082:
---------------------------------------------

I wouldn't say it's never ever the right thing to do. Though I would say it's not right for
most AutoClosable use cases that intersect with Transactional implementations (especially
OutputStream). In fact, if you're using a Transactional class as an OutputStream for the purpose
of making a write a noop, you may be committing reviewer abuse :).

Regarding the class living in SequentialWriter, it's not meant to be a general purpose wrapper,
but a one off thing for SequentialWriter. I usually wouldn't create a generic solution unless
it turns out to be a problem with more than just SequentialWriter.


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

Mime
View raw message