belliottsmith commented on code in PR #80:
URL: https://github.com/apache/cassandra-accord/pull/80#discussion_r1500882949
##########
accord-core/src/main/java/accord/coordinate/CoordinatePreAccept.java:
##########
@@ -210,92 +115,32 @@ public synchronized void onSuccess(Id from,
PreAcceptReply reply)
}
@Override
- public void setFailure(Throwable failure)
+ boolean onExtraSuccessInternal(Id from, PreAcceptReply reply)
{
- Invariants.checkState(!initialPreAcceptIsDone || (extraPreAccept !=
null && !extraPreAccept.extraPreAcceptIsDone));
- initialPreAcceptIsDone = true;
- if (extraPreAccept != null)
- extraPreAccept.extraPreAcceptIsDone = true;
- if (failure instanceof CoordinationFailed)
- {
- ((CoordinationFailed) failure).set(txnId, route.homeKey());
- if (failure instanceof Timeout)
- node.agent().metricsEventsListener().onTimeout(txnId);
- else if (failure instanceof Preempted)
- node.agent().metricsEventsListener().onPreempted(txnId);
- else if (failure instanceof Invalidated)
- node.agent().metricsEventsListener().onInvalidated(txnId);
- }
- super.setFailure(failure);
- }
-
- void onPreAcceptedOrNewEpoch()
- {
- Invariants.checkState(!initialPreAcceptIsDone || (extraPreAccept !=
null && !extraPreAccept.extraPreAcceptIsDone));
- initialPreAcceptIsDone = true;
- if (extraPreAccept != null)
- extraPreAccept.extraPreAcceptIsDone = true;
+ if (!reply.isOk())
+ return false;
- // if the epoch we are accepting in is later, we *must* contact the
later epoch for pre-accept, as this epoch
- // could have moved ahead, and the timestamp we may propose may be
stale.
- // Note that these future epochs are non-voting, they only serve to
inform the timestamp we decide
- Timestamp executeAt = foldl(successes, (ok, prev) ->
mergeMax(ok.witnessedAt, prev), Timestamp.NONE);
- if (executeAt.epoch() <= topologies.currentEpoch())
- onPreAccepted(topologies, executeAt, successes);
- else
- onNewEpoch(topologies, executeAt, successes);
+ PreAcceptOk ok = (PreAcceptOk) reply;
+ oks.add(ok);
+ return true;
}
- void onNewEpoch(Topologies prevTopologies, Timestamp executeAt,
List<PreAcceptOk> successes)
+ @Override
+ void onNewEpochTopologyMismatch(TopologyMismatch mismatch)
{
- // TODO (desired, efficiency): check if we have already have a valid
quorum for the future epoch
- // (noting that nodes may have adopted new ranges, in which case they
should be discounted, and quorums may have changed shape)
- node.withEpoch(executeAt.epoch(), () -> {
- TopologyMismatch mismatch =
TopologyMismatch.checkForMismatch(node.topology().globalForEpoch(executeAt.epoch()),
txnId, route.homeKey(), txn.keys());
- if (mismatch != null)
- {
- initialPreAcceptIsDone = true;
- Propose.Invalidate.proposeInvalidate(node, new
Ballot(node.uniqueNow()), txnId, route.someParticipatingKey(), (outcome,
failure) -> {
- if (failure != null)
- mismatch.addSuppressed(failure);
- accept(null, mismatch);
- });
- return;
- }
- topologies = node.topology().withUnsyncedEpochs(route,
txnId.epoch(), executeAt.epoch());
- boolean equivalent = topologies.oldestEpoch() <=
prevTopologies.currentEpoch();
- for (long epoch = topologies.currentEpoch() ; equivalent && epoch
> prevTopologies.currentEpoch() ; --epoch)
- equivalent =
topologies.forEpoch(epoch).shards().equals(prevTopologies.current().shards());
-
- if (equivalent)
- {
- onPreAccepted(topologies, executeAt, successes);
- }
- else
- {
- extraPreAccept = new
ExtraPreAccept(prevTopologies.currentEpoch() + 1, executeAt.epoch());
- extraPreAccept.start();
- }
+ Propose.Invalidate.proposeInvalidate(node, new
Ballot(node.uniqueNow()), txnId, route.someParticipatingKey(), (outcome,
failure) -> {
Review Comment:
It cannot succeed because the data involved no longer exists. A topology
mismatch means the ranges involved are partially not present anymore.
Regarding reading data that is no longer valid, replicas already have a
concept of stale ranges. When we drop data we can mark the range as stale for
the command stores that previously owned them, so any ephemeral read that
contacts them will understand the replica was unable to satisfy the request
(this is already plumbed in).
We don’t currently have any support for dropping data really, I will leave a
TODO related to this and file a ticket.
We also need to have ephemeral reads detect that they need to execute in a
later epoch, but I planned to address that later. But I might look at that
next, followed by asynchronous dependency handling.
--
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]
