aweisberg commented on code in PR #80:
URL: https://github.com/apache/cassandra-accord/pull/80#discussion_r1500849304
##########
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:
My concern here returning an error to the client is that we surface an error
for a read that should still be able to succeed if retried? I don't think a
topology change is a good reason for that?
--
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]
