[
https://issues.apache.org/jira/browse/CASSANDRA-8984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14542363#comment-14542363
]
Joshua McKenzie commented on CASSANDRA-8984:
--------------------------------------------
bq. I brought the semantics of each of SequentialWriter inline with
expectation, i.e. it deletes its file on abort.
I see us closing our file handles on SequentialWriter's abort sequence but
don't see file deletion, nor are we checking for that in SequentialWriterTest.
The comment in SequentialWriter also states that we're not deleting the file on
abort.
{noformat}
// we don't do anything on abort; deleting the files is up to others (retaining
prior behaviour)
// TODO: revisit this decisio
{noformat}
That being said, I like the new unit tests you've added for the various
Transactionals and they look good. I haven't gone so far as to deeply sanity
check the states checked for in BigTableWriterTest.assertInProgress, however
reading through them they look correct, the rest are good, and test results
look good.
There are some unused imports in BigTableWriterTest and
ChecksummedSequentialWriterTest and if you could clear up what I'm missing re:
SequentialWriters deleting their files on abort, I'm +1 overall.
> Introduce Transactional API for behaviours that can corrupt system state
> ------------------------------------------------------------------------
>
> Key: CASSANDRA-8984
> URL: https://issues.apache.org/jira/browse/CASSANDRA-8984
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Benedict
> Assignee: Benedict
> Fix For: 2.2 beta 1
>
> Attachments: 8984_windows_timeout.txt
>
>
> As a penultimate (and probably final for 2.1, if we agree to introduce it
> there) round of changes to the internals managing sstable writing, I've
> introduced a new API called "Transactional" that I hope will make it much
> easier to write correct behaviour. As things stand we conflate a lot of
> behaviours into methods like "close" - the recent changes unpicked some of
> these, but didn't go far enough. My proposal here introduces an interface
> designed to support four actions (on top of their normal function):
> * prepareToCommit
> * commit
> * abort
> * cleanup
> In normal operation, once we have finished constructing a state change we
> call prepareToCommit; once all such state changes are prepared, we call
> commit. If at any point everything fails, abort is called. In _either_ case,
> cleanup is called at the very last.
> These transactional objects are all AutoCloseable, with the behaviour being
> to rollback any changes unless commit has completed successfully.
> The changes are actually less invasive than it might sound, since we did
> recently introduce abort in some places, as well as have commit like methods.
> This simply formalises the behaviour, and makes it consistent between all
> objects that interact in this way. Much of the code change is boilerplate,
> such as moving an object into a try-declaration, although the change is still
> non-trivial. What it _does_ do is eliminate a _lot_ of special casing that we
> have had since 2.1 was released. The data tracker API changes and compaction
> leftover cleanups should finish the job with making this much easier to
> reason about, but this change I think is worthwhile considering for 2.1,
> since we've just overhauled this entire area (and not released these
> changes), and this change is essentially just the finishing touches, so the
> risk is minimal and the potential gains reasonably significant.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
