From a8e8a67306c0b26b8fe9c74a1fb00bacfa224cf7 Mon Sep 17 00:00:00 2001 From: Ariel Weisberg Date: Thu, 29 Oct 2015 12:36:32 -0400 Subject: [PATCH 1/2] Reject incremental repair requests combined with subrange repair Patch by Ariel Weisberg; reviewed by marcuse for CASSANDRA-10422 --- CHANGES.txt | 1 + src/java/org/apache/cassandra/service/StorageService.java | 3 +++ .../apache/cassandra/service/StorageServiceServerTest.java | 6 ++++++ 3 files changed, 10 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 2eeda94721..b6b394acf6 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.12 + * Reject incremental repair with subrange repair (CASSANDRA-10422) * Add a nodetool command to refresh size_estimates (CASSANDRA-9579) * Shutdown compaction in drain to prevent leak (CASSANDRA-10079) * Invalidate cache after stream receive task is completed (CASSANDRA-10341) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 665ce3a531..03c1960bd4 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -2830,6 +2830,9 @@ public class StorageService extends NotificationBroadcasterSupport implements IE public int forceRepairRangeAsync(String beginToken, String endToken, String keyspaceName, boolean isSequential, boolean isLocal, boolean fullRepair, String... columnFamilies) { + if (!fullRepair) + throw new IllegalArgumentException("Incremental repair can't be requested with subrange repair because " + + "each subrange repair would generate an anti-compacted table"); Collection> repairingRange = createRepairRangeFrom(beginToken, endToken); logger.info("starting user-requested repair of range {} for keyspace {} and column families {}", diff --git a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java index dd25b35641..564239b23b 100644 --- a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java +++ b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java @@ -508,4 +508,10 @@ public class StorageServiceServerTest repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "2000"); assert repairRangeFrom.size() == 0; } + + @Test(expected=IllegalArgumentException.class) + public void testIncrementalRepairWithSubrangesThrows() throws Exception + { + StorageService.instance.forceRepairRangeAsync("", "", "", true, true, false, ""); + } } From 99b82dbb43277035562e7b82bb9bdebd84510e96 Mon Sep 17 00:00:00 2001 From: Ariel Weisberg Date: Tue, 10 Nov 2015 13:08:05 -0500 Subject: [PATCH 2/2] Don't do anticompaction after subrange repair Patch by Ariel Weisberg; reviewed by marcuse for CASSANDRA-10422 --- CHANGES.txt | 1 + .../repair/messages/RepairOption.java | 19 ++++++++++++++----- .../service/ActiveRepairService.java | 2 ++ .../cassandra/service/StorageService.java | 9 +++++---- .../repair/messages/RepairOptionTest.java | 10 +++++++--- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 489a76d14a..f5d3416f27 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.2.4 + * Don't do anticompaction after subrange repair (CASSANDRA-10422) * Fix SimpleDateType type compatibility (CASSANDRA-10027) * (Hadoop) fix splits calculation (CASSANDRA-10640) * (Hadoop) ensure that Cluster instances are always closed (CASSANDRA-10058) diff --git a/src/java/org/apache/cassandra/repair/messages/RepairOption.java b/src/java/org/apache/cassandra/repair/messages/RepairOption.java index 1780b6b922..d50a2edf66 100644 --- a/src/java/org/apache/cassandra/repair/messages/RepairOption.java +++ b/src/java/org/apache/cassandra/repair/messages/RepairOption.java @@ -145,8 +145,9 @@ public class RepairOption if (rangesStr != null) { if (incremental) - throw new IllegalArgumentException("Incremental repair can't be requested with subrange repair " + - "because each subrange repair would generate an anti-compacted table"); + logger.warn("Incremental repair can't be requested with subrange repair " + + "because each subrange repair would generate an anti-compacted table. " + + "The repair will occur but without anti-compaction."); StringTokenizer tokenizer = new StringTokenizer(rangesStr, ","); while (tokenizer.hasMoreTokens()) { @@ -161,7 +162,7 @@ public class RepairOption } } - RepairOption option = new RepairOption(parallelism, primaryRange, incremental, trace, jobThreads, ranges); + RepairOption option = new RepairOption(parallelism, primaryRange, incremental, trace, jobThreads, ranges, !ranges.isEmpty()); // data centers String dataCentersStr = options.get(DATACENTERS_KEY); @@ -220,13 +221,14 @@ public class RepairOption private final boolean incremental; private final boolean trace; private final int jobThreads; + private final boolean isSubrangeRepair; private final Collection columnFamilies = new HashSet<>(); private final Collection dataCenters = new HashSet<>(); private final Collection hosts = new HashSet<>(); private final Collection> ranges = new HashSet<>(); - public RepairOption(RepairParallelism parallelism, boolean primaryRange, boolean incremental, boolean trace, int jobThreads, Collection> ranges) + public RepairOption(RepairParallelism parallelism, boolean primaryRange, boolean incremental, boolean trace, int jobThreads, Collection> ranges, boolean isSubrangeRepair) { if (FBUtilities.isWindows() && (DatabaseDescriptor.getDiskAccessMode() != Config.DiskAccessMode.standard || DatabaseDescriptor.getIndexAccessMode() != Config.DiskAccessMode.standard) && @@ -243,6 +245,7 @@ public class RepairOption this.trace = trace; this.jobThreads = jobThreads; this.ranges.addAll(ranges); + this.isSubrangeRepair = isSubrangeRepair; } public RepairParallelism getParallelism() @@ -292,8 +295,14 @@ public class RepairOption public boolean isGlobal() { - return dataCenters.isEmpty() && hosts.isEmpty(); + return dataCenters.isEmpty() && hosts.isEmpty() && !isSubrangeRepair(); } + + public boolean isSubrangeRepair() + { + return isSubrangeRepair; + } + @Override public String toString() { diff --git a/src/java/org/apache/cassandra/service/ActiveRepairService.java b/src/java/org/apache/cassandra/service/ActiveRepairService.java index a6389ead0d..0cb425253b 100644 --- a/src/java/org/apache/cassandra/service/ActiveRepairService.java +++ b/src/java/org/apache/cassandra/service/ActiveRepairService.java @@ -353,6 +353,8 @@ public class ActiveRepairService { assert parentRepairSession != null; ParentRepairSession prs = getParentRepairSession(parentRepairSession); + //A repair will be marked as not global if it is a subrange repair to avoid many small anti-compactions + //in addition to other scenarios such as repairs not involving all DCs or hosts if (!prs.isGlobal) { logger.info("Not a global repair, will not do anticompaction"); diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index b5ce38b379..80672dd018 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -2882,7 +2882,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE parallelism = RepairParallelism.PARALLEL; } - RepairOption options = new RepairOption(parallelism, primaryRange, !fullRepair, false, 1, Collections.>emptyList()); + RepairOption options = new RepairOption(parallelism, primaryRange, !fullRepair, false, 1, Collections.>emptyList(), false); if (dataCenters != null) { options.getDataCenters().addAll(dataCenters); @@ -2966,11 +2966,12 @@ public class StorageService extends NotificationBroadcasterSupport implements IE } if (!fullRepair) - throw new IllegalArgumentException("Incremental repair can't be requested with subrange repair " + - "because each subrange repair would generate an anti-compacted table"); + logger.warn("Incremental repair can't be requested with subrange repair " + + "because each subrange repair would generate an anti-compacted table. " + + "The repair will occur but without anti-compaction."); Collection> repairingRange = createRepairRangeFrom(beginToken, endToken); - RepairOption options = new RepairOption(parallelism, false, !fullRepair, false, 1, repairingRange); + RepairOption options = new RepairOption(parallelism, false, !fullRepair, false, 1, repairingRange, true); options.getDataCenters().addAll(dataCenters); if (hosts != null) { diff --git a/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java b/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java index 3257a10ce3..cc6f46a8fc 100644 --- a/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java +++ b/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java @@ -96,10 +96,14 @@ public class RepairOptionTest assertEquals(expectedHosts, option.getHosts()); } - @Test(expected=IllegalArgumentException.class) - public void testIncrementalRepairWithSubrangesThrows() throws Exception + @Test + public void testIncrementalRepairWithSubrangesIsNotGlobal() throws Exception { - RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, ""), + RepairOption ro = RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, "42:42"), Murmur3Partitioner.instance); + assertFalse(ro.isGlobal()); + ro = RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, ""), + Murmur3Partitioner.instance); + assertTrue(ro.isGlobal()); } }