Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not request votes if term changed in prevote phase #18394

Merged
merged 3 commits into from
May 14, 2024
Merged

Do not request votes if term changed in prevote phase #18394

merged 3 commits into from
May 14, 2024

Conversation

mmaslankaprv
Copy link
Member

@mmaslankaprv mmaslankaprv commented May 10, 2024 •
edited
Loading

In vote stm we didn't mark the vote replies with greater term as not granted votes. This may lead to unnecessary leader elections and longer recovery after failures. Change the logic in vote_stm to explicitly mark vote responses with greater term as failed. Additional check was added in consensus to recognise if a candidate state changed in between the pre-vote and vote phase.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

Improvements

  • Made electing a leader faster

@mmaslankaprv mmaslankaprv requested a review from ztlpn May 10, 2024 12:49
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented May 10, 2024 •
edited
Loading

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63af-8550-485b-8694-805474ef15c4

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63af-854d-46c0-b9d0-a32045246723

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63ce-bde0-4a42-a863-f82dfc0264e7

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63ce-bde7-4438-bcfe-7d1dc676419a

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63ce-bde2-4fe3-bd1e-1d9a5d169d8b

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48944#018f63ce-bde5-4586-80fe-087778e7016a

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48985#018f723e-856e-4b5c-8fb4-e4deaad8ded9

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49079#018f75f3-820d-44ca-8232-e8fd5b3ee40e

@redpanda-data redpanda-data deleted a comment from vbotbuildovich May 12, 2024
@mmaslankaprv
Copy link
Member Author

ci failure: #13275

ztlpn
ztlpn previously approved these changes May 13, 2024
View reviewed changes
src/v/raft/vote_stm.h Outdated Show resolved Hide resolved
src/v/raft/vote_stm.h Outdated
@@ -84,6 +88,10 @@ class vote_stm {
}

if (value->has_value()) {
if (value->value().term != _vote_stm._req.term) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if != should be >? Or is < impossible because the other side will always update its term?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, it is impossible in geral we should never treat replies coming from different ter as succesfull

src/v/raft/vote_stm.h Outdated Show resolved Hide resolved
@mmaslankaprv
r/vote_stm: do not mark vote as granted if reply has larger term
5adeba3 
In vote stm we didn't mark the vote replies with greater term as not
granted votes. This may lead to unnecessary leader elections and longer
recovery after failures. Change the logic in `vote_stm` to explicitly
mark vote responses with greater term as failed.

Signed-off-by: Michał Maślanka <michal@redpanda.com>
@mmaslankaprv
r/consensus: added validation if term has changed between pre and vote
40add5d 
phases

If term changed between pre-vote and actual vote phase a candidate
should not proceed and request votes.

Signed-off-by: Michał Maślanka <michal@redpanda.com>
@mmaslankaprv
r/consensus: added logging around pre-vote election phase
1f0203f 
Added more logging about the start and result of pre-vote election.

Signed-off-by: Michał Maślanka <michal@redpanda.com>
@mmaslankaprv mmaslankaprv requested a review from ztlpn May 14, 2024 10:18
@mmaslankaprv
Copy link
Member Author

ci failures:

@mmaslankaprv mmaslankaprv merged commit fabba81 into redpanda-data:dev May 14, 2024
13 of 17 checks passed
@mmaslankaprv mmaslankaprv deleted the fix-prevote-term branch May 14, 2024 12:32
@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-18394-v23.3.x-361 remotes/upstream/v23.3.x
git cherry-pick -x 5adeba30fa53fe696fa5555b46967397fa0547ed 40add5d6bae852bcb5a8c76255522a0fb06a9b71 1f0203fd915d4313614575a690a0a871cf06ff86

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request May 14, 2024
Open
@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v24.1.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-18394-v24.1.x-0 remotes/upstream/v24.1.x
git cherry-pick -x 5adeba30fa53fe696fa5555b46967397fa0547ed 40add5d6bae852bcb5a8c76255522a0fb06a9b71 1f0203fd915d4313614575a690a0a871cf06ff86

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request May 14, 2024
Closed
@mmaslankaprv mmaslankaprv mentioned this pull request May 15, 2024
Merged
@mmaslankaprv mmaslankaprv mentioned this pull request May 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ztlpn ztlpn ztlpn approved these changes

@bharathv bharathv Awaiting requested review from bharathv

@bashtanov bashtanov Awaiting requested review from bashtanov

Assignees
No one assigned
Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mmaslankaprv @vbotbuildovich @ztlpn