belliottsmith commented on code in PR #2056:
URL: https://github.com/apache/cassandra/pull/2056#discussion_r1067526565


##########
test/simulator/main/org/apache/cassandra/simulator/paxos/PairOfSequencesAccordSimulation.java:
##########
@@ -281,21 +211,151 @@ protected String preInsertStmt()
     }
 
     @Override
-    Operation verifying(int operationId, IInvokableInstance instance, int 
primaryKey, HistoryChecker historyChecker)
+    boolean allowMultiplePartitions() { return true; }
+
+    @Override
+    BiFunction<SimulatedSystems, int[], Supplier<Action>> actionFactory()
     {
-        return new VerifyingOperation(operationId, instance, 
serialConsistency, primaryKey, historyChecker);
+        AtomicInteger id = new AtomicInteger(0);
+
+        return (simulated, primaryKeyIndex) -> {
+            int[] partitions = IntStream.of(primaryKeyIndex).map(i -> 
primaryKeys[i]).toArray();
+            return () -> accordAction(id.getAndIncrement(), simulated, 
partitions);
+        };
     }
 
-    @Override
-    Operation nonVerifying(int operationId, IInvokableInstance instance, int 
primaryKey, HistoryChecker historyChecker)
+    private static IIsolatedExecutor.SerializableCallable<SimpleQueryResult> 
query(int id, int[] partitions, int[] readOnly)
     {
-        return new NonVerifyingOperation(operationId, instance, 
serialConsistency, primaryKey, historyChecker);
+        return () -> execute(createAccordTxn(id, partitions, readOnly), "pk", 
"count", "seq");
     }
 
-    @Override
-    Operation modifying(int operationId, IInvokableInstance instance, int 
primaryKey, HistoryChecker historyChecker)
+    public class ReadWriteOperation extends Operation
     {
-        return new ModifyingOperation(operationId, instance, ANY, 
serialConsistency, primaryKey, historyChecker);
+        private final IntSet allPartitions;
+        private final IntSet readOnlySet;

Review Comment:
   It does not do a read at the CQL layer, only internally. From the CQL 
perspective it is a blind write for Paxos.
   
   We _shouldn't_ have access to the read in Accord if we perform a blind write 
either, as it shouldn't be returned to us since we didn't ask for it. We should 
be verifying this behaviour, even if it is only a modest divergence. Perhaps a 
small minority of writes should be run as blind at the CQL layer, but we should 
run at least some.
   
   > I don't know what paxos does, can you explain? Given seq1 text I assume 
this is doing a set rather than append? So would need to extend the validators 
to handle this truncate of history? How does paxos handle this differently?
   
   In this instance I am proposing additional functionality compared to the 
Paxos test, namely for some test runs we might want to randomly disable `seq1` 
entirely for the test duration, operating only on `seq2`, i.e. maintaining only 
seq2 and reading only seq2. This would permit us to run entirely on _true_ 
blind writes, to confirm they behave correctly, as they diverge more in 
implementation than with Paxos.
   
   In the future, we might want to modify these tests not to rely on the 
reified history, by at least permitting history to be truncated. This is 
definitely possible with the strict serialisability verifier as it is designed, 
if we were to also request timestamps on read. But that's for another time.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to