You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "kzalys (via GitHub)" <gi...@apache.org> on 2025/10/31 05:02:57 UTC
[PR] Use LWTs for all auto-repair history mutations [cassandra]
kzalys opened a new pull request, #4456:
URL: https://github.com/apache/cassandra/pull/4456
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20996 Use LWTs for all auto-repair history mutations [cassandra]
Posted by "kzalys (via GitHub)" <gi...@apache.org>.
kzalys commented on code in PR #4456:
URL: https://github.com/apache/cassandra/pull/4456#discussion_r2590725371
##########
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()
Review Comment:
Also, updated the description
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20996 Use LWTs for all auto-repair history mutations [cassandra]
Posted by "kzalys (via GitHub)" <gi...@apache.org>.
kzalys commented on PR #4456:
URL: https://github.com/apache/cassandra/pull/4456#issuecomment-3514572181
@jaydeepkumar1984 can I have a review on this please?
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20996 Use LWTs for all auto-repair history mutations [cassandra]
Posted by "jaydeepkumar1984 (via GitHub)" <gi...@apache.org>.
jaydeepkumar1984 commented on code in PR #4456:
URL: https://github.com/apache/cassandra/pull/4456#discussion_r2516145970
##########
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);
+
Review Comment:
Also clear deleted hosts as part of the late event
```
AutoRepairUtils.clearDeleteHosts(repairType, nodeToDelete);
```
##########
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()
Review Comment:
This test passes even without any of the changes because _ADD_HOST_ID_TO_DELETE_HOSTS_ already had _IF EXISTS_, however, the changes in this PR are necessary.
Please clarify in the description that the PR includes the test case and a certain cases we missed earlier.
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20996 Use LWTs for all auto-repair history mutations [cassandra]
Posted by "kzalys (via GitHub)" <gi...@apache.org>.
kzalys commented on code in PR #4456:
URL: https://github.com/apache/cassandra/pull/4456#discussion_r2590698361
##########
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()
Review Comment:
Good point, I've extended this test to include cases that make this test fail without the changes in this PR.
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org