tici: support versioned lookup via per-key read ts (range_versions) by wshwsh12 · Pull Request #19302 · tikv/tikv

wshwsh12 · 2026-01-21T06:18:48Z

What is changed and how it works?

Issue Number: Close #19357

What's Changed:

coprocessor: support versioned lookup via per-key read ts (range_versions)

  This introduces a special "versioned lookup" read path for coprocessor DAG requests:

  - When `coppb::Request.range_versions` is empty, behavior is unchanged.
  - When `range_versions` is provided:
    - `ranges.len()` must equal `range_versions.len()`
    - all `ranges` must be point ranges
    - only DAG + TableScan is allowed
    - each `ranges[i]` is read at `range_versions[i]` (per-key read ts), while bypassing txn-related checks:
      - endpoint memory lock checking / max_ts update
      - lock-cf checking in PointGetter
      - RcCheckTs newer-ts conflicts

  Implementation overview:
  - Plumb `range_versions` through Endpoint -> DagHandlerBuilder -> TableScan -> RangesScanner.
  - Add Store/SnapshotStore `get_entry_at_ts` backed by PointGetter with `bypass_txn_check`.
  - Disable coprocessor cache for versioned lookup to avoid unintended cache hits.

  Tests:
  - Add unit tests for validation/guard rails and bypass behaviors.
  - Add executor tests to verify per-range ts usage (including backward scan alignment).
  - Extend coprocessor integration tests for reading older versions.

  Dependencies:
  - Requires kvproto change adding `coppb::Request.range_versions` (kvproto PR: pingcap/kvproto#1356).
    The temporary `[patch]` in `Cargo.toml` can be removed after kvproto is merged.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

coprocessor: support versioned lookup via per-key read ts (range_versions) used by tici

cfzjywxk · 2026-01-21T07:14:03Z

src/storage/mvcc/reader/point_getter.rs

    check_has_newer_ts_data: bool,
+    /// If true, skip all txn-related checks (lock checking, RcCheckTs newer-ts
+    /// check). Used by special read paths (e.g. TiCI versioned lookup).
+    bypass_txn_check: bool,


As I mentioned in previous PR #19019 (comment), the existing semantics of the standard DAG entry are designed to serve transactional coprocessor reads, which require read-write concurrency control and conflict handling.

The bypass_txn_check flag reduces the maintainability of the entire path, as it introduces a bypass transaction check flag within the transaction semantic logic. It is recommended to align it with a similar flag used by the coprocessor endpoint, such as is_versioned_lookup.

cfzjywxk · 2026-01-21T07:17:02Z

src/coprocessor/endpoint.rs

+        let data = req.take_data();
+        let ranges: Vec<_> = req.take_ranges().into();
+        let mut start_ts = req.get_start_ts();
+        let range_versions = req.take_range_versions();


Need comment to explain this is specfially for tici, and the endpoint kv entry of the kv-service.

Another question is, how to ensure it would not be used besides tici, considering the robustness.

Perhaps an request souce in RPC context is really needed. /cc @MyonKeminta @ekexium

coderabbitai · 2026-01-28T12:46:25Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

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

cfzjywxk · 2026-01-30T07:29:12Z

src/server/service/kv.rs

-            if let Some(err @ Error::ClusterIDMisMatch { .. }) = item.server_err {
-                let e = RpcStatus::with_message(RpcStatusCode::INVALID_ARGUMENT, err.to_string());
-                GrpcResult::<(BatchCommandsResponse, WriteFlags)>::Err(GrpcError::RpcFailure(e))
+            if let Some(err) = item.server_err.take() {


Is this unrelated change?

cfzjywxk · 2026-01-30T07:29:20Z

src/server/service/kv.rs

            if let Err(e) = sink.send_all(&mut response_retriever).await {
-                let e = RpcStatus::with_message(RpcStatusCode::INVALID_ARGUMENT, e.to_string());
-                sink.fail(e).await?;
+                match e {


cfzjywxk · 2026-01-30T07:31:05Z

src/storage/mvcc/reader/point_getter.rs

+    /// Defaults to `false`.
+    #[inline]
+    #[must_use]
+    pub(crate) fn is_versioned_lookup(mut self, enabled: bool) -> Self {


Should be set_versioned_lookup.

cfzjywxk · 2026-01-30T07:33:59Z

src/storage/txn/store.rs

+    ///
+    /// WARNING: This method intentionally violates transactional semantics. It
+    /// may read through locks and ignore `RcCheckTs` write conflicts. Do not
+    /// use it for normal reads.


Better to describe the current usage scneario, like TiCI.

cfzjywxk · 2026-01-30T07:38:34Z

components/tidb_query_executors/src/index_lookup_executor.rs

            primary_prefix_column_ids,
-        )
+            None,
+        )?)


Unrelated refactor? Please re-check to remove unrelated refactor if they are generated by AI agent.

All unrelated refactoring have been removed. PTAL again

ChangRui-Ryan · 2026-02-09T03:26:00Z

components/tidb_query_common/src/storage/mod.rs

+    fn get_entry_at_ts(
+        &mut self,
+        is_key_only: bool,
+        load_commit_ts: bool,


In what scenarios do we need to get load_commit_ts?

load_commit_ts belongs to other feature and is not related to versionedlookup. Here we are just preserving the existing behavior.

ChangRui-Ryan · 2026-02-09T06:36:32Z

src/coprocessor/endpoint.rs

            }
        };

        check_memory_locks_for_ranges(&self.concurrency_manager, &self.req_ctx, key_range)?;


Do we need to bypass this based on is_versioned_lookup here?

It’s not needed. check_memory_locks_for_ranges already performs transaction-related checks, which are not required for VersionedLookup

ChangRui-Ryan · 2026-02-09T07:31:46Z

lgtm

Copilot

Pull request overview

This PR introduces a “versioned lookup” coprocessor read path that allows supplying per-key (per-point-range) read timestamps via versioned_ranges, exposed through a dedicated VersionedKv RPC and plumbed through DAG/TableScan down to point-get MVCC reads with txn checks bypassed.

Changes:

Add VersionedKv.versioned_coprocessor RPC and reject versioned_ranges on the existing coprocessor RPCs.
Plumb per-range read timestamps (range_versions) through Endpoint → DAG runner → TableScan/ScanExecutor → RangesScanner → Storage, and add storage APIs to fetch a key at an explicit ts while bypassing txn checks.
Add/extend unit and integration tests covering validation/guardrails and per-range ts behavior.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/integrations/coprocessor/test_select.rs	Adds integration test verifying reading an older version via `versioned_ranges`.
tests/integrations/coprocessor/test_checksum.rs	Wires new `range_versions` option for checksum path (set to `None`).
src/storage/txn/store.rs	Adds `Store::get_entry_at_ts` and implements it for `SnapshotStore` (plus internal helper).
src/storage/mvcc/reader/point_getter.rs	Adds `is_versioned_lookup` flag to bypass lock + RcCheckTs newer-ts checks; adds tests.
src/server/service/kv.rs	Introduces `VersionedKv` RPC handler; rejects `versioned_ranges` on existing coprocessor RPCs.
src/server/server.rs	Registers the new VersionedKv gRPC service.
src/coprocessor/statistics/analyze_context.rs	Passes `range_versions: None` to scanner options.
src/coprocessor/statistics/analyze.rs	Extends table scan builder calls with `range_versions: None`.
src/coprocessor/endpoint.rs	Parses `versioned_ranges`, validates invariants, disables cache, and skips memory-lock checking for versioned lookup.
src/coprocessor/dag/storage_impl.rs	Implements `Storage::get_entry_at_ts` via `Store::get_entry_at_ts`.
src/coprocessor/dag/mod.rs	Plumbs `range_versions` into DAG handler construction.
src/coprocessor/checksum.rs	Passes `range_versions: None` to scanner options.
components/tidb_query_executors/src/util/scan_executor.rs	Validates/reverses `range_versions` and passes them into `RangesScanner`.
components/tidb_query_executors/src/util/mock_executor.rs	Adds stub `get_entry_at_ts` required by the updated Storage trait.
components/tidb_query_executors/src/table_scan_executor.rs	Accepts `range_versions` and enables point-range acceptance when present; adds executor tests.
components/tidb_query_executors/src/runner.rs	Plumbs `range_versions` into executor building/runner construction.
components/tidb_query_executors/src/index_scan_executor.rs	Sets `range_versions: None` for index scan executor options.
components/tidb_query_executors/src/index_lookup_executor.rs	Extends table task scan construction with `range_versions: None`.
components/tidb_query_common/src/storage/test_fixture.rs	Implements `get_entry_at_ts` for fixture storage (ts encoded into key) to test plumbing.
components/tidb_query_common/src/storage/scanner.rs	Adds `range_versions` support for point ranges in `RangesScanner`.
components/tidb_query_common/src/storage/mod.rs	Extends `Storage` trait with `get_entry_at_ts` and forwards it for `Box<T>`.
components/test_backup/src/lib.rs	Updates scanner options with `range_versions: None`.
Cargo.lock	Updates kvproto git revision and resulting lockfile resolution changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T03:40:23Z

components/tidb_query_common/src/storage/scanner.rs

                    self.ranges_iter.notify_drained();
                    self.scanned_rows_per_range.push(0);
-                    self.storage
-                        .get_entry(self.is_key_only, self.load_commit_ts, r)?
+                    if let Some(versions) = self.range_versions.as_ref() {
+                        let idx = self.scanned_rows_per_range.len() - 1;
+                        self.storage.get_entry_at_ts(
+                            self.is_key_only,
+                            self.load_commit_ts,
+                            r,
+                            versions[idx],
+                        )?


range_versions indexing is derived from scanned_rows_per_range.len() - 1, but collect_scanned_rows_per_range() drains scanned_rows_per_range between response slices (streaming/paging). After the first slice, this length no longer corresponds to the original range index, so subsequent point ranges can be read using the wrong read_ts. Track an independent monotonically-increasing range index (e.g. current_range_idx incremented on each NewRange) and use that to index range_versions, so stats collection doesn't affect per-range ts selection.

cfzjywxk · 2026-02-10T03:39:15Z

src/coprocessor/endpoint.rs

+            None,
+        );
+
+        let mut req = coppb::Request::default();


The test case should cover both "point range" in which point getter is used and "multi range" in which range scanner is used situations and verify the max_ts is not changed.

multi range can only pass multiple point ranges for testing. Requests with non-point ranges will be rejected directly. I’ve added the test—PTAL again.

cfzjywxk · 2026-02-10T03:41:13Z

src/server/service/kv.rs

 }

+impl<E: Engine, L: LockManager, F: KvFormat> VersionedKv for Service<E, L, F> {
+    fn versioned_coprocessor(


Need to add E2E test sending real RPC requests(like test_kv_service.rs) thourgh tikv client, and verify:

Coprocessor requests with version ranges to coprocessor api would be rejected.

Versioned requests to the versioned api, the results are expected and the max_ts of the TiKV node is not affected.

cfzjywxk · 2026-02-10T03:42:51Z

Besides, please reference the related issues in the description.

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 · 2026-02-10T07:42:36Z

PR 19302 Codex Review

PR: tici: support versioned lookup via per-key read ts (range_versions) #19302
Branch/Head: lookup-3 @ 88e8c3000538650d24b3957b38b1ea1a0ed73a87
Review date: 2026-02-10
Scope: static review only (no build/tests executed)
Assumption: versioned lookup requests are only generated by TiDB and are always DAG + TableScan

Findings (ordered by severity)

1. Improvement (non-blocking): Add an optional server-side defensive guard

Evidence 1: Endpoint only enforces that versioned_ranges must be on DAG requests, but does not explicitly enforce TableScan.
- src/coprocessor/endpoint.rs:248
Evidence 2: when is_versioned_lookup == true, memory lock checking (and related max_ts update path) is skipped.
- src/coprocessor/endpoint.rs:293
Evidence 3: in executor building, range_versions is only consumed by the TypeTableScan branch; TypeIndexScan does not consume it.
- components/tidb_query_executors/src/runner.rs:275
- components/tidb_query_executors/src/runner.rs:298

Impact under the stated assumption

If the "TiDB-only + TableScan-only" contract holds, this does not introduce an actual behavioral issue in practice.
This is mainly a hardening suggestion to reduce risk if future callers or routing change.

Suggested follow-up (optional)

In Endpoint, after parsing DAG, return InvalidArgument when is_versioned_lookup && first_executor != TableScan.
Add one negative test to ensure versioned_ranges + non-TableScan is rejected.

Conclusion

Recommendation: Approve (with assumption)
Reason: with the TiDB contract above, no merge-blocking issue was found; only one optional defensive improvement is suggested.

Signed-off-by: wshwsh12 <793703860@qq.com>

ti-chi-bot · 2026-02-13T07:09:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, gengliqi

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~~ [cfzjywxk,gengliqi]

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 · 2026-02-13T07:09:27Z

[LGTM Timeline notifier]

Timeline:

2026-02-12 06:38:36.830383966 +0000 UTC m=+86869.910374661: ☑️ agreed by cfzjywxk.
2026-02-13 07:09:25.746927509 +0000 UTC m=+175118.826918204: ☑️ agreed by gengliqi.

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the dco. labels Jan 21, 2026

wshwsh12 changed the title ~~tici: support VersionedLookup for tici~~ tici: support versioned lookup via per-key read ts (range_versions) Jan 21, 2026

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 21, 2026

wshwsh12 force-pushed the lookup-3 branch from 0c40ae6 to 3fa4afa Compare January 21, 2026 06:19

wshwsh12 requested a review from cfzjywxk January 21, 2026 06:56

cfzjywxk reviewed Jan 21, 2026

View reviewed changes

wshwsh12 requested a review from cfzjywxk January 22, 2026 06:48

wshwsh12 force-pushed the lookup-3 branch from 2972f40 to 880160b Compare January 26, 2026 13:36

cfzjywxk reviewed Jan 30, 2026

View reviewed changes

wshwsh12 requested a review from cfzjywxk February 2, 2026 09:16

wshwsh12 force-pushed the lookup-3 branch 2 times, most recently from 48d8366 to 6fbe56d Compare February 4, 2026 09:46

ChangRui-Ryan reviewed Feb 9, 2026

View reviewed changes

wshwsh12 requested a review from Copilot February 10, 2026 03:34

Copilot started reviewing on behalf of wshwsh12 February 10, 2026 03:35 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

cfzjywxk reviewed Feb 10, 2026

View reviewed changes

wshwsh12 added 8 commits February 10, 2026 13:54

demo

cdbab1d

Signed-off-by: wshwsh12 <793703860@qq.com>

tidb_query_executors: validate range_versions without extra pass

bae5792

Signed-off-by: wshwsh12 <793703860@qq.com>

storage: rename bypass flag to is_versioned_lookup

bd0a953

Signed-off-by: wshwsh12 <793703860@qq.com>

coprocessor: gate range_versions by request_source

4a421b6

Signed-off-by: wshwsh12 <793703860@qq.com>

demo

a3c1747

Signed-off-by: wshwsh12 <793703860@qq.com>

coprocessor: remove request_source gate for range_versions

8c1df0a

Signed-off-by: wshwsh12 <793703860@qq.com>

server: remove VersionedBatchCoprocessor implementation

f3eccda

Signed-off-by: wshwsh12 <793703860@qq.com>

server: align VersionedKv RPC with kvproto

87df9e0

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 added 15 commits February 10, 2026 13:54

server: ignore range_versions in coprocessor RPC

7cd69bc

Signed-off-by: wshwsh12 <793703860@qq.com>

server: reuse cluster-id reject macro for versioned_coprocessor

59094fc

Signed-off-by: wshwsh12 <793703860@qq.com>

server: reject range_versions in coprocessor

a959a60

Signed-off-by: wshwsh12 <793703860@qq.com>

server: reject range_versions outside VersionedKv

d00ef3a

Signed-off-by: wshwsh12 <793703860@qq.com>

server: simplify range_versions rejection

d4b87ae

Signed-off-by: wshwsh12 <793703860@qq.com>

coprocessor, server: migrate versioned lookup to versioned_ranges

314c179

Signed-off-by: wshwsh12 <793703860@qq.com>

tidb_query_executors, server: address review nits

6fff389

Signed-off-by: wshwsh12 <793703860@qq.com>

coprocessor, server: address review feedback

2135f20

Signed-off-by: wshwsh12 <793703860@qq.com>

server: simplify versioned_ranges validation

bc12d6f

Signed-off-by: wshwsh12 <793703860@qq.com>

server: polish versioned_ranges error handling

fc0278d

Signed-off-by: wshwsh12 <793703860@qq.com>

storage: rename versioned lookup setter

39cb507

Signed-off-by: wshwsh12 <793703860@qq.com>

storage: document versioned lookup store API

74d00cf

Signed-off-by: wshwsh12 <793703860@qq.com>

update dep

d048d96

Signed-off-by: wshwsh12 <793703860@qq.com>

format

9684f8d

Signed-off-by: wshwsh12 <793703860@qq.com>

coprocessor, server: add versioned coprocessor rpc e2e coverage

6428f86

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 force-pushed the lookup-3 branch from fa9df2f to 6428f86 Compare February 10, 2026 07:01

wshwsh12 added 2 commits February 10, 2026 15:13

coprocessor: fix clippy assertion on result state

88e8c30

Signed-off-by: wshwsh12 <793703860@qq.com>

fix clippy

87142c1

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 requested a review from cfzjywxk February 10, 2026 07:48

tests: fix clippy unnecessary map on constructor

93cc2f8

Signed-off-by: wshwsh12 <793703860@qq.com>

cfzjywxk approved these changes Feb 12, 2026

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Feb 12, 2026

gengliqi approved these changes Feb 13, 2026

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Feb 13, 2026

ti-chi-bot bot merged commit 28890b9 into tikv:feature/fts Feb 13, 2026
8 checks passed

Conversation

wshwsh12 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChangRui-Ryan commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cfzjywxk commented Feb 10, 2026

Uh oh!

wshwsh12 commented Feb 10, 2026

PR 19302 Codex Review

Findings (ordered by severity)

1. Improvement (non-blocking): Add an optional server-side defensive guard

Conclusion

Uh oh!

ti-chi-bot bot commented Feb 13, 2026

Uh oh!

ti-chi-bot bot commented Feb 13, 2026

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wshwsh12 commented Jan 21, 2026 •

edited

Loading

coderabbitai bot commented Jan 28, 2026 •

edited

Loading