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

store/tikv: always switch to a different peer when meets no-leader #22449

Merged
merged 17 commits into from
Mar 26, 2021

Conversation

youjiali1995
Copy link
Contributor

Signed-off-by: youjiali1995 zlwgx1023@gmail.com

What problem does this PR solve?

Issue Number: close #22400
Problem Summary:

Joint consensus is enabled in master, which is possible to make a leader step down as a learner during a conf change. And hibernate region is also enabled by default in master, so after the leader step down, there can be a long time that there is no leader in the region until requests are sent to followers or hibernate timeout. But from logs, I can see TiDB keeps querying the learner instead of trying other followers, hence it keeps timeout until hibernate is woken up.

What is changed and how it works?

What's Changed:

One solution is always to try a different peer when meets no-leader. There is a small probability that the current peer who reports not-leader becomes a leader and TiDB has to retry once in this case.

Related changes

Check List

Tests

  • Unit test

Release note

  • No release note

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
@youjiali1995
Copy link
Contributor Author

/run-all-tests

1 similar comment
@youjiali1995
Copy link
Contributor Author

/run-all-tests

@ichn-hu ichn-hu mentioned this pull request Jan 20, 2021
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 21, 2021
@cfzjywxk
Copy link
Contributor

Maybe we need to add more integrated failover tests as joint-consensus is enabled default to ensure there is no availability issues.

@cfzjywxk
Copy link
Contributor

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jan 21, 2021
ti-srebot
ti-srebot previously approved these changes Jan 21, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 21, 2021
@youjiali1995 youjiali1995 added the status/can-merge Indicates a PR has been approved by a committer. label Jan 21, 2021
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@youjiali1995 merge failed.

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
@youjiali1995
Copy link
Contributor Author

/run-all-tests

@youjiali1995
Copy link
Contributor Author

/run-integration-ddl-test

@youjiali1995
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@youjiali1995 merge failed.

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
@ti-chi-bot
Copy link
Member

@MyonKeminta: Please use /LGTM instead of LGTM when you want to approve the pull request by comment.
If you use the GitHub review feature, please approve the PR directly, the comment will not take effect in the GitHub review feature.
If you have any qustions please refer to lgtm command help or lgtm plugin design.

If you have approved this PR, please ignore this reply. This reply is being used as a temporary reply during the migration of the new bot and will be removed on April 1.

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.

Copy link
Contributor

@MyonKeminta MyonKeminta left a comment

Choose a reason for hiding this comment

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

LGTM

@youjiali1995
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 6a9946e

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 26, 2021
@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 26, 2021
@youjiali1995
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 44e8a12

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 26, 2021
@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Mar 26, 2021
@youjiali1995
Copy link
Contributor Author

/run-check_dev_2

@youjiali1995
Copy link
Contributor Author

/run-check_dev_2

@youjiali1995
Copy link
Contributor Author

/run-unit-test

@cfzjywxk
Copy link
Contributor

/run-check_dev_2

@ti-chi-bot ti-chi-bot merged commit 2df2ca0 into pingcap:master Mar 26, 2021
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Mar 26, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0 in PR #23595

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/store needs-cherry-pick-release-5.0 sig/sql-infra SIG: SQL Infra sig/transaction SIG:Transaction size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiDB timeout when killing an instance of TiKV
6 participants