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

kvserver: only pause followers when holding active lease #85732

Merged
merged 4 commits into from
Aug 10, 2022

Conversation

tbg
Copy link
Member

@tbg tbg commented Aug 8, 2022

If the raft leader is not the leaseholder (which includes the case in
which we just transferred the lease away), leave all followers unpaused.
Otherwise, the leaseholder won't learn that the entries it submitted
were committed which effectively causes range unavailability.

Fixes #84884.

Release note: None

tbg added 2 commits August 8, 2022 13:37
Intentionally leaving this unsimplified as a purely mechanical commit.

Release note: None
@tbg tbg requested review from erikgrinaker and a team August 8, 2022 11:51
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member Author

tbg commented Aug 8, 2022

Note: also still need to make sure a leader never considers itself paused. (edit: done)

tbg added 2 commits August 8, 2022 15:40
If the raft leader is not the leaseholder (which includes the case in
which we just transferred the lease away), leave all followers unpaused.
Otherwise, the leaseholder won't learn that the entries it submitted
were committed which effectively causes range unavailability.

Fixes cockroachdb#84884.

Release note: None
As observed in cockroachdb#84884, an overloaded store that held leases could end up
"pausing" replication traffic to itself. This (likely) had no practical
effect since the leader never sends messages to itself, but it meant
reporting bogus counts of paused replicas.

This commit ensures that a raft leader will never pause itself.

Release note: None
@tbg tbg force-pushed the paused-leader-not-leaseholder branch from b52a183 to e4ae047 Compare August 8, 2022 13:48
@tbg tbg marked this pull request as ready for review August 9, 2022 09:20
@tbg tbg requested a review from a team as a code owner August 9, 2022 09:20
@tbg
Copy link
Member Author

tbg commented Aug 10, 2022

bors r=erikgrinaker
TFTR!

@craig
Copy link
Contributor

craig bot commented Aug 10, 2022

Build succeeded:

@craig craig bot merged commit 9be7cf5 into cockroachdb:master Aug 10, 2022
@tbg tbg deleted the paused-leader-not-leaseholder branch August 11, 2022 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kv: paused followers interacts poorly with leader-not-leaseholder state
3 participants