[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14553848#comment-14553848 ] Marcus Eriksson commented on CASSANDRA-8568: bq. LifecycleTransaction yes, lets atleast do that. Regarding DataTracker - Tracker, I guess it is fine, but then everything mentioning DataTracker should be changed (for example cfs.getDataTracker()) comments for updated branch: * have you created a ticket for the failing ViewTest#testSSTablesInBounds() ? (I guess this is the old-functionality-test you mention that is failing?) * TrackerTest#testDropSSTables() seems to be failing due to the deleting task being run before asserting the sizes (reducing the totalDiskSpaceUsed) other than that, this looks good to me 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14553948#comment-14553948 ] Benedict commented on CASSANDRA-8568: - bq. TrackerTest#testDropSSTables() seems to be failing due to the deleting task being run before asserting the sizes (reducing the totalDiskSpaceUsed) That's a shame; I'd hoped that since that had to wait for a thread to spin up we'd be fine (it always passes on my box). I've introduced an artificial blocking mechanism, and have uploaded to cassci to vet. I've fixed the DataTracker references, and updated the LifecycleTransaction name. I will file a JIRA for testSSTablesInBounds shortly (this is indeed the new test of old behaviour that I was expecting to fail) 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14554156#comment-14554156 ] Marcus Eriksson commented on CASSANDRA-8568: Just small nit that only happens when running unit tests: {code} java.lang.NullPointerException: name cannot be null at javax.management.ObjectName.construct(ObjectName.java:421) ~[na:1.8.0_45] at javax.management.ObjectName.init(ObjectName.java:1382) ~[na:1.8.0_45] at org.apache.cassandra.db.ColumnFamilyStore.unregisterMBean(ColumnFamilyStore.java:448) ~[Cassandra/:na] at org.apache.cassandra.db.ColumnFamilyStore.invalidate(ColumnFamilyStore.java:419) ~[Cassandra/:na] at org.apache.cassandra.db.lifecycle.TrackerTest.testDropSSTables(TrackerTest.java:193) [Cassandra/:na] at org.apache.cassandra.db.lifecycle.TrackerTest.testDropSSTables(TrackerTest.java:177) [Cassandra/:na] ... {code} other than that, I'm +1 once cassci finishes the tests 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14554218#comment-14554218 ] Marcus Eriksson commented on CASSANDRA-8568: I ran the tests from within intellij and saw the NPE and figured it could be confusing/time wasting if someone sees it in the future and digs into why it occurs, so I'd say keep it suppressed. 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14554205#comment-14554205 ] Benedict commented on CASSANDRA-8568: - I've uploaded a minor update that suppresses it. Personally I had a slight preference to leave the production code unchanged here, since the warning message would be filtered out during an ant test run, but I'm pretty on the fence and it is minor, so you have the options there to choose from :) I'll then rebase again against trunk and run cassci on them all before merging to 2.2/trunk. Thanks for reviewing! 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14550461#comment-14550461 ] Benedict commented on CASSANDRA-8568: - I've uploaded two branches for cassci [for 2.2|https://github.com/belliottsmith/cassandra/tree/8568-2.2] and [trunk|https://github.com/belliottsmith/cassandra/tree/8568-trunk], but the merge was clean so nothing to worry about from a review perspective. The rebase was also nearly 100% clean (just a couple of methods that were introduced, that had to have a couple of arguments updated). I've fixed all of your nits bar 1: the naming. I'm not by any means wed to the naming I've used, but I think prefixing every class with DataTracker is a bit ugly. My intention was for the package to fully identify their domain, but I agree Transaction could perhaps be called LifecycleTransaction (or something) to make it clearer). We can certainly rollback Tracker to DataTracker, but since we now have a lifecycle.View (no longer DataTracker.View), unless we also prefix that (or move it back to an inner class - which I would be against: we have too many monolithic classes in the codebase) I'm not sure it improves clarity, and I'm personally in favour of reducing the number of characters where possible (especially since Data is a bit vacuous in a database). I don't mind you making the final call on the names since I don't have very strong feelings about them, but wanted to put across my rationale. Perhaps we can throw up a bikeshedding bat signal and get some other views? (This sentence is meant to do that, if you're watching) I've squashed all of the already reviewed commits. In the [2.2 branch|https://github.com/belliottsmith/cassandra/tree/8568-2.2] the new work is separated into commits: # addressing your clarification/nits concerns # making a minor tweak to Transactional so that cleanup can be post- or pre- abort; I realised that this was a potential mistake in the rebase onto that functionality, since we want to ensure the compacting set always contains the readers right up until the last moment. # a trivial TODO, introducing a UniqueIdentifier for SSTableReader, that permits our leak detection to kick in earlier around a lifecycle Transaction # new tests #* Since we're in a New World Order, I opted to introduce tests for every piece of code I modified, not just the brand new functionality, so this may be a bit of a slog to review, and I'm sorry about that :) #* One of the old functionality tests spotted an inconsistency in the usage of View.getOldestMemtable(), so I have removed this method and used a more suitable mechanism in the two callers #* There is a failing test covering old functionality that was too large to sneak in along with the test coverage, and I will file a follow-up bug to address it. It's not clear to me if it is at all serious or not, but it looks like it might be. 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.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545192#comment-14545192 ] Marcus Eriksson commented on CASSANDRA-8568: * the commit / finish relation in SSTRW is a bit confusing - sometimes we finish the SSTRW (in CompactionTask), and sometimes we don't (CompactionManager#antiCompactGroup). * in Tracker: Throwable apply(FunctionView, View function, Throwable accumulate) - catch Throwable instead of Exception? * Tests for Transaction.java - I know we test it indirectly via other tests, but having specific tests helps future refactorings etc * java.lang.NoClassDefFoundError: org/apache/mina/util/IdentityHashSet on startup - guess you forgot git add;ing a new .jar in lib/? * Fault injection tests: ** no way to run them? ** no documentation of the scripting language ** we probably need tests for the byteman script parsing/fault generation ** no comments in FaultInjectionTestRunner nits; * new dependency dependency groupId=org.slf4j artifactId=slf4j-log4j12 version=1.7.2/ - required by byteman? Should probably be 1.7.7 as thats the slf4j we use elsewhere * Unused buildIntervalTree in Tracker * Unused notifySSTablesChanged(CollectionSSTableReader removed, CollectionSSTableReader added, OperationType compactionType) in Tracker * Unused logger in Tracker (we should probably debug-log some stuff) * more comments in Helpers (esp filter_in and filter_out as it is not obvious from the names of the methods what they do) * underscores in method names in Helpers - not really consistent with either our code base or guavas * Why rename DataTracker to Tracker? People know what DataTracker is, feels like renaming is unnecessary here * Class called Transaction which only involves sstables/compaction, rename? (DataTrackerTransaction might be a bit long, but would tell us what it does) * Convert //-method-comments to /** */ ones in Helpers and Transaction 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545214#comment-14545214 ] Benedict commented on CASSANDRA-8568: - 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(FunctionView, 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545221#comment-14545221 ] Marcus Eriksson commented on CASSANDRA-8568: bq. I'll address your nits and some unit tests for Transaction itself shortly, if that all sounds good to you. it does 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14517305#comment-14517305 ] Benedict commented on CASSANDRA-8568: - [~krummas]: I've rebased against 8984-alt and force pushed. There are some failing unit tests but these are all broken against trunk, and appear to be due to embedded Cassandra and singletons. I'll rebase again once they're fixed, but since 8984-alt is agreed except for testing you should be good to begin review at your leisure. 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: 3.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14381637#comment-14381637 ] Marcus Eriksson commented on CASSANDRA-8568: [~benedict] could you rebase? And there seems to be some merge problems in build.xml 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: 3.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14381773#comment-14381773 ] Benedict commented on CASSANDRA-8568: - Yep, will do. I'm rebasing CASSANDRA-8984 onto trunk, and then will rebase this onto that, since it's likely that will be committed soon(ish). 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: 3.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14365266#comment-14365266 ] Jonathan Ellis commented on CASSANDRA-8568: --- SGTM. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14366487#comment-14366487 ] Benedict commented on CASSANDRA-8568: - Improvements to Byteman testing can be a follow up ticket, which I'll get to in a week or two. It's ready for review as stands, although I would prefer CASSANDRA-8984 take priority. 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: 3.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14366532#comment-14366532 ] Benedict commented on CASSANDRA-8568: - They can. They touch related code, and I will have to rebase this against the result of 8984 before commit which will result in a small follow up corroborative review, but the main body of work is independent. 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: 3.0 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14364352#comment-14364352 ] Benedict commented on CASSANDRA-8568: - What I will try to do for 2.1 is to backport the Byteman tests to just cover the set of failures we might encounter under normal operation. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363176#comment-14363176 ] Sylvain Lebresne commented on CASSANDRA-8568: - That's rather clearly not 2.1 material to me. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363184#comment-14363184 ] Benedict commented on CASSANDRA-8568: - Yep, I would agree with you 100% were it not for the recent problems. And I still lean in that direction, since this is a complex change. My concern is only if we haven't fully addressed the existing problems, because our testing as it stands doesn't seem able to elicit them. So anything that both improves testing and robustness in this area has a compelling reason for inclusion, if not a sufficient one. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363151#comment-14363151 ] Marcus Eriksson commented on CASSANDRA-8568: is this for 3.0 or 2.1? 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363092#comment-14363092 ] Benedict commented on CASSANDRA-8568: - This is up [here|https://github.com/belliottsmith/cassandra/tree/8568]. I'm working on integrating ByteMan tests into it, though, to ensure it always results in a safe application state regardless of other program behaviour. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363159#comment-14363159 ] Benedict commented on CASSANDRA-8568: - Very good question. I'm torn, personally, since this should be much more robust - especially once I get fault injection testing working. But it's definitely a non-trivial change. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363302#comment-14363302 ] Jonathan Ellis commented on CASSANDRA-8568: --- Can we get stability benefits in 2.1 by committing to 3.0 and backporting fixes as it exposes problems? 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363327#comment-14363327 ] Benedict commented on CASSANDRA-8568: - Doubtful. For one we only really see problems once deployed to the wild, so that won't likely help us until 3.0 is being widely used. But afaict the problems we're seeing are mostly down to either: # fiddliness of the current abstraction (lots of different methods that need to be interleaved in non-obvious ways), so this patch will either fix them and so give us no information, or have a different fix for the problem (and different set of problems, and in either case not help us safely reason about the old way); or # problematic rollback of changes - which really involves backporting this whole patch to address The current code has been predicated on the assumption there are no problems, and it's reached a level of complexity where that is an unsafe assumption, and it cannot safely handle when this assumption is broken. As a result of #2 we cannot safely fault injection test, because it cannot safely deal with arbitrary failures (at best it copes with system failures, but not programming failures - in fact the assertions dotted around the code at the moment ironically make it more brittle than less). 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14363960#comment-14363960 ] Benedict commented on CASSANDRA-8568: - I've force pushed an update to my repository that fixes a couple of minor bugs, and introduces a FaultInjectionTestRunner based on ByteMan. It is a work in progress, with the goal state being complete coverage of all possible interleavings of different pathways. Right now it is testing a majority of simple single failures that can occur during an online SSTableRewriter action. I intend to extend this to the complete set of single failures in the normal path (if a failure point can be hit multiple times, I intend also ensure each possible time is tried, to test all intermediate states), and also failures during recovery. My hope is we can roll the new FaultInjectionTestRunner out into a number of other places in the codebase as well. 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14267554#comment-14267554 ] Marcus Eriksson commented on CASSANDRA-8568: I linked CASSANDRA-8506 which I bet will be fixed here I also tried to put some lipstick on the pig in CASSANDRA-7852 but this sounds like the proper fix 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 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)
[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety
[ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14267011#comment-14267011 ] Jonathan Ellis commented on CASSANDRA-8568: --- ISTR [~krummas] also creating a ticket for this but I can't find it. Imagination? 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 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)