cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joshua McKenzie (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-8984) Introduce Transactional API for behaviours that can corrupt system state
Date Mon, 20 Apr 2015 18:38:59 GMT

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

Joshua McKenzie commented on CASSANDRA-8984:
--------------------------------------------

h6. Feedback:
There's enough usage of finish() and finishAndClose() floating around that it comes off as
an undocumented extension to the Transactional interface. Given how clear the flow of usage
is documented in the header on Transactional, it's a bit surprising to see finish() and finishAndClose()
calls with the frequency they have - there might be some value in codifying those methods
/ documenting their idiomatic usage.

BigTableWriter reaching into the guts of IndexWriter is error-prone as future changes to resources
in IndexWriter would need to be managed in the TxnProxy in BigTableWriter. Perhaps we could
change IndexWriter to implement Transactional and keep the management of its resources more
localized?

For instance, rather than seeing:
{noformat}
protected Throwable doCleanup(Throwable accumulate)
{
   accumulate = iwriter.summary.close(accumulate);
   accumulate = iwriter.bf.close(accumulate);
   accumulate = iwriter.builder.close(accumulate);
   accumulate = dbuilder.close(accumulate);
   return accumulate;
}
{noformat}

I'd be much more comfortable seeing:
{noformat}
protected Throwable doCleanup(Throwable accumulate)
{
   accumulate = iwriter.close(accumulate);
   accumulate = dbuilder.close(accumulate);
   return accumulate;
}
{noformat}

h6. Nits:
We may want to convert the touched /io tests to take advantage of and exercise the various
writers being Transactional
Formatting change in SSTableImport.java for JsonFactory factory decl/init is unnecessary

Tests look good on both linux and windows.

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

Mime
View raw message