[
https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14545214#comment-14545214
]
Benedict edited comment on CASSANDRA-8568 at 5/15/15 9:37 AM:
--------------------------------------------------------------
bq. the commit / finish relation in SSTRW is a bit confusing - sometimes we
finish the SSTRW (in CompactionTask), and sometimes we don't
(CompactionManager#antiCompactGroup).
antiCompactGroup is a particularly weird case, and I've put some comments in to
clarify this one in particular. Let me know if there's still confusion in any
of the other places.
bq. in Tracker: Throwable apply(Function<View, View> function, Throwable
accumulate) - catch Throwable instead of Exception?
Yes, thanks - fixed
bq. java.lang.NoClassDefFoundError: org/apache/mina/util/IdentityHashSet on
startup - guess you forgot git add;ing a new .jar in lib/?
This was a mistake: it's included on the build path and I accidentally included
it, forgetting JDK only had IdentityHashMap. It had already broken dtests and a
fix is already up (though we should probably fix build.xml to let that break
unit tests too)
bq. Fault injection tests
I think these need splitting out into a separate ticket, probably
CASSANDRA-9165, and I probably should have clarified that I did not expect them
to be committed with this review. They are a non-trivial undertaking in their
own right to get done properly, and there probably isn't time to do them
justice. I just wanted to get some minimal tooling to manually verify the new
code works in these scenarios (which can be run in IDEA as a JUnit task).
I'll address your nits and some unit tests for Transaction itself shortly, if
that all sounds good to you.
was (Author: benedict):
bq. the commit / finish relation in SSTRW is a bit confusing - sometimes we
finish the SSTRW (in CompactionTask), and sometimes we don't
(CompactionManager#antiCompactGroup).
antiCompactGroup is a particularly weird case, and I've put some comments in to
clarify this one in particular. Let me know if there's still confusion in any
of the other places.
bq. in Tracker: Throwable apply(Function<View, View> function, Throwable
accumulate) - catch Throwable instead of Exception?
Yes, thanks - fixed
bq. java.lang.NoClassDefFoundError: org/apache/mina/util/IdentityHashSet on
startup - guess you forgot git add;ing a new .jar in lib/?
This was a mistake: it's included on the build path and I accidentally included
it, forgetting JDK only had IdentityHashMap. It had already broken dtests and a
fix is already up (though we should probably fix build.xml to let that break
unit tests too)
bq. Fault injection tests
I think these need splitting out into a separate ticket, probably
CASSANDRA-9165, and I probably should have clarified that I did not expect them
to be committed with this review. They are a non-trivial undertaking in their
own right to get done properly, and there probably isn't time to do them
justice. I just wanted to get some minimal tooling to manually verify the new
code works in these scenarios.
I'll address your nits and some unit tests for Transaction itself shortly, if
that all sounds good to you.
> Impose new API on data tracker modifications that makes correct usage obvious
> and imposes safety
> ------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-8568
> URL: https://issues.apache.org/jira/browse/CASSANDRA-8568
> Project: Cassandra
> Issue Type: Bug
> Reporter: Benedict
> Assignee: Benedict
> Fix For: 2.2 rc1
>
>
> DataTracker has become a bit of a quagmire, and not at all obvious to
> interface with, with many subtly different modifiers. I suspect it is still
> subtly broken, especially around error recovery.
> I propose piggy-backing on CASSANDRA-7705 to offer RAII (and GC-enforced, for
> those situations where a try/finally block isn't possible) objects that have
> transactional behaviour, and with few simple declarative methods that can be
> composed simply to provide all of the functionality we currently need.
> See CASSANDRA-8399 for context
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
