raft: ignore setting the lead field from a MsgDeFortify at current term #142997
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
miraradeva
approved these changes
Mar 18, 2025
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @iskettaneh)
-- commits line 25 at r2:
nit: "that still know who think they know ..." -> "that still know who the leader is"
-- commits line 27 at r2:
Maybe say "lost for a different reason (e.g. not up-to-date log)".
This commit adds a test that reproduces a liveness issue that we currently have with leader leases where no peer can successfully campaign after a temporary loss of quorum. References: cockroachdb#142994 Release note: None
When receiving a MsgDeFortifyLeader at the same term as we are, we should not set the lead field to the sender. Mainly for two reasons: 1) By definition, a MsgDeFortifyLeader is sent by an ex-leader until it hears of a new committed term. If we forgot the leader at the current term, we shouldn't remember it since we are using the fact that lead==None to indicate that this replica has been leaderless for some time. Read the leaderlessWatcher for more details. 2) This could lead to a situation where no replica can win an election as it could require votes from some replicas that still know who know the leader is (due to MsgDefortifyLeader), and that have recently campaigned and lost due to not having the most up-to-date log, which reset the electionElapsed to 0. Meaning that this replica is in a heartbeat lease, and will reject MsgVotes. Fixes: cockroachdb#142994 Release note: None
iskettaneh
force-pushed
the
RangeUnavailable3
branch
from
64a231c to
e8a9669
Compare
|
TFTR! bors r+ |
celeste-cockroachdb
bot
added
v25.2.0-prerelease
and removed
target-release-25.2.0
labels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When receiving a MsgDeFortifyLeader at the same term as we are, we
should not set the lead field to the sender. Mainly for two reasons:
By definition, a MsgDeFortifyLeader is sent by an ex-leader until it
hears of a new committed term. If we forgot the leader at the current
term, we shouldn't remember it since we are using the fact that
lead==None to indicate that this replica has been leaderless for
some time. Read the leaderlessWatcher for more details.
This could lead to a situation where no replica can win an election
as it could require votes from some replicas that still know who think
they know leader is (due to MsgDefortifyLeader), and that have recently
campaigned and lost, which reset the electionElapsed to 0. Meaning that
this replica is in a heartbeat lease, and will reject MsgVotes.
Fixes: #142994
Release note: None