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.

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

box: prevent demoted leader from being a candidate in the next elections #10003

Merged

Conversation

CuriousGeorgiy
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy commented May 11, 2024

This patch prevents the demoted leader from being a candidate in the next elections.

Closes #9855

@coveralls
Copy link

coveralls commented May 11, 2024

Coverage Status

coverage: 87.135% (+0.02%) from 87.116%
when pulling 374f8c9 on CuriousGeorgiy:gh-9855-demote-self-vote
into 4ccb6db
on tarantool:master
.

Copy link
Contributor

@Serpentian Serpentian left a comment

Choose a reason for hiding this comment

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

Thank you for the patch. Left some comments

src/box/box.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@Serpentian Serpentian left a comment

Choose a reason for hiding this comment

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

Thank you for the patch. LGTM! No major objections

src/lib/raft/raft.c Outdated Show resolved Hide resolved
src/box/box.cc Outdated Show resolved Hide resolved
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9855-demote-self-vote branch 2 times, most recently from 08567b4 to edc283a Compare May 15, 2024 08:59
src/lib/raft/raft.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!
Please find my comments below.

src/lib/raft/raft.c Outdated Show resolved Hide resolved
src/box/box.cc Outdated Show resolved Hide resolved
Suggested by Nikita Zheleztsov in the scope of tarantool#9855.

Needed for tarantool#9855

NO_CHANGELOG=<refactoring>
NO_DOC=<refactoring>
NO_TEST=<refactoring>

Co-authored-by: Nikita Zheleztsov <[email protected]>
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9855-demote-self-vote branch 4 times, most recently from 8e7c77a to b3284e1 Compare May 24, 2024 12:14
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

src/box/box.cc Outdated Show resolved Hide resolved
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

LGTM.

@sergepetrenko sergepetrenko added the full-ci Enables all tests for a pull request label May 29, 2024
@sergepetrenko
Copy link
Collaborator

@CuriousGeorgiy, some tests still fail in CI:
replication-luatest/gh_6860_election_off_demot> [ fail ]
https://github.com/tarantool/tarantool/actions/runs/9284403504/job/25546715760?pr=10003#step:8:12557

@sergepetrenko sergepetrenko removed the full-ci Enables all tests for a pull request label May 29, 2024
Currently, the demoted leader sees that nobody has requested a vote in the
newly persisted term (because it has just written it without voting, and
nobody had time to see the new term yet), and hence votes for itself,
becoming the most probable winner of the next elections.

To prevent this from happening, let's forbid the demoted leader to be a
candidate in the next elections using `box_raft_leader_step_off`.

Closes tarantool#9855

NO_DOC=<bugfix>

Co-authored-by: Serge Petrenko <[email protected]>
@CuriousGeorgiy CuriousGeorgiy added static-build-ci Enables static build tests for a pull request full-ci Enables all tests for a pull request labels May 29, 2024
@CuriousGeorgiy
Copy link
Member Author

riousGeorgiy, some tests still fail in CI:
replication-luatest/gh_6860_election_off_demot> [ fail ]
tarantool/tarantool/actions/runs/9284403504/job/25546715760?pr=10003#step:8:12557

@sergepetrenko fixed: added a synchronization point to wait for the restarted server to synchronize with the leader.

@sergepetrenko sergepetrenko merged commit 05d03a1 into tarantool:master May 29, 2024
110 of 120 checks passed
@sergepetrenko
Copy link
Collaborator

Backports:
3.1 #10067
2.11 #10068

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request static-build-ci Enables static build tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

box.ctl.demote() does not guarantee demoting
5 participants