[
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
