[
https://issues.apache.org/jira/browse/CASSANDRA-8984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14378119#comment-14378119
]
Benedict commented on CASSANDRA-8984:
-------------------------------------
bq. it's a different kind of complexity and price to be paid for having 5
objects with 20x complexity vs. 100 objects with 1x each;
My hope is that these extra classes confer no complexity cost _after
acclimation_ - which may take a little while, of course. Once the APIs are
understood and used daily, many of those 1x complexity burdens disappear. But
I'm well aware this increases a burden initially, and also that I apparently
have a propensity to naming and conventions that are not easily assimilated by
others, so any help that can be offered in spotting potential for improvement
in that area is greatly appreciated. One other thing to comment on, since we're
talking in general about the whole cohort of changes, is that my goal here is
to make it _hard to ignore_ complexity by making it more explicit, which
naturally makes code appear more complex. So my main point against this being
"more" complex is that, at least I _think_ so, the code is only making very
obvious the complexities that were already inherent but were not apparent.
bq. As a general observation, rewriting the logic of SSTRW and parts of SSTW
repeatedly isn't giving this code time to settle. The bug fixes we've made to
previous implementations are out the window each time we re-write this code and
this patch is again doing that on much of the SSTable early open, renaming,
closing, etc.
Agreed. Which is one of the reasons I'm keen to get all of these changes out of
the way in one patch version increment :) - partially this is down to making
the changes "incremental", and partially down to realising more of the
deficiencies of the overall design with each modification. Once 8568 goes in, I
expect changes to this area to fall off dramatically.
bq. Not sure how we want to handle this given our proximity to 3.0 and our
point in the 2.1 release life-cycle.
There _is_ the possibility that we could just try to shore up only this part,
without the overall refactor. I marked CASSANDRA-8690 as a duplicate of this,
but in reality it was smaller in scope and might deliver a lot of the safety
necessary. It could be addressed with exactly that in mind, followed by
expanding the fault injection testing of 8568 to test as many of these
scenarios as we can, to introduce the least number of changes to 2.1. My
concern is this will necessarily take longer to achieve - especially with NGCC
and 3.0 coming up and the many responsibilities each of us have. This change in
contrast is ready to go, and given the major refactoring done in this area
since 2.1.3, the increased risk is likely not huge. But it does introduce its
own risk, and is close to the wire for 2.1.4. I'm not certain which is the
better route, to be honest, so any other opinions more than welcome.
> 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.4
>
> 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)
