tikvrpc: clear ReplicaRead when falling back to leader read after meeting a lock

[sanketkedia]

ref pingcap/tidb#71705

What problem does this PR solve?

DisableStaleReadMeetLock() switches a request to leader read after it meets a lock, but leaves ReplicaRead untouched.

TiDB's coprocessor calls it for every lock fallback (meetLockFallback), not only for stale reads, and the retry request has ReplicaRead = true. nextForReplicaReadLeader never resets the flag when it picks the leader, so the leader receives replica_read = true.

TiKV v8.5 drops the In-Memory Engine snapshot for any request with replica_read set. Every lock retry therefore scans RocksDB on the leader even when the region is cached in IME. More details in pingcap/tidb#71705.

What is changed and how does it work?

Clear ReplicaRead together with StaleRead in DisableStaleReadMeetLock(), matching EnableStaleWithMixedReplicaRead(), which already sets it explicitly.

This is safe for leader mode: if the selector later falls back to a follower (busy leader, or a leader deadline error), nextForReplicaReadLeader sets ReplicaRead = true itself.

Tests

• Unit test: TestLockFallbackLeaderReadClearsReplicaRead builds requests in follower, mixed, prefer-leader, learner and stale-read modes, calls DisableStaleReadMeetLock(), and asserts the selector picks the leader with ReplicaRead = false and StaleRead = false. It fails without this change.
• go test ./internal/locate/ ./tikvrpc/ ./txnkv/txnsnapshot/ passes.

🤖 Generated with Claude Code

[ti-chi-bot]

Hi @sanketkedia. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ti-chi-bot]

Welcome @sanketkedia!

It looks like this is your first PR to tikv/client-go 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to tikv/client-go. 😃

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: b7b72a72-9145-42eb-99cd-b9c38d1eaf7a

📥 Commits

Reviewing files that changed from the base of the PR and between 8edb23f and 6bd6ade.

📒 Files selected for processing (2)

• internal/locate/replica_selector_test.go
• tikvrpc/tikvrpc.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

When a key is locked, DisableStaleReadMeetLock now clears ReplicaRead as it switches the request to a leader read. A regression test checks that the selector chooses the leader across five replica-read and stale-read configurations.

Changes

Lock-triggered leader-read fallback

Layer / File(s)	Summary
Fallback request state and leader selection tikvrpc/tikvrpc.go, internal/locate/replica_selector_test.go	DisableStaleReadMeetLock now clears ReplicaRead when it disables stale reads and sets the replica-read type to leader. The regression test checks request flags and leader selection across five configurations.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: ekexium

Merge Risk: ⚪ Minimal · up to 6bd6a

Lock-retry requests will now be sent as plain leader reads instead of carrying a leftover replica-read flag. That should avoid the extra latency reported on TiKV v8.5. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6bd6a

The correction aligns lock retries with their existing leader-read intent. No introduced access-control bypass or privilege increase was identified. Later recovery behavior, concurrent request reuse, and server-side handling remain only partially verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is existing read requests that invoke lock fallback and then enter replica selection. The change does not add a caller or increase request authority in the inspected implementation; maximum deployment-wide exposure through external callers is not established.

Trust Boundaries and Controls

• inferred — The transition aligns request flags with existing leader-read intent rather than introducing a new trust boundary. Intended follower fallbacks remain explicit selector decisions. Whether every server version enforces the expected leader-versus-replica semantics was not verified from server source.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: clearing ReplicaRead when falling back to a leader read after meeting a lock.

• Fix all pre-merge checks with AI

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Connor1996]

LGTM

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Connor1996, LykxSassinator

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Connor1996,LykxSassinator]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-09-30 04:20:23.142240116 +0000 UTC m=+769748.367461213: ☑️ agreed by LykxSassinator.
• 2026-10-01 05:50:01.832072842 +0000 UTC m=+861527.057293949: ☑️ agreed by Connor1996.