[ https://issues.apache.org/jira/browse/CASSANDRA-14935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16743256#comment-16743256 ]
Blake Eggleston commented on CASSANDRA-14935: --------------------------------------------- Looks functionally solid, I like how you collapsed the 2 acquisition attempts in {{PendingAntiCompaction}}. There are a few minor / stylistic things though: * There’s an unused import in {{AutoSavingCache}} * I think {{CancellationType}} can be removed from {{CompactionInfo}}. Anywhere it’s set to {{ALL}}, {{sstables.isEmpty()}} should always be true, making it redundant. * We shouldn't prompt users to check repair_admin in the exception message if there are conflicting anti-compactions. Those won’t be caused by hung sessions, but users starting multiple repairs at the same time. * I think we should use ImmutableSet inside CompactionInfo, but I don’t think we should make callers make the immutable copy. If we just accepted a Set and made an immutable copy in the ctor, the places we construct CompactionInfo would be a bit cleaner. * Nit: when quickly reading code, referring to the ActiveCompactionTracker instance as {{tracker}} clashes (in my head) with the cfs data tracker. Not a huge deal, but calling it something like {{compactions}} or {{activeCompactions}} would make it a bit clearer at a glance. > PendingAntiCompaction should be more judicious in the compactions it cancels > ---------------------------------------------------------------------------- > > Key: CASSANDRA-14935 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14935 > Project: Cassandra > Issue Type: Improvement > Reporter: Marcus Eriksson > Assignee: Marcus Eriksson > Priority: Major > Fix For: 4.0 > > > PendingAntiCompaction currently cancels all ongoing compactions to be able to > grab the sstables it requires - it should; > * avoid stopping ongoing anticompactions - for example, if we have an sstable > [0, 100] and are anticompacting it on [0, 50] - now if we start a new > incremental repair on [51, 100] we will (try to) stop the first one. Instead > we should fail the new incremental repair request. > * avoid stopping unrelated regular compactions - we should only stop regular > compactions for the sstables the new anticompaction needs > This requires us to keep track of which sstables are being compacted by a > given compaction id. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org