jaydeepkumar1984 commented on code in PR #4304:
URL: https://github.com/apache/cassandra/pull/4304#discussion_r2271632169


##########
src/java/org/apache/cassandra/config/Config.java:
##########
@@ -374,6 +374,9 @@ public MemtableOptions()
     // if you want to disable this feature (the recommendation is not to, but 
if you want to disable it for whatever reason) then set the ratio to 0.0
     public volatile double incremental_repair_disk_headroom_reject_ratio = 0.2;
 
+    // by default repair is disabled if there are mixed major versions 
detected, but you can enable it using this flag
+    public volatile boolean mixed_major_version_repair_enabled = false;

Review Comment:
   Can you add the configuration inside AutoRepairConfig.java?



##########
src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java:
##########
@@ -165,6 +166,11 @@ public void repair(AutoRepairConfig.RepairType repairType)
             logger.debug("Auto-repair is disabled for repair type {}", 
repairType);
             return;
         }
+        if (!DatabaseDescriptor.getMixedMajorVersionRepairEnabled() &&
+            Gossiper.instance.hasMultipleLiveMajorVersions()) {
+            logger.info("Auto-repair is disabled when nodes in the cluster 
have different major versions");

Review Comment:
   Please add a metric



##########
test/distributed/org/apache/cassandra/distributed/test/GossipTest.java:
##########
@@ -285,4 +288,40 @@ public void testQuarantine() throws IOException
             assertTrue(removed);
         }
     }
+
+    @Test
+    public void testMultipleMajorVersionsWithMultipleMajorVersions() throws 
Exception
+    {
+        try (Cluster cluster = builder()
+                               .withNodes(2)
+                               .withConfig(config -> config.with(GOSSIP))
+                               .start())
+        {
+            InetSocketAddress peer1 = cluster.get(1).broadcastAddress();
+            String version1 = "4.0.1920";

Review Comment:
   Can you add a test case by setting different minor versions explicitly, say 
`4.0.1920`, `4.1.2`?



##########
src/java/org/apache/cassandra/gms/Gossiper.java:
##########
@@ -2034,6 +2034,23 @@ private boolean 
nodesAgreeOnSchema(Collection<InetAddressAndPort> nodes)
         return true;
     }
 
+    public boolean hasMultipleLiveMajorVersions()

Review Comment:
   Can we extend the existing `AutoRepairUtils.java::getHostIdsInCurrentRing`, 
which already goes through all the nodes in a cluster?
   
   
https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java#L429



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to