aweisberg commented on code in PR #101:
URL: https://github.com/apache/cassandra-accord/pull/101#discussion_r1663020922


##########
accord-core/src/main/java/accord/coordinate/RecoverWithRoute.java:
##########
@@ -211,9 +211,15 @@ else if (!known.definition.isOrWasKnown())
                 if (known.executeAt.isDecidedAndKnownToExecute() && 
known.deps.hasDecidedDeps() && known.outcome == Apply)
                 {
                     Deps deps = full.stableDeps.reconstitute(route());
-                    node.withEpoch(full.executeAt.epoch(), () -> {
+                    node.withEpoch(full.executeAt.epoch(), (ignored, 
withEpochFailure) -> {
+                        if (withEpochFailure != null)
+                        {
+                            // TODO (review): We already ignore persist errors 
here by not providing a callback to persist
+                            return;
+                        }
                         persist(node.coordinationAdapter(txnId, 
InitiateRecovery), node, topologies, route(), txnId, txn, full.executeAt, deps, 
full.writes, full.result, null);
                     });
+                    // TODO (review): We say it was applied regardless of what 
happens elsewhere? Could/should this move into the withEpoch?
                     callback.accept(APPLIED, null);

Review Comment:
   @belliottsmith I want to call this out. It seems like we assume it's applied 
without checking if it actually worked?



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