Repository: tikv/client-go
Pull request #1511 (2024-11-26 03:43:51)

[6.5]retry as stale reads and remove back off for server busy errors
It contains two changes 
1. Backport of this PR https://github.com/tikv/client-go/pull/1509.
2. Remove back off for server busy errors.  It is already there in master. 
3. Try reads as stale reads if leader is not accessible

Unit test

1. Injected skewed traffic and confirmed that reads are not retried as follower read by looking at read index messages.

ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:


The full list of commands accepted by this bot can be found [here](https://prow.tidb.net/command-help?repo=tikv%2Fclient-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/tikv/client-go/blob/tidb-6.5-with-kv-timeout-feature/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure
- [6b81169](https://github.com/tikv/client-go/commits/6b81169b6ecacf7a0cf279599911a987f08f324a) fixed test cases

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure
- [6b81169](https://github.com/tikv/client-go/commits/6b81169b6ecacf7a0cf279599911a987f08f324a) fixed test cases
- [ec97c19](https://github.com/tikv/client-go/commits/ec97c1936f3f52c7baafedbbf614bd563f0c783e) allow stale read if leader is not even tried

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure
- [6b81169](https://github.com/tikv/client-go/commits/6b81169b6ecacf7a0cf279599911a987f08f324a) fixed test cases
- [ec97c19](https://github.com/tikv/client-go/commits/ec97c1936f3f52c7baafedbbf614bd563f0c783e) allow stale read if leader is not even tried
- [1e34922](https://github.com/tikv/client-go/commits/1e34922baaa1ff842dc6106dce9fc81606761a96) fixed test cases

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure
- [6b81169](https://github.com/tikv/client-go/commits/6b81169b6ecacf7a0cf279599911a987f08f324a) fixed test cases
- [ec97c19](https://github.com/tikv/client-go/commits/ec97c1936f3f52c7baafedbbf614bd563f0c783e) allow stale read if leader is not even tried
- [1e34922](https://github.com/tikv/client-go/commits/1e34922baaa1ff842dc6106dce9fc81606761a96) fixed test cases
- [ba8c21b](https://github.com/tikv/client-go/commits/ba8c21bca439c2f67cef9fcc5d634357f9d8e4ee) disable test

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

:memo: **Please follow instructions in the [contributing guide](https://github.com/tikv/client-go/blob/master/CONTRIBUTING.md) to update your commits with the DCO**

Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).

**The list of commits missing DCO signoff**:

- [0832080](https://github.com/tikv/client-go/commits/0832080ab189da94d112c6823ba62ff874463d87) retry as stale reads
- [6b4a6b5](https://github.com/tikv/client-go/commits/6b4a6b5af0531489b167d37e3a0fd0e60d24838a) build failure
- [6b81169](https://github.com/tikv/client-go/commits/6b81169b6ecacf7a0cf279599911a987f08f324a) fixed test cases
- [ec97c19](https://github.com/tikv/client-go/commits/ec97c1936f3f52c7baafedbbf614bd563f0c783e) allow stale read if leader is not even tried
- [1e34922](https://github.com/tikv/client-go/commits/1e34922baaa1ff842dc6106dce9fc81606761a96) fixed test cases
- [ba8c21b](https://github.com/tikv/client-go/commits/ba8c21bca439c2f67cef9fcc5d634357f9d8e4ee) disable test
- [7fe8732](https://github.com/tikv/client-go/commits/7fe87322dca486687cdef40e25efe5dcb7a6de78) fixed test cases

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>

ti-chi-bot[bot]: @cfzjywxk: adding LGTM is restricted to approvers and reviewers in OWNERS files.

<details>

In response to [this](https://github.com/tikv/client-go/pull/1511#pullrequestreview-2513477972):

>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tikv/client-go/pull/1511#pullrequestreview-2513477972" title="Approved">cfzjywxk</a>*


The full list of commands accepted by this bot can be found [here](https://prow.tidb.net/command-help?repo=tikv%2Fclient-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/tikv/client-go/blob/tidb-6.5-with-kv-timeout-feature/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
ti-chi-bot[bot]: @crazycs520: adding LGTM is restricted to approvers and reviewers in OWNERS files.

<details>

In response to [this](https://github.com/tikv/client-go/pull/1511#pullrequestreview-2513486039):

>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tikv/client-go/pull/1511#pullrequestreview-2513477972" title="Approved">cfzjywxk</a>*, *<a href="https://github.com/tikv/client-go/pull/1511#pullrequestreview-2513486039" title="Approved">crazycs520</a>*


The full list of commands accepted by this bot can be found [here](https://prow.tidb.net/command-help?repo=tikv%2Fclient-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/tikv/client-go/blob/tidb-6.5-with-kv-timeout-feature/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
mittalrishabh: I don't have access to merge. Please merge it. 

Patch:
diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go
index 433aa73..5469c32 100644
--- a/internal/locate/region_request.go
+++ b/internal/locate/region_request.go
@@ -464,6 +464,7 @@ type replica struct {
 	attempts int
 	// deadlineErrUsingConfTimeout indicates the replica is already tried, but the received deadline exceeded error.
 	deadlineErrUsingConfTimeout bool
+	serverIsBusy                bool
 }
 
 func (r *replica) isEpochStale() bool {
@@ -625,7 +626,7 @@ func (state *accessKnownLeader) next(bo *retry.Backoffer, selector *replicaSelec
 	// a request. So, before the new leader is elected, we should not send requests
 	// to the unreachable old leader to avoid unnecessary timeout.
 	if liveness != reachable || leader.isExhausted(maxReplicaAttempt) {
-		selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true}
+		selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true, isStaleRead: false}
 		return nil, stateChanged{}
 	}
 	selector.targetIdx = state.leaderIdx
@@ -640,7 +641,7 @@ func (state *accessKnownLeader) onSendFailure(bo *retry.Backoffer, selector *rep
 		return
 	}
 	if liveness != reachable || selector.targetReplica().isExhausted(maxReplicaAttempt) {
-		selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true}
+		selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true, isStaleRead: false}
 	}
 	if liveness != reachable {
 		selector.invalidateReplicaStore(selector.targetReplica(), cause)
@@ -648,7 +649,7 @@ func (state *accessKnownLeader) onSendFailure(bo *retry.Backoffer, selector *rep
 }
 
 func (state *accessKnownLeader) onNoLeader(selector *replicaSelector) {
-	selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true}
+	selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true, isStaleRead: false}
 }
 
 // tryFollower is the state where we cannot access the known leader
@@ -665,6 +666,7 @@ type tryFollower struct {
 	labels    []*metapb.StoreLabel
 	// fromAccessKnownLeader indicates whether the state is changed from `accessKnownLeader`.
 	fromAccessKnownLeader bool
+	isStaleRead           bool
 }
 
 func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) (*RPCContext, error) {
@@ -724,8 +726,13 @@ func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) (
 		replicaRead := true
 		rpcCtx.contextPatcher.replicaRead = &replicaRead
 	}
-	staleRead := false
-	rpcCtx.contextPatcher.staleRead = &staleRead
+	leader := selector.replicas[state.leaderIdx]
+	if leader.attempts == 0 || leader.deadlineErrUsingConfTimeout || leader.serverIsBusy {
+		rpcCtx.contextPatcher.staleRead = &state.isStaleRead
+	} else {
+		staleRead := false
+		rpcCtx.contextPatcher.staleRead = &staleRead
+	}
 	return rpcCtx, nil
 }
 
@@ -934,9 +941,10 @@ func (state *accessFollower) next(bo *retry.Backoffer, selector *replicaSelector
 			if (state.isStaleRead && !selector.StaleRead.PreventRetryFollower) ||
 				(!state.isStaleRead && leader.deadlineErrUsingConfTimeout) {
 				selector.state = &tryFollower{
-					leaderIdx: state.leaderIdx,
-					lastIdx:   state.leaderIdx,
-					labels:    state.option.labels,
+					leaderIdx:   state.leaderIdx,
+					lastIdx:     state.leaderIdx,
+					labels:      state.option.labels,
+					isStaleRead: state.isStaleRead,
 				}
 				if leaderEpochStale {
 					selector.regionCache.scheduleReloadRegion(selector.region)
@@ -1215,7 +1223,7 @@ func (s *replicaSelector) onReadReqConfigurableTimeout(req *tikvrpc.Request) boo
 		}
 		if accessLeader, ok := s.state.(*accessKnownLeader); ok {
 			// If leader return deadline exceeded error, we should try to access follower next time.
-			s.state = &tryFollower{leaderIdx: accessLeader.leaderIdx, lastIdx: accessLeader.leaderIdx}
+			s.state = &tryFollower{leaderIdx: accessLeader.leaderIdx, lastIdx: accessLeader.leaderIdx, isStaleRead: false}
 		}
 		return true
 	default:
@@ -1224,6 +1232,16 @@ func (s *replicaSelector) onReadReqConfigurableTimeout(req *tikvrpc.Request) boo
 	}
 }
 
+func (s *replicaSelector) onServerIsBusy(req *tikvrpc.Request) {
+	switch req.Type {
+	case tikvrpc.CmdGet, tikvrpc.CmdBatchGet, tikvrpc.CmdScan,
+		tikvrpc.CmdCop, tikvrpc.CmdBatchCop, tikvrpc.CmdCopStream:
+		if target := s.targetReplica(); target != nil {
+			target.serverIsBusy = true
+		}
+	}
+}
+
 func (s *replicaSelector) checkLiveness(bo *retry.Backoffer, accessReplica *replica) livenessState {
 	store := accessReplica.store
 	liveness := store.requestLiveness(bo, s.regionCache)
@@ -1306,8 +1324,9 @@ func (s *replicaSelector) canFallback2Follower() bool {
 	if !ok {
 		return false
 	}
-	if !state.isStaleRead {
-		return false
+	if state.isStaleRead {
+		// fallback to follower if it is stale reads
+		return true
 	}
 	// can fallback to follower only when the leader is exhausted.
 	return state.lastIdx == state.leaderIdx && state.IsLeaderExhausted(s.replicas[state.leaderIdx])
@@ -2127,6 +2146,8 @@ func (s *RegionRequestSender) onRegionError(bo *retry.Backoffer, ctx *RPCContext
 			if s.replicaSelector.onReadReqConfigurableTimeout(req) {
 				return true, nil
 			}
+		} else if s.replicaSelector != nil {
+			s.replicaSelector.onServerIsBusy(req)
 		}
 		logutil.Logger(bo.GetCtx()).Debug(
 			"tikv reports `ServerIsBusy` retry later",


Test patch:
diff --git a/internal/locate/region_request3_test.go b/internal/locate/region_request3_test.go
index c7c7553..cb0b3cd 100644
--- a/internal/locate/region_request3_test.go
+++ b/internal/locate/region_request3_test.go
@@ -1291,9 +1291,6 @@ func (s *testRegionRequestToThreeStoresSuite) TestDoNotTryUnreachableLeader() {
 	follower, _, _, _ := region.FollowerStorePeer(regionStore, 0, &storeSelectorOp{})
 
 	s.regionRequestSender.client = &fnClient{fn: func(ctx context.Context, addr string, req *tikvrpc.Request, timeout time.Duration) (response *tikvrpc.Response, err error) {
-		if req.StaleRead && addr == follower.addr {
-			return &tikvrpc.Response{Resp: &kvrpcpb.GetResponse{RegionError: &errorpb.Error{DataIsNotReady: &errorpb.DataIsNotReady{}}}}, nil
-		}
 		return &tikvrpc.Response{Resp: &kvrpcpb.GetResponse{
 			Value: []byte(addr),
 		}}, nil
diff --git a/internal/locate/region_request_state_test.go b/internal/locate/region_request_state_test.go
index 0fc5c27..74d4d5e 100644
--- a/internal/locate/region_request_state_test.go
+++ b/internal/locate/region_request_state_test.go
@@ -333,7 +333,7 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       Some(true),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     None[bool](),
 			followerSuccessReplica:  []string{"z2"},
@@ -379,7 +379,7 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       Some(false),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     Some(false),
 			followerSuccessReplica:  []string{"z2"},
@@ -443,11 +443,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       Some(false),
 			leaderSuccessReplica:    []string{"z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     Some(false),
 			followerSuccessReplica:  []string{"z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 		{
 			do:                      leaderServerIsBusy,
@@ -456,11 +456,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       Some(false),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     Some(false),
 			followerSuccessReplica:  []string{"z2", "z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 		{
 			do:                      leaderServerIsBusy,
@@ -469,11 +469,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       Some(false),
 			leaderSuccessReplica:    []string{"z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     Some(false),
 			followerSuccessReplica:  []string{"z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 		{
 			do:                      leaderDown,