From add7453f48933a826345d9d4ba9a9c5393ab5c5e Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Feb 2024 12:18:19 +0800 Subject: [PATCH 1/2] replica-read request with mixed strategy and with label, should be able to retry all remain replicas Signed-off-by: crazycs520 --- internal/locate/region_request.go | 3 ++- internal/locate/replica_selector_test.go | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index 0bcc6d7345..f33717b9b3 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -766,7 +766,8 @@ func (state *accessFollower) next(bo *retry.Backoffer, selector *replicaSelector // If the leader is also unavailable, we can fallback to the follower and use replica-read flag again, // The remote follower not tried yet, and the local follower can retry without stale-read flag. // If leader tried and received deadline exceeded error, try follower. - if state.isStaleRead || leader.deadlineErrUsingConfTimeout { + // If has labels, some followers's labels may not match, then can't be candidate, need to try follower. + if state.isStaleRead || leader.deadlineErrUsingConfTimeout || len(state.option.labels) > 0 { selector.state = &tryFollower{ leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, diff --git a/internal/locate/replica_selector_test.go b/internal/locate/replica_selector_test.go index 5259903cf9..791451e596 100644 --- a/internal/locate/replica_selector_test.go +++ b/internal/locate/replica_selector_test.go @@ -94,7 +94,7 @@ type accessPathResult struct { regionIsValid bool } -func TestReplicaReadStaleReadAccessPathByCase(t *testing.T) { +func TestReplicaReadAccessPathByCase(t *testing.T) { s := new(testReplicaSelectorSuite) s.SetupTest(t) defer s.TearDownTest() @@ -282,6 +282,27 @@ func TestReplicaReadStaleReadAccessPathByCase(t *testing.T) { }, } s.True(s.runCaseAndCompare(ca)) + + ca = replicaSelectorAccessPathCase{ + reqType: tikvrpc.CmdGet, + readType: kv.ReplicaReadMixed, + staleRead: false, + timeout: time.Second, + label: &metapb.StoreLabel{Key: "id", Value: "2"}, + accessErr: []RegionErrorType{DeadLineExceededErr, ServerIsBusyErr}, + expect: &accessPathResult{ + accessPath: []string{ + "{addr: store2, replica-read: true, stale-read: false}", + "{addr: store1, replica-read: true, stale-read: false}", + "{addr: store3, replica-read: true, stale-read: false}"}, + respErr: "", + respRegionError: nil, + backoffCnt: 1, + backoffDetail: []string{"tikvServerBusy+1"}, + regionIsValid: true, + }, + } + s.True(s.runCaseAndCompare(ca)) } func (s *testReplicaSelectorSuite) runCaseAndCompare(ca2 replicaSelectorAccessPathCase) bool { ca2.run(s) From a1a2baf3bb1d5a81dc4d0fdbb597e672afdbd893 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Feb 2024 20:55:24 +0800 Subject: [PATCH 2/2] address comment Signed-off-by: crazycs520 --- internal/locate/region_request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index f33717b9b3..113d5625f9 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -766,7 +766,7 @@ func (state *accessFollower) next(bo *retry.Backoffer, selector *replicaSelector // If the leader is also unavailable, we can fallback to the follower and use replica-read flag again, // The remote follower not tried yet, and the local follower can retry without stale-read flag. // If leader tried and received deadline exceeded error, try follower. - // If has labels, some followers's labels may not match, then can't be candidate, need to try follower. + // If labels are used, some followers would be filtered by the labels and can't be candidates, they still need to be retried. if state.isStaleRead || leader.deadlineErrUsingConfTimeout || len(state.option.labels) > 0 { selector.state = &tryFollower{ leaderIdx: state.leaderIdx,