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

storage: precheck whether the peer is leader when acquiring latches failed (#13254) #13318

Merged
merged 3 commits into from Oct 12, 2022

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Aug 19, 2022

cherry-pick #13254 to release-6.1
You can switch your code base to this Pull Request by using git-extras:

# In tikv repo:
git pr https://github.com/tikv/tikv/pull/13318

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/tikv.git pr/13318:release-6.1-58fa80e0de0d

What is changed and how it works?

Issue Number: Close #12966

What's Changed:

When a tikv is isolated from other tikv instances, some requests will be 
blocked in raftstore and the corresponding latches are not released. 
Following requests which require the latches will receive ServerIsBusy error
and keep retrying. However, In such case, peers on the tikv are not leader
anymore. The client is supposed to receive NotLeader error immediately.

This commit introduces fail fast mode to scheduler. When a request 
fails to acquire any latch, scheduler checks if the peer is still leader.
If it still the leader, schedule the request as usual, fail fast otherwise.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

Now
image

Release note

Fix the problem that QPS may drop to zero for several mintues when a tikv is partitioned.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 19, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • sticnarf
  • tonyxuqqi

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.

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@cosven please accept the invitation then you can push to the cherry-pick pull requests. Comment with /cherry-pick-invite if the invitation is expired.
https://github.com/ti-srebot/tikv/invitations

Signed-off-by: cosven <yinshaowen241@gmail.com>
@cosven
Copy link
Member

cosven commented Aug 22, 2022

Conflicts are resolved and this PR is ready for review.

/cc @sticnarf @tonyxuqqi @BusyJay

Compared to the original PR, this PR has little change. You can see all changes in this commit f1a52c0 .

Copy link
Contributor

@sticnarf sticnarf 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-chi-bot ti-chi-bot added the status/LGT1 Status: PR - There is already 1 approval label Aug 23, 2022
@ti-chi-bot ti-chi-bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Aug 24, 2022
@VelocityLight VelocityLight added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Oct 12, 2022
@BusyJay
Copy link
Member

BusyJay commented Oct 12, 2022

/merge

@ti-chi-bot
Copy link
Member

@BusyJay: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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.

@ti-chi-bot
Copy link
Member

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

Commit hash: bd001fa

@ti-chi-bot ti-chi-bot added the status/can-merge Status: Can merge to base branch label Oct 12, 2022
@ti-chi-bot ti-chi-bot merged commit 1ae05e3 into tikv:release-6.1 Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Cherry pick PR approved by release team. release-note size/XL status/can-merge Status: Can merge to base branch status/LGT2 Status: PR - There are already 2 approvals type/cherry-pick Type: PR - Cherry pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants