store/tikv: increase max backoff time of read requests #25153

sticnarf · 2021-06-04T07:45:36Z

What is changed and how it works?

Like how #25040 changes prewrite and commit, this PR increases the max backoff to 10 minutes. So, if some TiKV regions are unavailable in a short time due to slow apply, the user will not receive an error.

This PR also make real EpochNotMatch errors not backoff.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Release note

Increase timeout for read requests to avoid propagating TiKV errors.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

youjiali1995 · 2021-06-04T11:30:10Z

store/tikv/2pc.go

@@ -745,7 +746,7 @@ func (tm *ttlManager) keepAlive(c *twoPhaseCommitter) {
 			if tm.lockCtx != nil && tm.lockCtx.Killed != nil && atomic.LoadUint32(tm.lockCtx.Killed) != 0 {
 				return
 			}
-			bo := retry.NewBackofferWithVars(context.Background(), pessimisticLockMaxBackoff, c.txn.vars)
+			bo := retry.NewBackofferWithVars(context.Background(), keepAliveMaxBackoff, c.txn.vars)


I think it shouldn't break the loop if the error is not a key error at L779.

youjiali1995

Rest lgtm

youjiali1995 · 2021-06-04T11:31:37Z

store/tikv/pessimistic.go

@@ -132,10 +132,22 @@ func (action actionPessimisticLock) handleSingleBatch(c *twoPhaseCommitter, bo *
 			return errors.Trace(err)
 		}
 		if regionErr != nil {
-			err = bo.Backoff(retry.BoRegionMiss, errors.New(regionErr.String()))
+			/// For other region error and the fake region error, backoff because


Suggested change

/// For other region error and the fake region error, backoff because

// For other region error and the fake region error, backoff because

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

lysu

rest LGTM

lysu · 2021-06-05T02:35:52Z

store/tikv/2pc.go

 			}
 			continue
 		}
 		if resp.Resp == nil {
-			return 0, errors.Trace(tikverr.ErrBodyMissing)
+			return 0, errors.Trace(err), false


those err should always be nil~?

Sorry, it's a copy-paste mistake...

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

ti-chi-bot · 2021-06-05T06:39:20Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lysu
youjiali1995

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.

lysu · 2021-06-05T06:39:53Z

/merge

ti-chi-bot · 2021-06-05T06:39:56Z

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

Commit hash: 6fb6d1d

bb7133 · 2021-06-05T09:44:36Z

[2021-06-05T07:53:19.089Z] GO111MODULE=on go build -o ../bin/revive github.com/mgechev/revive
[2021-06-05T07:53:19.089Z] linting
[2021-06-05T07:53:19.089Z]   ✘  https://revive.run/r#error-return  error should be the last type when returning multiple items  
[2021-06-05T07:53:19.089Z]   store/tikv/2pc.go:802:1
[2021-06-05T07:53:19.089Z]

@sticnarf Please fix the linting check.

bb7133 · 2021-06-05T09:44:43Z

https://ci.pingcap.net/blue/organizations/jenkins/tidb_ghpr_check/detail/tidb_ghpr_check/9030/pipeline

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

sticnarf · 2021-06-06T05:30:32Z

/merge

ti-chi-bot · 2021-06-06T05:30:35Z

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

Commit hash: 4d42c42

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

sticnarf · 2021-06-06T06:54:20Z

/merge

ti-chi-bot · 2021-06-06T06:54:23Z

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

Commit hash: 1091693

ti-chi-bot · 2021-06-06T07:10:58Z

@sticnarf: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sticnarf · 2021-06-06T07:19:49Z

/rebuild

store/tikv: increase max backoff time of read requests

75edd56

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

sticnarf added the sig/transaction SIG:Transaction label Jun 4, 2021

ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 4, 2021

Merge branch 'master' into read-backoff

e0a4da2

sticnarf requested a review from youjiali1995 June 4, 2021 08:47

increase max backoff for pessimistic locks

dbd77ee

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

sticnarf requested review from lysu and ekexium June 4, 2021 10:59

Merge branch 'master' into read-backoff

bb67238

youjiali1995 reviewed Jun 4, 2021

View reviewed changes

sticnarf added 2 commits June 4, 2021 20:49

address comment

42297a1

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Merge remote-tracking branch 'upstream/master' into read-backoff

e367917

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 4, 2021

increase fail count before check

b0ecf26

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

youjiali1995 approved these changes Jun 4, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 4, 2021

lysu reviewed Jun 5, 2021

View reviewed changes

address comment

ab6d396

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

lysu approved these changes Jun 5, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 5, 2021

Merge branch 'master' into read-backoff

6fb6d1d

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 5, 2021

Merge branch 'master' into read-backoff

aaaa673

ti-chi-bot and others added 2 commits June 6, 2021 01:30

Merge branch 'master' into read-backoff

461c7e6

fix lint

4d42c42

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 6, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 6, 2021

ti-chi-bot and others added 2 commits June 6, 2021 13:30

Merge branch 'master' into read-backoff

3e0096f

try to fix cla

1091693

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 6, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 6, 2021

Merge branch 'master' into read-backoff

50ac302

ti-chi-bot merged commit 7cc1ebc into pingcap:master Jun 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

store/tikv: increase max backoff time of read requests #25153

store/tikv: increase max backoff time of read requests #25153

sticnarf commented Jun 4, 2021

youjiali1995 Jun 4, 2021

youjiali1995 left a comment

youjiali1995 Jun 4, 2021

lysu left a comment

lysu Jun 5, 2021

sticnarf Jun 5, 2021

ti-chi-bot commented Jun 5, 2021

lysu commented Jun 5, 2021

ti-chi-bot commented Jun 5, 2021

bb7133 commented Jun 5, 2021

bb7133 commented Jun 5, 2021

sticnarf commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

sticnarf commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

sticnarf commented Jun 6, 2021

	/// For other region error and the fake region error, backoff because
	// For other region error and the fake region error, backoff because

store/tikv: increase max backoff time of read requests #25153

store/tikv: increase max backoff time of read requests #25153

Conversation

sticnarf commented Jun 4, 2021

What is changed and how it works?

Check List

Release note

youjiali1995 Jun 4, 2021

Choose a reason for hiding this comment

youjiali1995 left a comment

Choose a reason for hiding this comment

youjiali1995 Jun 4, 2021

Choose a reason for hiding this comment

lysu left a comment

Choose a reason for hiding this comment

lysu Jun 5, 2021

Choose a reason for hiding this comment

sticnarf Jun 5, 2021

Choose a reason for hiding this comment

ti-chi-bot commented Jun 5, 2021

lysu commented Jun 5, 2021

ti-chi-bot commented Jun 5, 2021

bb7133 commented Jun 5, 2021

bb7133 commented Jun 5, 2021

sticnarf commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

sticnarf commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

ti-chi-bot commented Jun 6, 2021

sticnarf commented Jun 6, 2021