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