From c7de33c597d4aa8a1f082974bb8fd1900e9f15d2 Mon Sep 17 00:00:00 2001 From: Benedict Elliott Smith Date: Mon, 15 Sep 2025 21:34:15 +0100 Subject: [PATCH] Strict Serializability Verifier may incorrectly handle blind writes when witnessed before the writing operation ACKs patch by Benedict; reviewed by Alex Petrov for CASSANDRA-20905 --- .gitmodules | 4 ++-- modules/accord | 2 +- .../apache/cassandra/simulator/paxos/HistoryValidator.java | 2 +- .../cassandra/simulator/paxos/LinearizabilityValidator.java | 2 +- .../cassandra/simulator/paxos/LoggingHistoryValidator.java | 4 ++-- .../simulator/paxos/PairOfSequencesAccordSimulation.java | 2 +- .../simulator/paxos/StrictSerializabilityValidator.java | 4 ++-- .../cassandra/simulator/paxos/HistoryValidatorTest.java | 4 ++-- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.gitmodules b/.gitmodules index 616dacf610..a0aed84702 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "modules/accord"] path = modules/accord - url = https://github.com/apache/cassandra-accord.git - branch = trunk + url = https://github.com/belliottsmith/cassandra-accord.git + branch = 20905-ssverifier diff --git a/modules/accord b/modules/accord index 9c7f856c3e..ff4119fa45 160000 --- a/modules/accord +++ b/modules/accord @@ -1 +1 @@ -Subproject commit 9c7f856c3e1f10c6f985495702edb844cfee2b80 +Subproject commit ff4119fa45ad9700cfa94cd50b5cd4af9dd0b7a4 diff --git a/test/simulator/main/org/apache/cassandra/simulator/paxos/HistoryValidator.java b/test/simulator/main/org/apache/cassandra/simulator/paxos/HistoryValidator.java index 282b16d3b1..cae26323cd 100644 --- a/test/simulator/main/org/apache/cassandra/simulator/paxos/HistoryValidator.java +++ b/test/simulator/main/org/apache/cassandra/simulator/paxos/HistoryValidator.java @@ -22,7 +22,7 @@ import javax.annotation.Nullable; public interface HistoryValidator { - Checker witness(int start, int end); + Checker witness(Object witnessedBy, int start, int end); void print(@Nullable Integer pk); diff --git a/test/simulator/main/org/apache/cassandra/simulator/paxos/LinearizabilityValidator.java b/test/simulator/main/org/apache/cassandra/simulator/paxos/LinearizabilityValidator.java index 67c95a7378..47a88aec89 100644 --- a/test/simulator/main/org/apache/cassandra/simulator/paxos/LinearizabilityValidator.java +++ b/test/simulator/main/org/apache/cassandra/simulator/paxos/LinearizabilityValidator.java @@ -37,7 +37,7 @@ public class LinearizabilityValidator implements HistoryValidator } @Override - public Checker witness(int start, int end) + public Checker witness(Object witnessedBy, int start, int end) { return new Checker() { diff --git a/test/simulator/main/org/apache/cassandra/simulator/paxos/LoggingHistoryValidator.java b/test/simulator/main/org/apache/cassandra/simulator/paxos/LoggingHistoryValidator.java index b39c3111ea..f51cbe3b00 100644 --- a/test/simulator/main/org/apache/cassandra/simulator/paxos/LoggingHistoryValidator.java +++ b/test/simulator/main/org/apache/cassandra/simulator/paxos/LoggingHistoryValidator.java @@ -35,11 +35,11 @@ public class LoggingHistoryValidator implements HistoryValidator } @Override - public Checker witness(int start, int end) + public Checker witness(Object witnessedBy, int start, int end) { StringBuilder sb = new StringBuilder(); sb.append("Witness(start=").append(start).append(", end=").append(end).append(")\n"); - Checker sub = delegate.witness(start, end); + Checker sub = delegate.witness(witnessedBy, start, end); return new Checker() { @Override diff --git a/test/simulator/main/org/apache/cassandra/simulator/paxos/PairOfSequencesAccordSimulation.java b/test/simulator/main/org/apache/cassandra/simulator/paxos/PairOfSequencesAccordSimulation.java index 4abc65aaf7..35deb18744 100644 --- a/test/simulator/main/org/apache/cassandra/simulator/paxos/PairOfSequencesAccordSimulation.java +++ b/test/simulator/main/org/apache/cassandra/simulator/paxos/PairOfSequencesAccordSimulation.java @@ -203,7 +203,7 @@ public class PairOfSequencesAccordSimulation extends AbstractPairOfSequencesPaxo IntHashSet seen = new IntHashSet(); //TODO if there isn't a value then we get empty read, which then doesn't make it into the QueryResult // given the fact that we always run with the partitions defined this should be fine - try (HistoryValidator.Checker checker = validator.witness(outcome.start, outcome.end)) + try (HistoryValidator.Checker checker = validator.witness(outcome, outcome.start, outcome.end)) { while (result.hasNext()) { diff --git a/test/simulator/main/org/apache/cassandra/simulator/paxos/StrictSerializabilityValidator.java b/test/simulator/main/org/apache/cassandra/simulator/paxos/StrictSerializabilityValidator.java index 332bb3d660..5691d7ce02 100644 --- a/test/simulator/main/org/apache/cassandra/simulator/paxos/StrictSerializabilityValidator.java +++ b/test/simulator/main/org/apache/cassandra/simulator/paxos/StrictSerializabilityValidator.java @@ -43,7 +43,7 @@ public class StrictSerializabilityValidator implements HistoryValidator } @Override - public Checker witness(int start, int end) + public Checker witness(Object witnessedBy, int start, int end) { verifier.begin(); return new Checker() @@ -63,7 +63,7 @@ public class StrictSerializabilityValidator implements HistoryValidator @Override public void close() { - convertHistoryViolation(() -> verifier.apply("", start, end)); + convertHistoryViolation(() -> verifier.apply(witnessedBy, start, end)); } }; } diff --git a/test/simulator/test/org/apache/cassandra/simulator/paxos/HistoryValidatorTest.java b/test/simulator/test/org/apache/cassandra/simulator/paxos/HistoryValidatorTest.java index a52345d080..46b4345ab4 100644 --- a/test/simulator/test/org/apache/cassandra/simulator/paxos/HistoryValidatorTest.java +++ b/test/simulator/test/org/apache/cassandra/simulator/paxos/HistoryValidatorTest.java @@ -326,7 +326,7 @@ public class HistoryValidatorTest { String type = events.length == 1 ? "single" : "multiple"; logger.info("[Validator={}, Observation=({}, {}, {})] Validating {} {}}", validator.getClass().getSimpleName(), ob.id, ob.start, ob.end, type, events); - try (HistoryValidator.Checker check = validator.witness(ob.start, ob.end)) + try (HistoryValidator.Checker check = validator.witness(ob, ob.start, ob.end)) { for (Event e : events) e.process(ob, check); @@ -477,7 +477,7 @@ public class HistoryValidatorTest void process(HistoryValidator validator) { - try (HistoryValidator.Checker check = validator.witness(start, end)) + try (HistoryValidator.Checker check = validator.witness("", start, end)) { for (Operation a : actions) a.check(check);