Fix issue when running cms reconfiguration with paxos repair disabled

Patch by marcuse; reviewed by Sam Tunnicliffe for CASSANDRA-20869

Author: krummas

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Fix issue when running cms reconfiguration with paxos repair disabled (CASSANDRA-20869)
  * Added additional parameter to JVM shutdown to allow for logs to be properly shutdown (CASSANDRA-20978)
  * Improve isGossipOnlyMember and location lookup performance (CASSANDRA-21039)
  * Refactor the way we check if a transformation is allowed to be committed during upgrades (CASSANDRA-21043)

src/java/org/apache/cassandra/service/ActiveRepairService.java

@@ -20,7 +20,6 @@
 import java.io.IOException;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -118,7 +117,6 @@
 import org.apache.cassandra.utils.concurrent.AsyncPromise;
 import org.apache.cassandra.utils.concurrent.Future;
 import org.apache.cassandra.utils.concurrent.FutureCombiner;
-import org.apache.cassandra.utils.concurrent.ImmediateFuture;
 
 import static com.google.common.collect.Iterables.concat;
 import static com.google.common.collect.Iterables.transform;
@@ -1190,13 +1188,13 @@ public List<Supplier<Future<?>>> repairPaxosForTopologyChangeAsync(String ksName
         if (!paxosRepairEnabled())
         {
             logger.warn("Not running paxos repair for topology change because paxos repair has been disabled");
-            return Arrays.asList(() -> ImmediateFuture.success(null));
+            return Collections.emptyList();
         }
 
         if (ranges.isEmpty())
         {
             logger.warn("Not running paxos repair for topology change because there are no ranges to repair");
-            return Arrays.asList(() -> ImmediateFuture.success(null));
+            return Collections.emptyList();
         }
         ClusterMetadata metadata = ClusterMetadata.current();
         List<TableMetadata> tables = Lists.newArrayList(metadata.schema.getKeyspaces().getNullable(ksName).tables);

src/java/org/apache/cassandra/tcm/sequences/ReconfigureCMS.java

@@ -30,6 +30,7 @@
 import java.util.function.Supplier;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -198,7 +199,7 @@ public SequenceState executeNext()
         {
             String message = "Some data streaming failed. Use nodetool to check CMS reconfiguration state and resume. " +
                              "For more, see `nodetool help cms reconfigure`.";
-            logger.warn(message);
+            logger.warn(message, t);
             return SequenceState.error(new RuntimeException(message));
         }
     }
@@ -344,10 +345,10 @@ static void repairPaxosForCMSTopologyChange()
         // overlapping quorums invariant holds.
 
         Retry retry = Retry.withNoTimeLimit(TCMMetrics.instance.repairPaxosTopologyRetries);
-        List<Supplier<Future<?>>> remaining = ActiveRepairService.instance()
-                                                                 .repairPaxosForTopologyChangeAsync(SchemaConstants.METADATA_KEYSPACE_NAME,
-                                                                                                    Collections.singletonList(entireRange),
-                                                                                                    "CMS reconfiguration");
+        List<Supplier<Future<?>>> remaining = Lists.newArrayList(ActiveRepairService.instance()
+                                                                                    .repairPaxosForTopologyChangeAsync(SchemaConstants.METADATA_KEYSPACE_NAME,
+                                                                                                                       Collections.singletonList(entireRange),
+                                                                                                                       "CMS reconfiguration"));
 
         while (true)
         {

test/distributed/org/apache/cassandra/distributed/test/log/ReconfigureCMSTest.java

@@ -308,6 +308,19 @@ public void cmsTopologyChangePaxosTest() throws Throwable
         }
     }
 
+    @Test
+    public void testReconfigurePaxosRepairDisabled() throws IOException
+    {
+        try (Cluster cluster = builder().withNodes(3)
+                                        .withConfig(c -> c.with(Feature.NETWORK)
+                                                          .set("paxos_repair_enabled", "false"))
+                                        .withoutVNodes()
+                                        .start())
+        {
+            cluster.get(1).nodetoolResult("cms", "reconfigure", "3").asserts().success();
+        }
+    }
+
     private PaxosRepairHistory paxosRepairHistory(IInvokableInstance instance)
     {
         Object[][] rows = instance.executeInternal("select points from system.paxos_repair_history " +