belliottsmith commented on code in PR #3527:
URL: https://github.com/apache/cassandra/pull/3527#discussion_r1764818343
##########
src/java/org/apache/cassandra/service/paxos/PaxosPrepare.java:
##########
@@ -459,26 +459,42 @@ public synchronized void onResponse(Response response,
InetAddressAndPort from)
}
Permitted permitted = response.permitted();
- if (permitted.gossipInfo.isEmpty())
+
+ // If the peer's local electorate disagreed with ours it will be
signalled in the permitted response.
+ // Pre 5.1 this used gossip state to assess the relative currency of
either peer's view of the ring/placements
+ // from which the electorate is derived. Post 5.1, this is driven by
cluster metadata rather than gossip but we
+ // preserve the signalling via gossip state for continuity during
upgrades
+ Epoch remoteElectorateEpoch = permitted.electorateEpoch;
+
+ if (remoteElectorateEpoch.is(Epoch.EMPTY) &&
permitted.gossipInfo.isEmpty())
+ {
// we agree about the electorate, so can simply accept the
promise/permission
+ // TODO: once 5.1 is the minimum supported version, we can stop
sending and checking gossipInfo and just
+ // use the electorateEpoch
permitted(permitted, from);
- else if (!needsGossipUpdate(permitted.gossipInfo))
- // our gossip is up-to-date, but our original electorate could
have been built with stale gossip, so verify it
+ }
+ else if (remoteElectorateEpoch.isAfter(Epoch.EMPTY))
+ {
+ // The remote peer sent back an epoch for its local electorate,
implying that it did not match our original.
+ // That epoch may be after the one we built the original from, so
catch up if we need to and haven't
+ // already. Either way, verify the electorate is still valid
according to the current topology.
+
ClusterMetadataService.instance().fetchLogFromPeerOrCMS(ClusterMetadata.current(),
from, remoteElectorateEpoch);
permittedOrTerminateIfElectorateMismatch(permitted, from);
+ }
else
- // otherwise our beliefs about the ring potentially diverge, so
update gossip with the peer's information
- Stage.GOSSIP.executor().execute(() -> {
- Gossiper.instance.notifyFailureDetector(permitted.gossipInfo);
- Gossiper.instance.applyStateLocally(permitted.gossipInfo);
- // TODO: We should also wait for schema pulls/pushes, however
this would be quite an involved change to MigrationManager
- // (which currently drops some migration tasks on the
floor).
- // Note it would be fine for us to fail to complete the
migration task and simply treat this response as a failure/timeout.
-
- // once any pending ranges have been calculated, refresh our
Participants list and submit the promise
- // todo: verify that this is correct, we no longer have any
pending ranges, just call this immediately
- //
PendingRangeCalculatorService.instance.executeWhenFinished(() ->
permittedOrTerminateIfElectorateMismatch(permitted, from));
- permittedOrTerminateIfElectorateMismatch(permitted, from);
- });
+ {
+ // The remote peer indicated a mismatch, but is either still
running a pre-5.1 version or we have not yet
Review Comment:
Isn't it possible for the responding replica to have an older version of the
CMS?
--
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]
