From 6209394f9f1fefe8f3c275221eb6abfa436f8fd6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sun, 7 Apr 2024 19:24:10 +0800 Subject: [PATCH 1/2] fix issue replica selector v2 not compatible with v1 in mixed mode Signed-off-by: crazycs520 --- internal/locate/replica_selector.go | 20 ++++++++++++-------- internal/locate/replica_selector_test.go | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/internal/locate/replica_selector.go b/internal/locate/replica_selector.go index 5d5a927bb..3a20eac7f 100644 --- a/internal/locate/replica_selector.go +++ b/internal/locate/replica_selector.go @@ -79,6 +79,9 @@ func newReplicaSelectorV2( for _, op := range opts { op(&option) } + if req.ReplicaReadType == kv.ReplicaReadPreferLeader { + WithPerferLeader()(&option) + } return &replicaSelectorV2{ baseReplicaSelector: baseReplicaSelector{ regionCache: regionCache, @@ -166,17 +169,10 @@ func (s *replicaSelectorV2) nextForReplicaReadMixed(req *tikvrpc.Request) { return } } - preferLeader := req.ReplicaReadType == kv.ReplicaReadPreferLeader - if s.attempts > 1 { - if req.ReplicaReadType == kv.ReplicaReadMixed { - // For mixed read retry, prefer retry leader first. - preferLeader = true - } - } strategy := ReplicaSelectMixedStrategy{ leaderIdx: leaderIdx, tryLeader: req.ReplicaReadType == kv.ReplicaReadMixed || req.ReplicaReadType == kv.ReplicaReadPreferLeader, - preferLeader: preferLeader, + preferLeader: s.option.preferLeader, leaderOnly: s.option.leaderOnly, learnerOnly: req.ReplicaReadType == kv.ReplicaReadLearner, labels: s.option.labels, @@ -201,6 +197,14 @@ func (s *replicaSelectorV2) nextForReplicaReadMixed(req *tikvrpc.Request) { req.StaleRead = false req.ReplicaRead = s.isReadOnlyReq } + // Monitor the flows destination if selector is under `ReplicaReadPreferLeader` mode. + if s.option.preferLeader { + if s.target.peer.Id != s.region.GetLeaderPeerID() { + s.target.store.recordReplicaFlowsStats(toFollower) + } else { + s.target.store.recordReplicaFlowsStats(toLeader) + } + } } } diff --git a/internal/locate/replica_selector_test.go b/internal/locate/replica_selector_test.go index bb6cd01a6..add7b7486 100644 --- a/internal/locate/replica_selector_test.go +++ b/internal/locate/replica_selector_test.go @@ -663,6 +663,28 @@ func TestReplicaReadAccessPathByCase(t *testing.T) { }, } s.True(s.runCaseAndCompare(ca)) + + s.changeRegionLeader(3) + ca = replicaSelectorAccessPathCase{ + reqType: tikvrpc.CmdGet, + readType: kv.ReplicaReadMixed, + staleRead: false, + accessErr: []RegionErrorType{ServerIsBusyErr, ServerIsBusyErr, ServerIsBusyErr}, + expect: &accessPathResult{ + accessPath: []string{ + "{addr: store1, replica-read: true, stale-read: false}", + "{addr: store2, replica-read: true, stale-read: false}", + "{addr: store3, replica-read: true, stale-read: false}", + }, + respErr: "", + respRegionError: fakeEpochNotMatch, + backoffCnt: 1, + backoffDetail: []string{"tikvServerBusy+1"}, + regionIsValid: false, + }, + } + s.True(s.runCaseAndCompare(ca)) + s.changeRegionLeader(1) } func TestReplicaReadAccessPathByCase2(t *testing.T) { From e1edb7c4122deb15118dcdfacb904800137b49f9 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 8 Apr 2024 00:39:39 +0800 Subject: [PATCH 2/2] add prefer-leader test case Signed-off-by: crazycs520 --- internal/locate/replica_selector_test.go | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/internal/locate/replica_selector_test.go b/internal/locate/replica_selector_test.go index add7b7486..bdb8325b6 100644 --- a/internal/locate/replica_selector_test.go +++ b/internal/locate/replica_selector_test.go @@ -684,6 +684,47 @@ func TestReplicaReadAccessPathByCase(t *testing.T) { }, } s.True(s.runCaseAndCompare(ca)) + + ca = replicaSelectorAccessPathCase{ + reqType: tikvrpc.CmdGet, + readType: kv.ReplicaReadPreferLeader, + staleRead: false, + accessErr: []RegionErrorType{ServerIsBusyErr, ServerIsBusyErr, ServerIsBusyErr}, + expect: &accessPathResult{ + accessPath: []string{ + "{addr: store3, replica-read: true, stale-read: false}", + "{addr: store1, replica-read: true, stale-read: false}", + "{addr: store2, replica-read: true, stale-read: false}", + }, + respErr: "", + respRegionError: fakeEpochNotMatch, + backoffCnt: 1, + backoffDetail: []string{"tikvServerBusy+1"}, + regionIsValid: false, + }, + } + s.True(s.runCaseAndCompare(ca)) + + ca = replicaSelectorAccessPathCase{ + reqType: tikvrpc.CmdGet, + readType: kv.ReplicaReadPreferLeader, + staleRead: false, + label: &metapb.StoreLabel{Key: "id", Value: "2"}, + accessErr: []RegionErrorType{ServerIsBusyErr, ServerIsBusyErr, ServerIsBusyErr}, + expect: &accessPathResult{ + accessPath: []string{ + "{addr: store2, replica-read: true, stale-read: false}", + "{addr: store3, replica-read: true, stale-read: false}", + "{addr: store1, replica-read: true, stale-read: false}", + }, + respErr: "", + respRegionError: fakeEpochNotMatch, + backoffCnt: 1, + backoffDetail: []string{"tikvServerBusy+1"}, + regionIsValid: false, + }, + } + s.True(s.runCaseAndCompare(ca)) s.changeRegionLeader(1) }