[
https://issues.apache.org/jira/browse/CASSANDRA-3430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13592356#comment-13592356
]
Sylvain Lebresne commented on CASSANDRA-3430:
---------------------------------------------
Sounds reasonable on principle. A few remarks/comments however:
* I believe the reason the assertion in runWithCompactionsDisabled fails in
CompactionsTest is because when a strategy is paused, compaction can still be
submitted, they will just exit right away in CompactionTask. But sstables are
marked compacting before that happens. In other words, pausing don't guaranteed
we won't mark sstable compacting, just that they won't really compact (so they
will be marked and unmark almost right away). So maybe we could move the
isActive check earlier (this would have my preference, but I'm not sure where
without dublicating the check), or we should remove the assertion .
* During truncate, we only stop compactions on the main CF. We should also stop
it for indexes too.
* We used to ensure that Scrub, SSTableRewrite and Cleanup were running on
*all* sstable (at the time it ran). This is no longer the case, being compacted
sstable will now be excluded. Is that on purpose? It's clearly fine for
SSTableRewrite given it's use case. Cleanup is more problematic, though if we
get read of manual cleanup for 2.0, that's fixes that :). Less sure about
scrub. Scrub can detect corruption that normal compaction wouldn't see, so if
scrub skips being compacted sstable, it's harder to ensure all sstables have
been scrubbed.
* We still get the truncateAt outside of the "protected" block. Meaning, we
could still have a memtable flushed and compacted with old sstable between the
grab of truncateAt and when we interrupt compaction (sure, we've make that even
more unlickely but not "impossible"). But I think we can more truncateAt
inside the truncateRunnable now to fix that once and for all.
* This makes CFS.truncate() blocking, and thus having it return a Future is
misleading. We should probably rename it truncateBlocking and have it return
void. Or we keep it asynchronous by pushing the runWithCompactionsDisabled on
some executor.
* We might want to move the 'collector.finishCompaction(ci)' in CompactionTask
up in it's finally block. If sound like an IO error could let a task "in
compaction" which would then block the wait in runWithCompactionsDisabled.
* Concerning that wait in runWithCompactionsDisabled, wouldn't it make sense to
move it inside interruptCompactionFor?
* One slightly changed tradeoff is that a major compaction may kill almost done
compactions. Not a big deal really (a good idea in fact, at least for
truncate), but may be worth an entry in the NEWS file.
* The truncate comment is CFS is not up to date (mentions
CompactionManager.submitTruncate).
> Break Big Compaction Lock apart
> -------------------------------
>
> Key: CASSANDRA-3430
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3430
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jonathan Ellis
> Assignee: Jonathan Ellis
> Priority: Minor
> Labels: compaction
> Fix For: 2.0
>
> Attachments: 3430-1.0.txt, 3430-1.1.txt, 3430-v2.txt, 3430-v3.txt
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
