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

tikv: fix switch region leader to TiFlash when TiFlash return EpochNotMatch #17931

Closed
wants to merge 3 commits into from
Closed

tikv: fix switch region leader to TiFlash when TiFlash return EpochNotMatch #17931

wants to merge 3 commits into from

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Jun 10, 2020

What problem does this PR solve?

Issue Number: close #17930

Problem Summary:

see detail in issue

What is changed and how it works?

What's Changed, How it Works:

we cannot get leader info from epochNotMatchError, but we can choose any tikv store peer as leader.

region cache find leader with one NotLeader resp from any tikv peer.

Related changes

  • Need to cherry-pick to the release branch 4.0

Check List

Tests

  • Unit test

Side effects

  • n/a

Release note

  • Fix switch region leader to TiFlash when TiFlash return EpochNotMatch

This change is Reviewable

@lysu
Copy link
Contributor Author

lysu commented Jun 10, 2020

/run-unit-test

@coocood
Copy link
Member

coocood commented Jun 11, 2020

LGTM

@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #17931 into master will decrease coverage by 0.0567%.
The diff coverage is 87.5000%.

@@               Coverage Diff                @@
##             master     #17931        +/-   ##
================================================
- Coverage   79.5042%   79.4475%   -0.0568%     
================================================
  Files           524        524                
  Lines        142449     141939       -510     
================================================
- Hits         113253     112767       -486     
+ Misses        20070      20037        -33     
- Partials       9126       9135         +9     

@lysu
Copy link
Contributor Author

lysu commented Jun 11, 2020

/rebuild

@lysu lysu requested review from tiancaiamao and jackysp June 11, 2020 08:16
@lysu
Copy link
Contributor Author

lysu commented Jun 11, 2020

/run-unit-test

jackysp
jackysp previously approved these changes Jun 11, 2020
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Member

jackysp commented Jun 11, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 11, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 11, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jun 11, 2020

@lysu merge failed.

@lysu lysu requested a review from jackysp June 12, 2020 12:40
@lysu
Copy link
Contributor Author

lysu commented Jun 12, 2020

@coocood @jackysp at last commit 5bfecf5#diff-708f6242b27e2b7bcf0e905e9b0eacf2R530%EF%BC%8C this PR add mechanism to let wrong request (e.g. leader send to tiflash) can be retry to right store(e.g. retry it to tikv)

choose "request store type" instead of "current store type" to decide retry logic
(so leader request send to tiflash, can be retry to tikv in next time)

to fix the unknown situation in future that lead request stuck..

@@ -522,7 +527,7 @@ func (c *RegionCache) OnSendFail(bo *Backoffer, ctx *RPCContext, scheduleReload
}

// try next peer to found new leader.
if ctx.Store.storeType == kv.TiKV {
if ctx.ReqStoreType == kv.TiKV {
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear what's the difference between ReqStoreType and Store.storeType.

@lysu
Copy link
Contributor Author

lysu commented Jun 16, 2020

replace this with #18040, and this became a hotfix backup pr - -

@lysu lysu closed this Jun 16, 2020
@lysu lysu deleted the dev-fix-tiflash-epoch-not-match branch June 16, 2020 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tiflash epochNotMatch make tiflash as region leader forever
4 participants