cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benedict (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
Date Fri, 15 May 2015 09:38:00 GMT

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

Mime
View raw message