From 05c71c91db32a686385d6f729fe437a538972941 Mon Sep 17 00:00:00 2001 From: you06 Date: Mon, 21 Aug 2023 20:44:32 +0800 Subject: [PATCH 1/2] fix tryFollower Signed-off-by: you06 --- internal/locate/region_request.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index 47fac7b07..c952a73d2 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -347,7 +347,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} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromKnownLeader: true} return nil, stateChanged{} } if selector.busyThreshold > 0 { @@ -371,7 +371,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} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromKnownLeader: true} } if liveness != reachable { selector.invalidateReplicaStore(selector.targetReplica(), cause) @@ -379,7 +379,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, fromOnNotLeader: true} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromKnownLeader: true} } // tryFollower is the state where we cannot access the known leader @@ -393,8 +393,8 @@ type tryFollower struct { stateBase leaderIdx AccessIndex lastIdx AccessIndex - // fromOnNotLeader indicates whether the state is changed from onNotLeader. - fromOnNotLeader bool + // fromKnownLeader indicates whether the state is changed from onNotLeader. + fromKnownLeader bool labels []*metapb.StoreLabel } @@ -454,7 +454,7 @@ func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) ( if err != nil || rpcCtx == nil { return rpcCtx, err } - if !state.fromOnNotLeader { + if !state.fromKnownLeader { replicaRead := true rpcCtx.contextPatcher.replicaRead = &replicaRead } @@ -464,7 +464,7 @@ func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) ( } func (state *tryFollower) onSendSuccess(selector *replicaSelector) { - if state.fromOnNotLeader { + if state.fromKnownLeader { peer := selector.targetReplica().peer if !selector.region.switchWorkLeaderToPeer(peer) { logutil.BgLogger().Warn("the store must exist", From fd9c884864838b5236b76d3a73deb6cfd6d99f3d Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 22 Aug 2023 11:55:18 +0800 Subject: [PATCH 2/2] address comment Signed-off-by: you06 --- internal/locate/region_request.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index c952a73d2..96d259aac 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -347,7 +347,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, fromKnownLeader: true} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true} return nil, stateChanged{} } if selector.busyThreshold > 0 { @@ -371,7 +371,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, fromKnownLeader: true} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true} } if liveness != reachable { selector.invalidateReplicaStore(selector.targetReplica(), cause) @@ -379,7 +379,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, fromKnownLeader: true} + selector.state = &tryFollower{leaderIdx: state.leaderIdx, lastIdx: state.leaderIdx, fromAccessKnownLeader: true} } // tryFollower is the state where we cannot access the known leader @@ -393,9 +393,9 @@ type tryFollower struct { stateBase leaderIdx AccessIndex lastIdx AccessIndex - // fromKnownLeader indicates whether the state is changed from onNotLeader. - fromKnownLeader bool - labels []*metapb.StoreLabel + // fromAccessKnownLeader indicates whether the state is changed from `accessKnownLeader`. + fromAccessKnownLeader bool + labels []*metapb.StoreLabel } func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) (*RPCContext, error) { @@ -454,7 +454,7 @@ func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) ( if err != nil || rpcCtx == nil { return rpcCtx, err } - if !state.fromKnownLeader { + if !state.fromAccessKnownLeader { replicaRead := true rpcCtx.contextPatcher.replicaRead = &replicaRead } @@ -464,7 +464,7 @@ func (state *tryFollower) next(bo *retry.Backoffer, selector *replicaSelector) ( } func (state *tryFollower) onSendSuccess(selector *replicaSelector) { - if state.fromKnownLeader { + if state.fromAccessKnownLeader { peer := selector.targetReplica().peer if !selector.region.switchWorkLeaderToPeer(peer) { logutil.BgLogger().Warn("the store must exist",