[jira] [Commented] (CASSANDRA-8568) Impose new API on data tracker modifications that makes correct usage obvious and imposes safety

["Marcus Eriksson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-8568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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(Function<View, 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(Collection<SSTableReader> removed, Collection<SSTableReader> 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)