Repository: tikv/client-go
Pull request #1673 (2025-06-20 08:00:33)

snapshot: set `ReplicaRead` to false when `ReplicaReadType` fallbacks to `ReplicaReadLeader` (#1663)
Cherry pick #1663 to tidb-8.5

---

ref pingcap/tidb#61745

The [`replicaReadAdjuster`](https://github.com/tikv/client-go/blob/96b6487fac411fea3b4e15170d419d7b8307d6f2/txnkv/txnsnapshot/snapshot.go#L147-L148) can choose leader and follower replica after a request is created.
When the request is created as a replica-read request but fallback to leader read by `replicaReadAdjuster`, we need to set the [`replica_read`](https://github.com/pingcap/kvproto/blob/dc99a8f6e3487e3a73013c2e7d7cf9d5cbb55c45/proto/kvrpcpb.proto#L861) to false as well. Unless it can perform a meaningless read-index in leader and harm the performance.

## Before this PR

![image](https://github.com/user-attachments/assets/ce29517a-2c8c-47a2-9849-c869278eeee9)

Many read-index proposals.

![image](https://github.com/user-attachments/assets/fe8caa05-3f4c-4bd9-88c7-651872607887)

## This PR

![image](https://github.com/user-attachments/assets/53b73a55-689e-4436-b137-f1e8814fc496)

Few read-index proposals.

![image](https://github.com/user-attachments/assets/e935cbe7-73ed-4dcf-8076-3e7df0c2764c)


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

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign [innerr](https://github.com/innerr) for approval. For more information see [the Code Review Process](https://book.prow.tidb.net/#/workflows/pr).
**Please ensure that each of them provides their approval before proceeding.**

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/master/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":["innerr"]} -->
ti-chi-bot[bot]: [LGTM Timeline notifier]
---
Timeline:

- `2025-06-30 07:41:10.482232131 +0000 UTC m=+1294323.205411112`: :ballot_box_with_check: agreed by [glorv](https://github.com/glorv).
ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

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

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

The pull request process is described [here](https://book.prow.tidb.net/#/workflows/pr)

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

- ~~[OWNERS](https://github.com/tikv/client-go/blob/tidb-8.5/OWNERS)~~ [glorv]

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":[]} -->

Patch:
diff --git a/examples/gcworker/go.mod b/examples/gcworker/go.mod
index 99cddaa..8d4b416 100644
--- a/examples/gcworker/go.mod
+++ b/examples/gcworker/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/rawkv/go.mod b/examples/rawkv/go.mod
index 8a29142..8d8bd75 100644
--- a/examples/rawkv/go.mod
+++ b/examples/rawkv/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/1pc_txn/go.mod b/examples/txnkv/1pc_txn/go.mod
index 593f105..42dae0a 100644
--- a/examples/txnkv/1pc_txn/go.mod
+++ b/examples/txnkv/1pc_txn/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/async_commit/go.mod b/examples/txnkv/async_commit/go.mod
index b3693c8..4c9a14b 100644
--- a/examples/txnkv/async_commit/go.mod
+++ b/examples/txnkv/async_commit/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/delete_range/go.mod b/examples/txnkv/delete_range/go.mod
index 6cf4613..7f8559e 100644
--- a/examples/txnkv/delete_range/go.mod
+++ b/examples/txnkv/delete_range/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/go.mod b/examples/txnkv/go.mod
index d398b7a..8e9ff96 100644
--- a/examples/txnkv/go.mod
+++ b/examples/txnkv/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/pessimistic_txn/go.mod b/examples/txnkv/pessimistic_txn/go.mod
index 5e90cca..40b9887 100644
--- a/examples/txnkv/pessimistic_txn/go.mod
+++ b/examples/txnkv/pessimistic_txn/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/examples/txnkv/unsafedestoryrange/go.mod b/examples/txnkv/unsafedestoryrange/go.mod
index 030c399..027824a 100644
--- a/examples/txnkv/unsafedestoryrange/go.mod
+++ b/examples/txnkv/unsafedestoryrange/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
-	github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c // indirect
+	github.com/pingcap/errors v0.11.5-0.20241219054535-6b8c588c3122 // indirect
 	github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c // indirect
 	github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d // indirect
 	github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 // indirect
diff --git a/internal/locate/replica_selector.go b/internal/locate/replica_selector.go
index c906454..dfd92bf 100644
--- a/internal/locate/replica_selector.go
+++ b/internal/locate/replica_selector.go
@@ -491,7 +491,7 @@ func (s *replicaSelector) onFlashbackInProgress(req *tikvrpc.Request) bool {
 		req.BusyThresholdMs = 0
 		s.busyThreshold = 0
 		s.replicaReadType = kv.ReplicaReadLeader
-		req.ReplicaReadType = kv.ReplicaReadLeader
+		req.SetReplicaReadType(kv.ReplicaReadLeader)
 		return true
 	}
 	return false
diff --git a/tikvrpc/tikvrpc.go b/tikvrpc/tikvrpc.go
index 96a480c..46e952b 100644
--- a/tikvrpc/tikvrpc.go
+++ b/tikvrpc/tikvrpc.go
@@ -293,6 +293,14 @@ func NewReplicaReadRequest(typ CmdType, pointer interface{}, replicaReadType kv.
 	return req
 }
 
+func (req *Request) SetReplicaReadType(replicaReadType kv.ReplicaReadType) {
+	if req == nil {
+		return
+	}
+	req.ReplicaRead = replicaReadType.IsFollowerRead()
+	req.ReplicaReadType = replicaReadType
+}
+
 // GetReplicaReadSeed returns ReplicaReadSeed pointer.
 func (req *Request) GetReplicaReadSeed() *uint32 {
 	if req != nil {
diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go
index e49637a..d7a268e 100644
--- a/txnkv/txnsnapshot/snapshot.go
+++ b/txnkv/txnsnapshot/snapshot.go
@@ -460,7 +460,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys,
 			if op != nil {
 				ops = append(ops, op)
 			}
-			req.ReplicaReadType = readType
+			req.SetReplicaReadType(readType)
 		}
 		resp, _, _, err := cli.SendReqCtx(bo, req, batch.region, timeout, tikvrpc.TiKV, "", ops...)
 		if err != nil {
@@ -699,7 +699,7 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([]
 		if op != nil {
 			ops = append(ops, op)
 		}
-		req.ReplicaReadType = readType
+		req.SetReplicaReadType(readType)
 	}
 
 	var firstLock *txnlock.Lock


Test patch:
diff --git a/integration_tests/snapshot_test.go b/integration_tests/snapshot_test.go
index aee2095..b81bb20 100644
--- a/integration_tests/snapshot_test.go
+++ b/integration_tests/snapshot_test.go
@@ -44,12 +44,15 @@ import (
 
 	"github.com/pingcap/failpoint"
 	"github.com/pingcap/kvproto/pkg/kvrpcpb"
+	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/suite"
-	"github.com/tikv/client-go/v2/error"
+	tikverr "github.com/tikv/client-go/v2/error"
+	"github.com/tikv/client-go/v2/kv"
 	"github.com/tikv/client-go/v2/oracle"
 	"github.com/tikv/client-go/v2/tikv"
 	"github.com/tikv/client-go/v2/tikvrpc"
+	"github.com/tikv/client-go/v2/tikvrpc/interceptor"
 	"github.com/tikv/client-go/v2/txnkv"
 	"github.com/tikv/client-go/v2/txnkv/transaction"
 	"github.com/tikv/client-go/v2/txnkv/txnsnapshot"
@@ -163,7 +166,7 @@ func (s *testSnapshotSuite) TestSnapshotCache() {
 	_, err = snapshot.Get(context.Background(), []byte("a"))
 	s.Nil(err)
 	_, err = snapshot.Get(context.Background(), []byte("b"))
-	s.True(error.IsErrNotFound(err))
+	s.True(tikverr.IsErrNotFound(err))
 
 	s.Nil(failpoint.Enable("tikvclient/snapshot-get-cache-fail", `return(true)`))
 	ctx := context.WithValue(context.Background(), "TestSnapshotCache", true)
@@ -173,14 +176,14 @@ func (s *testSnapshotSuite) TestSnapshotCache() {
 	s.Nil(err)
 	s.Equal([]byte("x"), value)
 	_, err = snapshot.Get(ctx, []byte("y"))
-	s.True(error.IsErrNotFound(err))
+	s.True(tikverr.IsErrNotFound(err))
 
 	// check cache from Get
 	value, err = snapshot.Get(ctx, []byte("a"))
 	s.Nil(err)
 	s.Equal([]byte("a"), value)
 	_, err = snapshot.Get(ctx, []byte("b"))
-	s.True(error.IsErrNotFound(err))
+	s.True(tikverr.IsErrNotFound(err))
 
 	s.Nil(failpoint.Disable("tikvclient/snapshot-get-cache-fail"))
 }
@@ -228,7 +231,7 @@ func (s *testSnapshotSuite) TestSkipLargeTxnLock() {
 	txn1 := s.beginTxn()
 	// txn1 is not blocked by txn in the large txn protocol.
 	_, err = txn1.Get(ctx, x)
-	s.True(error.IsErrNotFound(err))
+	s.True(tikverr.IsErrNotFound(err))
 
 	res, err := toTiDBTxn(&txn1).BatchGet(ctx, toTiDBKeys([][]byte{x, y, []byte("z")}))
 	s.Nil(err)
@@ -260,7 +263,7 @@ func (s *testSnapshotSuite) TestPointGetSkipTxnLock() {
 	s.Equal(committer.GetPrimaryKey(), x)
 	// Point get secondary key. Shouldn't be blocked by the lock and read old data.
 	_, err = snapshot.Get(ctx, y)
-	s.True(error.IsErrNotFound(err))
+	s.True(tikverr.IsErrNotFound(err))
 	s.Less(time.Since(start), 500*time.Millisecond)
 
 	// Commit the primary key
@@ -420,3 +423,43 @@ func (s *testSnapshotSuite) TestSnapshotCacheBypassMaxUint64() {
 	snapshot.BatchGet(context.Background(), [][]byte{[]byte("y"), []byte("z")})
 	s.Empty(snapshot.SnapCache())
 }
+
+func (s *testSnapshotSuite) TestReplicaReadAdjuster() {
+	_, err := s.store.SplitRegions(context.Background(), [][]byte{[]byte("y1")}, false, nil)
+	s.Nil(err)
+	for _, hit := range []bool{true, false} {
+		txn := s.beginTxn()
+
+		// check the replica read type
+		fn := func(next interceptor.RPCInterceptorFunc) interceptor.RPCInterceptorFunc {
+			return func(target string, req *tikvrpc.Request) (*tikvrpc.Response, error) {
+				// when the request is fallback to leader read, the ReplicaRead should be false to avoid read-index in store.
+				s.Equal(hit, req.ReplicaRead)
+				if hit {
+					s.Equal(kv.ReplicaReadMixed, req.ReplicaReadType)
+				} else {
+					s.Equal(kv.ReplicaReadLeader, req.ReplicaReadType)
+				}
+				return next(target, req)
+			}
+		}
+		txn.SetRPCInterceptor(interceptor.NewRPCInterceptor("check-req", fn))
+
+		// set up the replica read and adaptive read
+		txn.GetSnapshot().SetReplicaRead(kv.ReplicaReadMixed)
+		txn.GetSnapshot().SetReplicaReadAdjuster(func(int) (tikv.StoreSelectorOption, kv.ReplicaReadType) {
+			if hit {
+				return tikv.WithMatchLabels([]*metapb.StoreLabel{
+					{
+						Key:   tikv.DCLabelKey,
+						Value: "dc1",
+					},
+				}), kv.ReplicaReadMixed
+			}
+			return nil, kv.ReplicaReadLeader
+		})
+		txn.Get(context.Background(), []byte("x"))
+		txn.BatchGet(context.Background(), [][]byte{[]byte("y"), []byte("z")})
+		txn.Rollback()
+	}
+}
diff --git a/internal/locate/replica_selector_test.go b/internal/locate/replica_selector_test.go
index 0002716..ae030cd 100644
--- a/internal/locate/replica_selector_test.go
+++ b/internal/locate/replica_selector_test.go
@@ -833,7 +833,7 @@ func TestReplicaReadAccessPathByCase2(t *testing.T) {
 		expect: &accessPathResult{
 			accessPath: []string{
 				"{addr: store2, replica-read: true, stale-read: false}",
-				"{addr: store1, replica-read: true, stale-read: false}"},
+				"{addr: store1, replica-read: false, stale-read: false}"},
 			respErr:         "",
 			respRegionError: nil,
 			backoffCnt:      0,