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