belliottsmith commented on code in PR #203:
URL: https://github.com/apache/cassandra-accord/pull/203#discussion_r2141172335


##########
accord-core/src/main/java/accord/coordinate/CoordinatePreAccept.java:
##########
@@ -142,7 +142,7 @@ void onNewEpochTopologyMismatch(TopologyMismatch mismatch)
         proposeInvalidate(node, node.uniqueTimestamp(Ballot::fromValues), 
txnId, route.homeKey(), (outcome, failure) -> {
             if (failure != null)
                 mismatch.addSuppressed(failure);
-            setFailure(mismatch);
+            callback.accept(null, mismatch);

Review Comment:
   > "logically" or is isDone already set to true? 
   
   both....
   
   > Based off the current logic when isDone is set to true we also notify the 
callback. 
   
   No, that is not true. `isDone` does not mean the callback is invoked, it 
means we have completed the `PreAccept` phase and have passed the callback onto 
another phase, normally either Execute, Accept or AcceptInvalidate. It _could_ 
mean we invoke the callback if there were no other phase to invoke; indeed, 
invoking AcceptInvalidate is logically optional, so we could choose to invoke 
the callback immediately.
   
   > Can we also keep the eventListener logic?
   
   It's a no-op? We know what the type is, and it doesn't get invoked. We 
should revisit all of this in future, as maybe this isn't ideal behaviour, but 
I intend to look at observability holistically once stability is resolved, so 
better considered then.



-- 
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

Reply via email to