This is an automated email from the ASF dual-hosted git repository.

jmckenzie pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 51944c5e68 Make sure preview repairs don't optimise streams unless 
configured to
51944c5e68 is described below

commit 51944c5e68bfcee0c2c8e2aeb6b572eae0167965
Author: Josh McKenzie <jmcken...@apache.org>
AuthorDate: Wed Sep 7 13:12:22 2022 -0400

    Make sure preview repairs don't optimise streams unless configured to
    
    Patch by Chris Lohfink; reviewed by Josh McKenzie and Marcus Eriksson for 
CASSANDRA-17865
    
    Co-authored-by: Chris Lohfink <clohf...@apple.com>
    Co-authored-by: Josh McKenzie <jmcken...@apache.org>
---
 CHANGES.txt                                        |  1 +
 .../cassandra/repair/messages/RepairOption.java    | 20 +++----
 .../repair/messages/RepairOptionTest.java          | 67 ++++++++++++++++++++--
 3 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index e4566fa723..9bf2063066 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.2
+ * Make sure preview repairs don't optimise streams unless configured to 
(CASSANDRA-17865)
  * Optionally avoid hint transfer during decommission (CASSANDRA-17808)
  * Make disabling auto snapshot on selected tables possible (CASSANDRA-10383)
  * Introduce compaction priorities to prevent upgrade compaction inability to 
finish (CASSANDRA-17851)
diff --git a/src/java/org/apache/cassandra/repair/messages/RepairOption.java 
b/src/java/org/apache/cassandra/repair/messages/RepairOption.java
index 6bb7fdb61f..f0508a3e42 100644
--- a/src/java/org/apache/cassandra/repair/messages/RepairOption.java
+++ b/src/java/org/apache/cassandra/repair/messages/RepairOption.java
@@ -395,22 +395,20 @@ public class RepairOption
 
     public boolean optimiseStreams()
     {
-        if(optimiseStreams)
-            return true;
-
-        if (isPullRepair() || isForcedRepair())
+        if (isPullRepair())
             return false;
 
-        if (isIncremental() && 
DatabaseDescriptor.autoOptimiseIncRepairStreams())
-            return true;
-
-        if (isPreview() && 
DatabaseDescriptor.autoOptimisePreviewRepairStreams())
+        if (isPreview())
+        {
+            if (DatabaseDescriptor.autoOptimisePreviewRepairStreams())
+                return true;
+        }
+        else if (isIncremental() && 
DatabaseDescriptor.autoOptimiseIncRepairStreams())
             return true;
-
-        if (!isIncremental() && 
DatabaseDescriptor.autoOptimiseFullRepairStreams())
+        else if (!isIncremental() && 
DatabaseDescriptor.autoOptimiseFullRepairStreams())
             return true;
 
-        return false;
+        return optimiseStreams;
     }
 
     public boolean ignoreUnreplicatedKeyspaces()
diff --git 
a/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java 
b/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java
index a6ca084c28..0483fcf15c 100644
--- a/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java
+++ b/test/unit/org/apache/cassandra/repair/messages/RepairOptionTest.java
@@ -23,7 +23,6 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
-import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.cassandra.config.DatabaseDescriptor;
@@ -32,6 +31,7 @@ import org.apache.cassandra.dht.Murmur3Partitioner;
 import org.apache.cassandra.dht.Range;
 import org.apache.cassandra.dht.Token;
 import org.apache.cassandra.repair.RepairParallelism;
+import org.apache.cassandra.streaming.PreviewKind;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -149,17 +149,76 @@ public class RepairOptionTest
 
         // default value
         option = RepairOption.parse(options, Murmur3Partitioner.instance);
-        Assert.assertFalse(option.isForcedRepair());
+        assertFalse(option.isForcedRepair());
 
         // explicit true
         options.put(RepairOption.FORCE_REPAIR_KEY, "true");
         option = RepairOption.parse(options, Murmur3Partitioner.instance);
-        Assert.assertTrue(option.isForcedRepair());
+        assertTrue(option.isForcedRepair());
 
         // explicit false
         options.put(RepairOption.FORCE_REPAIR_KEY, "false");
         option = RepairOption.parse(options, Murmur3Partitioner.instance);
-        Assert.assertFalse(option.isForcedRepair());
+        assertFalse(option.isForcedRepair());
+    }
+
+    @Test
+    public void testOptimiseStreams()
+    {
+        boolean optFull = DatabaseDescriptor.autoOptimiseFullRepairStreams();
+        boolean optInc = DatabaseDescriptor.autoOptimiseIncRepairStreams();
+        boolean optPreview = 
DatabaseDescriptor.autoOptimisePreviewRepairStreams();
+        try
+        {
+            for (PreviewKind previewKind : PreviewKind.values())
+                for (boolean inc : new boolean[] {true, false})
+                    assertOptimise(previewKind, inc);
+        }
+        finally
+        {
+            setOptimise(optFull, optInc, optPreview);
+        }
+    }
+
+    private void assertHelper(Map<String, String> options, boolean full, 
boolean inc, boolean preview, boolean expected)
+    {
+        setOptimise(full, inc, preview);
+        assertEquals(expected, RepairOption.parse(options, 
Murmur3Partitioner.instance).optimiseStreams());
+    }
+
+    private void setOptimise(boolean full, boolean inc, boolean preview)
+    {
+        DatabaseDescriptor.setAutoOptimiseFullRepairStreams(full);
+        DatabaseDescriptor.setAutoOptimiseIncRepairStreams(inc);
+        DatabaseDescriptor.setAutoOptimisePreviewRepairStreams(preview);
+    }
+
+    private void assertOptimise(PreviewKind previewKind, boolean incremental)
+    {
+        Map<String, String> options = new HashMap<>();
+        options.put(RepairOption.PREVIEW, previewKind.toString());
+        options.put(RepairOption.INCREMENTAL_KEY, 
Boolean.toString(incremental));
+        for (boolean a : new boolean[]{ true, false })
+        {
+            for (boolean b : new boolean[]{ true, false })
+            {
+                if (previewKind.isPreview())
+                {
+                    assertHelper(options, a, b, true, true);
+                    assertHelper(options, a, b, false, false);
+                }
+                else if (incremental)
+                {
+                    assertHelper(options, a, true, b, true);
+                    assertHelper(options, a, false, b, false);
+                }
+                else
+                {
+                    assertHelper(options, true, a, b, true);
+                    assertHelper(options, false, a, b, false);
+                }
+            }
+        }
     }
 
     private void 
assertParseThrowsIllegalArgumentExceptionWithMessage(Map<String, String> 
optionsToParse, String expectedErrorMessage)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to