retry stale read as stale if leader is not accessible

Signed-off-by: rishabh_mittal <[email protected]>
tikv · Dec 20, 2024 · ee1cfd2 · ee1cfd2
1 parent c5d92ba
commit ee1cfd2
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 18 deletions.
diff --git a/internal/locate/region_request3_test.go b/internal/locate/region_request3_test.go
@@ -1412,9 +1412,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
@@ -17,6 +17,8 @@ package locate
 import (
 	"context"
 	"fmt"
+	"reflect"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync/atomic"
@@ -322,7 +324,7 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.Some(false),
 			followerSuccessReplica:  []string{"z2"},
@@ -333,7 +335,7 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.None[bool](),
 			followerSuccessReplica:  []string{"z2"},
@@ -344,7 +346,7 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.None[bool](),
 			followerSuccessReplica:  []string{"z2"},
@@ -432,11 +434,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z2", "z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.Some(true),
 			followerSuccessReplica:  []string{"z2", "z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 		{
 			do:                      leaderDown,
@@ -445,11 +447,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.Some(true),
 			followerSuccessReplica:  []string{"z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 		{
 			do:                      leaderDown,
@@ -458,11 +460,11 @@ func TestRegionCacheStaleRead(t *testing.T) {
 			leaderRegionValid:       true,
 			leaderAsyncReload:       util.Some(true),
 			leaderSuccessReplica:    []string{"z3"},
-			leaderSuccessReadType:   SuccessFollowerRead,
+			leaderSuccessReadType:   SuccessStaleRead,
 			followerRegionValid:     true,
 			followerAsyncReload:     util.Some(true),
 			followerSuccessReplica:  []string{"z3"},
-			followerSuccessReadType: SuccessFollowerRead,
+			followerSuccessReadType: SuccessStaleRead,
 		},
 	}
 	tests := []func(*testRegionCacheStaleReadSuite, *RegionCacheTestCase){
@@ -562,6 +564,8 @@ func testStaleRead(s *testRegionCacheStaleReadSuite, r *RegionCacheTestCase, zon
 			}
 		}
 	} else {
+		funcName := runtime.FuncForPC(reflect.ValueOf(r.do).Pointer()).Name()
+		fmt.Printf("Function name of r.do(): %s\n", funcName)
 		s.Equal(r.followerSuccessReadType, successReadType)
 		for _, z := range r.followerSuccessReplica {
 			if z == successZone {

diff --git a/internal/locate/replica_selector.go b/internal/locate/replica_selector.go
@@ -307,8 +307,11 @@ func (s *ReplicaSelectMixedStrategy) next(selector *replicaSelector) *replica {
 func (s *ReplicaSelectMixedStrategy) canSendReplicaRead(selector *replicaSelector) bool {
 	replicas := selector.replicas
 	replica := replicas[s.leaderIdx]
-	if replica.hasFlag(deadlineErrUsingConfTimeoutFlag) || replica.hasFlag(serverIsBusyFlag) {
-		// don't overwhelm the leader if it is busy
+	liveness := replica.store.getLivenessState()
+	epochStale := replica.isEpochStale()
+
+	if liveness != reachable || epochStale || replica.hasFlag(deadlineErrUsingConfTimeoutFlag) || replica.hasFlag(serverIsBusyFlag) {
+		// don't overwhelm the leader is busy, timeout, not accessible or stale epoch.
 		return false
 	}
 	return true

diff --git a/internal/locate/replica_selector_test.go b/internal/locate/replica_selector_test.go
@@ -898,8 +898,8 @@ func TestReplicaReadAccessPathByCase2(t *testing.T) {
 		expect: &accessPathResult{
 			accessPath: []string{
 				"{addr: store1, replica-read: false, stale-read: true}",
-				"{addr: store2, replica-read: true, stale-read: false}",
-				"{addr: store3, replica-read: true, stale-read: false}"},
+				"{addr: store2, replica-read: false, stale-read: true}",
+				"{addr: store3, replica-read: false, stale-read: true}"},
 			respErr:         "",
 			respRegionError: fakeEpochNotMatch,
 			backoffCnt:      3,
@@ -2125,7 +2125,7 @@ func TestReplicaReadAccessPathByStaleReadCase(t *testing.T) {
 				accessPath: []string{
 					"{addr: store2, replica-read: false, stale-read: true}",
 					"{addr: store1, replica-read: false, stale-read: false}",
-					"{addr: store3, replica-read: true, stale-read: false}",
+					"{addr: store3, replica-read: false, stale-read: true}",
 				},
 				respErr:         "",
 				respRegionError: nil,
@@ -2144,7 +2144,7 @@ func TestReplicaReadAccessPathByStaleReadCase(t *testing.T) {
 			beforeRun: func() { /* don't resetStoreState */ },
 			expect: &accessPathResult{
 				accessPath: []string{
-					"{addr: store3, replica-read: true, stale-read: false}",
+					"{addr: store3, replica-read: false, stale-read: true}",
 				},
 				respErr:         "",
 				respRegionError: fakeEpochNotMatch,