belliottsmith commented on code in PR #113:
URL: https://github.com/apache/cassandra-accord/pull/113#discussion_r1746073492
##########
accord-core/src/main/java/accord/messages/PreAccept.java:
##########
@@ -110,26 +111,32 @@ public PreAcceptReply apply(SafeCommandStore safeStore)
// note: this diverges from the paper, in that instead of waiting for
JoinShard,
// we PreAccept to both old and new topologies and require
quorums in both.
// This necessitates sending to ALL replicas of old topology,
not only electorate (as fast path may be unreachable).
- if (minUnsyncedEpoch < txnId.epoch() &&
!safeStore.ranges().coordinates(txnId).intersects(scope))
+ if (minEpoch < txnId.epoch() &&
!safeStore.ranges().coordinates(txnId).intersects(scope))
return applyIfDoesNotCoordinate(safeStore);
SafeCommand safeCommand = safeStore.get(txnId, this, route);
- switch (Commands.preaccept(safeStore, safeCommand, txnId, maxEpoch,
partialTxn, route, progressKey))
+ Commands.AcceptOutcome outcome = Commands.preaccept(safeStore,
safeCommand, txnId, maxEpoch, partialTxn, route);
+ Command command = safeCommand.current();
+ switch (outcome)
{
default:
- case Success:
// we might hit 'Redundant' if we have to contact later epochs
and partially re-contact a node we already contacted
- // TODO (expected): consider dedicated special case, or rename
+ // TODO (desired): consider dedicated special case, or rename
case Redundant:
- Command command = safeCommand.current();
- // for efficiency, we don't usually return dependencies newer
than txnId as they aren't necessarily needed
- // for recovery, and it's better to persist less data than
more. However, for exclusive sync points we
- // don't need to perform an Accept round, nor do we need to
persist this state to aid recovery. We just
- // want the issuer of the sync point to know which
transactions to wait for before it can safely treat
- // all transactions with lower txnId as expired.
- Ranges ranges =
safeStore.ranges().allBetween(minUnsyncedEpoch, txnId);
- return new PreAcceptOk(txnId, command.executeAt(),
- calculatePartialDeps(safeStore, txnId,
command.partialTxn().keys(), scope, EpochSupplier.constant(minUnsyncedEpoch),
txnId, ranges));
+ case Success:
+ if (command.saveStatus().compareTo(SaveStatus.PreAccepted) > 0)
Review Comment:
Either the coordinator has already moved on to Accept and later phases, so
this has been delivered out of order and the coordinator will discard this
nack, or it has already been interrupted and the command is being successfully
recovered by some recovery coordinator in which case it should stop.
--
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]
