pd
pd copied to clipboard
server: Resign as leader on exit or SIGHUP
close tikv/pd#6344
What problem does this PR solve?
Issue Number: Close #6344
What is changed and how does it work?
Check List
Tests
- Unit test
- Integration test
- Manual test (add detailed scripts or steps below)
- No code
Code changes
- Has the configuration change
- Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
- Has persistent data change
Side effects
- Possible performance regression
- Increased code complexity
- Breaking backward compatibility
Related changes
- PR to update
pingcap/docs
/pingcap/docs-cn
: - PR to update
pingcap/tiup
: - Need to cherry-pick to the release branch
Release note
None.
[REVIEW NOTIFICATION]
This pull request has not been approved.
To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer
in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer
in the comment to help you merge this pull request.
The full list of commands accepted by this bot can be found here.
Reviewer can indicate their review by submitting an approval review. Reviewer can cancel approval by submitting a request changes review.
Hi @mjonss. Thanks for your PR.
I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
I'm not sure if this patch is intended to solve the query latency spike issue caused by rolling restart of PD. If so, this patch may not solve the problem.
We conducted an experiment by simulating the rolling update process of PD using the command /pd-ctl member leader transfer
manually. We made sure that the leadership of a PD was transferred to another PD WITHOUT shutting it down. However, we still observed 2-3 minute spikes in P100 TiDB query latency.
This experiment demonstrates that even with graceful shutdown, re-election still occurs and it impacts TiDB queries. This PR should only save the time for PD to realize there is no leader (etcd election timeout, by default it's 1 second).
cc: @nolouch
I'm not sure if this patch is intended to solve the query latency spike issue caused by rolling restart of PD. If so, this patch may not solve the problem.
We conducted an experiment by simulating the rolling update process of PD using the command
/pd-ctl member leader transfer
manually. We made sure that the leadership of a PD was transferred to another PD WITHOUT shutting it down. However, we still observed 2-3 minute spikes in P100 TiDB query latency.This experiment demonstrates that even with graceful shutdown, re-election still occurs and it impacts TiDB queries. This PR should only save the time for PD to realize there is no leader (etcd election timeout, by default it's 1 second).
cc: @nolouch
@coderplay Is it only a P100/long tail latency, or all TiDB queries that are affected in your experiment?
@coderplay Is it only a P100/long tail latency, or all TiDB queries that are affected in your experiment?
This is P100/long tail latency, no obvious spikes on P95.
Theoretically, this PR won't avoid the leader re-election process, right?
The best approach would be to make the PD leader cutover an atomic process. The old leader should still serve as a leader and continue to serve TSO until the cutover occurs. However, there is a detail here. PD's clients -- TiDB/TiKV, are not aware of this atomic cutover. They will only query the new leader's address after a failure.
@coderplay: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.
After looking at the PD code, the pd-ctl transfer leader
and the resign
in this PR are actually equivalent. Resign
simply allows PD to randomly select another PD as the new leader. It actually avoids the Etcd election timeout and re-election process, while a non-graceful shutdown does NOT. In another experiment, we found that although this PR still has some blips in P100 latency, P999 is not affected, while non-graceful shutdowns have obvious blips in P999.
so, +1 for the patch.
PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.