coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request by tiancaiamao · Pull Request #19166 · tikv/tikv

tiancaiamao · 2025-12-04T13:10:39Z

What is changed and how it works?

What's Changed:

change KVPair to KvPairEntry to include the commit ts information
pass load_commit_ts from top side down
no protocol change, for coprocessor request, DAG contains the executors like TableScan, and also column info are available, there is an extra EXTRA_COMMIT_TS_COL_ID = -5 column for the _tidb_commit_ts
In table scan executor, when it detects EXTRA_COMMIT_TS_COL_ID, pass load_commit_ts = true to get the extra commit ts, fill data to that _tidb_commit_ts column
In MVCC scanner, return commit_ts by ValueEntry

coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request

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

support return _tidb_commit_ts column for TableScan coprocessor request

ti-chi-bot · 2025-12-04T13:10:50Z

Hi @tiancaiamao. 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.

…quest Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

you06 · 2025-12-10T11:06:21Z

/ok-to-test

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

tiancaiamao · 2025-12-11T02:11:42Z

/hold

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

tiancaiamao · 2025-12-11T04:50:34Z

/retest

tiancaiamao · 2025-12-11T06:55:18Z

/unhold

lcwangchao · 2025-12-12T05:20:59Z

I use codex to review this code, it seems no obvious bug for this PR

No regressions or functional issues surfaced while reviewing the changes for commit timestamp loading; interfaces were updated consistently and the new logic preserves existing
behavior when the flag is disabled.

lcwangchao · 2025-12-12T05:26:57Z

components/tidb_query_executors/src/table_scan_executor.rs

+                columns[*idx]
+                    .mut_decoded()
+                    .push_int(Some(ts.into_inner() as i64));
+                self.is_column_filled[*idx] = true;


Could we return an error when EXTRA_COMMIT_TS_COL_ID but commit_ts is None? I think it is unexpected

lcwangchao · 2025-12-12T05:36:03Z

src/storage/txn/store.rs

        assert_eq!(result, expect, "expect {:?}, but got {:?}", expect, result);
+
+        // cover load commit ts
+        let mut scanner1 = snapshot_store


Could we also add some tests to test access_locks should be ignored when requiring commit ts?

cfzjywxk · 2025-12-15T02:28:00Z

components/tidb_query_executors/src/table_scan_executor.rs

+                columns[*idx]
+                    .mut_decoded()
+                    .push_int(Some(ts.into_inner() as i64));
+                self.is_column_filled[*idx] = true;


src/storage/mvcc/reader/scanner/mod.rs

cfzjywxk

rest LGTM

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

lcwangchao · 2025-12-16T02:00:06Z

LGTM

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ti-chi-bot · 2025-12-16T02:12:20Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,ekexium]

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 · 2025-12-16T02:12:22Z

[LGTM Timeline notifier]

Timeline:

2025-12-15 02:30:42.010593216 +0000 UTC m=+1440186.824370788: ☑️ agreed by cfzjywxk.
2025-12-16 02:12:21.365200347 +0000 UTC m=+1525486.178977929: ☑️ agreed by ekexium.

…ocessor request (tikv#19166) close pingcap/tidb#64949 coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ti-chi-bot bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 4, 2025

coprocessor: support return _tidb_commit_ts column for coprocessor re…

f92227c

…quest Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

tiancaiamao force-pushed the table_scan_commit_ts branch from 11da24a to f92227c Compare December 4, 2025 13:13

ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. and removed dco-signoff: no Indicates the PR's author has not signed dco. labels Dec 4, 2025

fix panic

d20880f

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

tiancaiamao changed the title ~~coprocessor: support return _tidb_commit_ts column for coprocessor request (WIP)~~ coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request (WIP) Dec 10, 2025

tiancaiamao changed the title ~~coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request (WIP)~~ coprocessor: support return _tidb_commit_ts column for TableScan coprocessor request Dec 10, 2025

build test

351851b

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Dec 10, 2025

erase warnings

e842dd5

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2025

remove test

0d662c4

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2025

lcwangchao reviewed Dec 12, 2025

View reviewed changes

cfzjywxk reviewed Dec 15, 2025

View reviewed changes

cfzjywxk approved these changes Dec 15, 2025

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Dec 15, 2025

address comment

4b9e50a

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

lcwangchao mentioned this pull request Dec 16, 2025

Active-Active Replication Feature pingcap/tidb#64281

Open

95 tasks

make golint happy

f9d90ee

Signed-off-by: tiancaiamao <tiancaiamao@gmail.com>

ekexium approved these changes Dec 16, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Dec 16, 2025

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 16, 2025

ti-chi-bot bot merged commit 15c08c0 into tikv:feature/active-active Dec 16, 2025
5 checks passed

tiancaiamao deleted the table_scan_commit_ts branch December 16, 2025 02:58

tiancaiamao mentioned this pull request Dec 18, 2025

unistore: support return _tidb_commit_ts for coprocessor request | tidb-test=feature/active-active tikv=feature/active-active pingcap/tidb#65107

Merged

13 tasks

lcwangchao mentioned this pull request Dec 25, 2025

*: support returning commit ts for Get / BatchGet / Coprocessor #19230

Merged

9 tasks

Conversation

tiancaiamao commented Dec 4, 2025 • 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!

ti-chi-bot bot commented Dec 4, 2025

Uh oh!

you06 commented Dec 10, 2025

Uh oh!

tiancaiamao commented Dec 11, 2025

Uh oh!

tiancaiamao commented Dec 11, 2025

Uh oh!

tiancaiamao commented Dec 11, 2025

Uh oh!

lcwangchao commented Dec 12, 2025

Uh oh!

lcwangchao Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

cfzjywxk Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

tiancaiamao Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

lcwangchao Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

tiancaiamao Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

cfzjywxk Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cfzjywxk left a comment

Choose a reason for hiding this comment

Uh oh!

lcwangchao commented Dec 16, 2025

Uh oh!

ti-chi-bot bot commented Dec 16, 2025

Uh oh!

ti-chi-bot bot commented Dec 16, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tiancaiamao commented Dec 4, 2025 •

edited

Loading