From 5cd00c5f5245bccc810ae40a69821071e8080319 Mon Sep 17 00:00:00 2001 From: Kristijonas Zalys Date: Thu, 30 Oct 2025 22:02:32 -0700 Subject: [PATCH 1/3] Use LWTs for all auto-repair history mutations --- CHANGES.txt | 1 + .../repair/autorepair/AutoRepairUtils.java | 14 +++++++------- .../service/AutoRepairServiceSetterTest.java | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b1290e1fe867..88e0efeebb64 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Use LWTs for all auto-repair history mutations (CASSANDRA-20996) * Add cqlsh autocompletion for the identity mapping feature (CASSANDRA-20021) * Add DDL Guardrail enabling administrators to disallow creation/modification of keyspaces with durable_writes = false (CASSANDRA-20913) * Optimize Counter, Meter and Histogram metrics using thread local counters (CASSANDRA-20250) diff --git a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java index d8c0c52f208e..09a677378383 100644 --- a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java +++ b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java @@ -133,10 +133,10 @@ public class AutoRepairUtils "SELECT * FROM %s.%s WHERE %s = ?", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String DEL_REPAIR_PRIORITY = String.format( - "DELETE %s[?] FROM %s.%s WHERE %s = ?", COL_REPAIR_PRIORITY, SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, + "DELETE %s[?] FROM %s.%s WHERE %s = ? IF EXISTS", COL_REPAIR_PRIORITY, SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String ADD_PRIORITY_HOST = String.format( - "UPDATE %s.%s SET %s = %s + ? WHERE %s = ?", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, + "UPDATE %s.%s SET %s = %s + ? WHERE %s = ? IF EXISTS", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_PRIORITY, COL_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String INSERT_NEW_REPAIR_HISTORY = String.format( @@ -150,27 +150,27 @@ public class AutoRepairUtils COL_DELETE_HOSTS, COL_DELETE_HOSTS_UPDATE_TIME, COL_REPAIR_TYPE, COL_HOST_ID); final static String DEL_AUTO_REPAIR_HISTORY = String.format( - "DELETE FROM %s.%s WHERE %s = ? AND %s = ?" + "DELETE FROM %s.%s WHERE %s = ? AND %s = ? IF EXISTS" , SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, COL_REPAIR_TYPE, COL_HOST_ID); final static String RECORD_START_REPAIR_HISTORY = String.format( - "UPDATE %s.%s SET %s= ?, repair_turn = ? WHERE %s = ? AND %s = ?" + "UPDATE %s.%s SET %s= ?, repair_turn = ? WHERE %s = ? AND %s = ? IF EXISTS" , SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, COL_REPAIR_START_TS, COL_REPAIR_TYPE, COL_HOST_ID); final static String RECORD_FINISH_REPAIR_HISTORY = String.format( - "UPDATE %s.%s SET %s= ?, %s=false WHERE %s = ? AND %s = ?" + "UPDATE %s.%s SET %s= ?, %s=false WHERE %s = ? AND %s = ? IF EXISTS" , SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, COL_REPAIR_FINISH_TS, COL_FORCE_REPAIR, COL_REPAIR_TYPE, COL_HOST_ID); final static String CLEAR_DELETE_HOSTS = String.format( - "UPDATE %s.%s SET %s= {} WHERE %s = ? AND %s = ?" + "UPDATE %s.%s SET %s= {} WHERE %s = ? AND %s = ? IF EXISTS" , SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, COL_DELETE_HOSTS, COL_REPAIR_TYPE, COL_HOST_ID); final static String SET_FORCE_REPAIR = String.format( - "UPDATE %s.%s SET %s=true WHERE %s = ? AND %s = ?" + "UPDATE %s.%s SET %s=true WHERE %s = ? AND %s = ? IF EXISTS" , SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, COL_FORCE_REPAIR, COL_REPAIR_TYPE, COL_HOST_ID); diff --git a/test/unit/org/apache/cassandra/service/AutoRepairServiceSetterTest.java b/test/unit/org/apache/cassandra/service/AutoRepairServiceSetterTest.java index db87e995f558..1ab2937b0735 100644 --- a/test/unit/org/apache/cassandra/service/AutoRepairServiceSetterTest.java +++ b/test/unit/org/apache/cassandra/service/AutoRepairServiceSetterTest.java @@ -132,6 +132,10 @@ public void prepare() QueryProcessor.executeInternal(String.format( "TRUNCATE %s.%s", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY)); + QueryProcessor.executeInternal(String.format( + "INSERT INTO %s.%s (host_id, repair_type) VALUES (%s, '%s')", + SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, + StorageService.instance.getHostIdForEndpoint(InetAddressAndPort.getLocalHost()), repairTypeStr)); } @Test From d4cd17c311edb4a450f959c54d490ac33eaad6e4 Mon Sep 17 00:00:00 2001 From: Kristijonas Zalys Date: Mon, 10 Nov 2025 16:55:21 -0800 Subject: [PATCH 2/3] Remove LWTs from priority queries --- .../apache/cassandra/repair/autorepair/AutoRepairUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java index 09a677378383..169bbb5cf923 100644 --- a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java +++ b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairUtils.java @@ -133,10 +133,10 @@ public class AutoRepairUtils "SELECT * FROM %s.%s WHERE %s = ?", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String DEL_REPAIR_PRIORITY = String.format( - "DELETE %s[?] FROM %s.%s WHERE %s = ? IF EXISTS", COL_REPAIR_PRIORITY, SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, + "DELETE %s[?] FROM %s.%s WHERE %s = ?", COL_REPAIR_PRIORITY, SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String ADD_PRIORITY_HOST = String.format( - "UPDATE %s.%s SET %s = %s + ? WHERE %s = ? IF EXISTS", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, + "UPDATE %s.%s SET %s = %s + ? WHERE %s = ?", SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_PRIORITY, COL_REPAIR_PRIORITY, COL_REPAIR_PRIORITY, COL_REPAIR_TYPE); final static String INSERT_NEW_REPAIR_HISTORY = String.format( From 8ed0cb252f39febb9f1ba818eede429caf41a8bf Mon Sep 17 00:00:00 2001 From: Kristijonas Zalys Date: Mon, 10 Nov 2025 17:08:41 -0800 Subject: [PATCH 3/3] Add test case to simulate race condition --- .../autorepair/AutoRepairUtilsTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairUtilsTest.java b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairUtilsTest.java index 57974f6c8c31..3af5c3654599 100644 --- a/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairUtilsTest.java +++ b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairUtilsTest.java @@ -538,4 +538,42 @@ public void testSkipSystemTraces() { assertFalse(AutoRepairUtils.shouldConsiderKeyspace(Keyspace.open(SchemaConstants.TRACE_KEYSPACE_NAME))); } + + @Test + public void testAutoRepairHistoryOutOfOrderDeleteRaceCondition() + { + // Setup: Create a node that will be deleted + UUID nodeToDelete = UUID.randomUUID(); + UUID votingNode = UUID.randomUUID(); + + // Insert an initial row for the node to be deleted + QueryProcessor.executeInternal(String.format( + "INSERT INTO %s.%s (repair_type, host_id, repair_start_ts, repair_finish_ts) VALUES ('%s', %s, 100, 200)", + SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, + repairType, nodeToDelete)); + + // Verify the row exists + UntypedResultSet beforeDelete = QueryProcessor.executeInternal(String.format( + "SELECT * FROM %s.%s WHERE repair_type = '%s' AND host_id = %s", + SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, + repairType, nodeToDelete)); + assertNotNull(beforeDelete); + assertEquals(1, beforeDelete.size()); + + // Simulate the race condition: + // 1. First, the delete is executed (this should create a tombstone) + AutoRepairUtils.deleteAutoRepairHistory(repairType, nodeToDelete); + + // 2. Then, a vote to delete arrives after the row has already been deleted + AutoRepairUtils.addHostIdToDeleteHosts(repairType, votingNode, nodeToDelete); + + // Verify that the row is still deleted despite the out-of-order vote + UntypedResultSet afterRace = QueryProcessor.executeInternal(String.format( + "SELECT * FROM %s.%s WHERE repair_type = '%s' AND host_id = %s", + SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, SystemDistributedKeyspace.AUTO_REPAIR_HISTORY, + repairType, nodeToDelete)); + assertNotNull(afterRace); + // The row should not exist - the delete should win despite the vote arriving later + assertEquals("Row should remain deleted despite out-of-order vote", 0, afterRace.size()); + } }