Repository: cassandra Updated Branches: refs/heads/cassandra-3.7 3b72c4202 -> f40c632e4 refs/heads/trunk e14e7310c -> 61c988e51
Fix race in CompactionStrategyManager's pause/resume Patch by Alex Petrov; reviewed by yukim for CASSANDRA-11922 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f40c632e Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f40c632e Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f40c632e Branch: refs/heads/cassandra-3.7 Commit: f40c632e42e7abc38b28bdbb5b729294f8c49fbd Parents: 3b72c42 Author: Alex Petrov <[email protected]> Authored: Mon May 30 14:10:18 2016 +0200 Committer: Yuki Morishita <[email protected]> Committed: Tue May 31 15:52:54 2016 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../compaction/CompactionStrategyManager.java | 40 +++++++++++++++----- .../cassandra/db/compaction/CompactionTask.java | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f40c632e/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index d081587..d7b3dd4 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.7 + * Fix race in CompactionStrategyManager's pause/resume (CASSANDRA-11922) Merged from 3.0: * Avoid referencing DatabaseDescriptor in AbstractType (CASSANDRA-11912) * Don't use static dataDirectories field in Directories instances (CASSANDRA-11647) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f40c632e/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java index 125f6f3..fbb25a3 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java @@ -24,7 +24,6 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; import java.util.stream.Stream; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Iterables; import org.apache.cassandra.index.Index; import com.google.common.primitives.Ints; @@ -67,7 +66,7 @@ public class CompactionStrategyManager implements INotificationConsumer private final List<AbstractCompactionStrategy> repaired = new ArrayList<>(); private final List<AbstractCompactionStrategy> unrepaired = new ArrayList<>(); private volatile boolean enabled = true; - public volatile boolean isActive = true; + private volatile boolean isActive = true; private volatile CompactionParams params; private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final ReentrantReadWriteLock.ReadLock readLock = lock.readLock(); @@ -103,14 +102,15 @@ public class CompactionStrategyManager implements INotificationConsumer */ public AbstractCompactionTask getNextBackgroundTask(int gcBefore) { - if (!isEnabled()) - return null; - - maybeReload(cfs.metadata); - List<AbstractCompactionStrategy> strategies = new ArrayList<>(); readLock.lock(); try { + if (!isEnabled()) + return null; + + maybeReload(cfs.metadata); + List<AbstractCompactionStrategy> strategies = new ArrayList<>(); + strategies.addAll(repaired); strategies.addAll(unrepaired); Collections.sort(strategies, (o1, o2) -> Ints.compare(o2.getEstimatedRemainingTasks(), o1.getEstimatedRemainingTasks())); @@ -133,9 +133,22 @@ public class CompactionStrategyManager implements INotificationConsumer return enabled && isActive; } + public boolean isActive() + { + return isActive; + } + public void resume() { - isActive = true; + writeLock.lock(); + try + { + isActive = true; + } + finally + { + writeLock.unlock(); + } } /** @@ -145,7 +158,16 @@ public class CompactionStrategyManager implements INotificationConsumer */ public void pause() { - isActive = false; + writeLock.lock(); + try + { + isActive = false; + } + finally + { + writeLock.unlock(); + } + } private void startup() http://git-wip-us.apache.org/repos/asf/cassandra/blob/f40c632e/src/java/org/apache/cassandra/db/compaction/CompactionTask.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java index 5df91fd..b3a94bb 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java @@ -173,7 +173,7 @@ public class CompactionTask extends AbstractCompactionTask collector.beginCompaction(ci); long lastCheckObsoletion = start; - if (!controller.cfs.getCompactionStrategyManager().isActive) + if (!controller.cfs.getCompactionStrategyManager().isActive()) throw new CompactionInterruptedException(ci.getCompactionInfo()); try (CompactionAwareWriter writer = getCompactionAwareWriter(cfs, getDirectories(), transaction, actuallyCompact))
