You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ma...@apache.org on 2015/11/17 10:19:02 UTC

[3/4] cassandra git commit: Don't do anticompaction after subrange repair

Don't do anticompaction after subrange repair

Patch by Ariel Weisberg; reviewed by marcuse for CASSANDRA-10422


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/99b82dbb
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/99b82dbb
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/99b82dbb

Branch: refs/heads/cassandra-3.0
Commit: 99b82dbb43277035562e7b82bb9bdebd84510e96
Parents: d434a33
Author: Ariel Weisberg <ar...@datastax.com>
Authored: Tue Nov 10 13:08:05 2015 -0500
Committer: Marcus Eriksson <ma...@apache.org>
Committed: Tue Nov 17 10:07:59 2015 +0100

----------------------------------------------------------------------
 CHANGES.txt                                      |  1 +
 .../cassandra/repair/messages/RepairOption.java  | 19 ++++++++++++++-----
 .../cassandra/service/ActiveRepairService.java   |  2 ++
 .../apache/cassandra/service/StorageService.java |  9 +++++----
 .../repair/messages/RepairOptionTest.java        | 10 +++++++---
 5 files changed, 29 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/99b82dbb/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 489a76d..f5d3416 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)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/99b82dbb/src/java/org/apache/cassandra/repair/messages/RepairOption.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/repair/messages/RepairOption.java b/src/java/org/apache/cassandra/repair/messages/RepairOption.java
index 1780b6b..d50a2ed 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<String> columnFamilies = new HashSet<>();
     private final Collection<String> dataCenters = new HashSet<>();
     private final Collection<String> hosts = new HashSet<>();
     private final Collection<Range<Token>> ranges = new HashSet<>();
 
-    public RepairOption(RepairParallelism parallelism, boolean primaryRange, boolean incremental, boolean trace, int jobThreads, Collection<Range<Token>> ranges)
+    public RepairOption(RepairParallelism parallelism, boolean primaryRange, boolean incremental, boolean trace, int jobThreads, Collection<Range<Token>> 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()
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/99b82dbb/src/java/org/apache/cassandra/service/ActiveRepairService.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/ActiveRepairService.java b/src/java/org/apache/cassandra/service/ActiveRepairService.java
index a6389ea..0cb4252 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");

http://git-wip-us.apache.org/repos/asf/cassandra/blob/99b82dbb/src/java/org/apache/cassandra/service/StorageService.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index b5ce38b..80672dd 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.<Range<Token>>emptyList());
+        RepairOption options = new RepairOption(parallelism, primaryRange, !fullRepair, false, 1, Collections.<Range<Token>>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<Range<Token>> 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)
         {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/99b82dbb/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java b/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java
index 3257a10..cc6f46a 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());
     }
 }