Skip to content

*: support need_commit_ts for TiKV command batch_get_command#19301

Merged
ti-chi-bot[bot] merged 1 commit intotikv:masterfrom
lcwangchao:commit_ts_batch_get_command
Jan 22, 2026
Merged

*: support need_commit_ts for TiKV command batch_get_command#19301
ti-chi-bot[bot] merged 1 commit intotikv:masterfrom
lcwangchao:commit_ts_batch_get_command

Conversation

@lcwangchao
Copy link
Contributor

@lcwangchao lcwangchao commented Jan 21, 2026 •
edited
Loading

What is changed and how it works?

Issue Number: Close #19102

What's Changed:

support `need_commit_ts` for TiKV command batch_get_command

This PR fix two places:

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

None

Copilot AI review requested due to automatic review settings January 21, 2026 05:16
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed do-not-merge/needs-linked-issue dco-signoff: yes Indicates the PR's author has signed the dco. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 21, 2026
@lcwangchao lcwangchao force-pushed the commit_ts_batch_get_command branch from c822848 to 93fb15e Compare January 21, 2026 05:17
@lcwangchao
Copy link
Contributor Author

lcwangchao commented Jan 21, 2026

/retest

Copilot AI reviewed Jan 21, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the implementation for supporting the need_commit_ts flag in TiKV's batch_get_command, fixing two issues from the previous PR #19103:

Changes:

  • Fixed the test case test_batch_get_with_need_commit_ts to properly set the context with committed_locks
  • Extended batch_get_command to return ValueEntry (containing value and optional commit_ts) instead of just raw bytes
  • Updated all response consumers to handle ValueEntry and properly set commit_ts in responses when present

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/integrations/server/kv_service.rs Fixed test to use correct context variable and added assertions for reading through committed_locks
src/storage/mod.rs Changed batch_get_command signature to return ValueEntry, threaded need_commit_ts flag through the call chain, added take_values() helper method, and included comprehensive unit test
src/server/service/batch.rs Updated GetCommandResponseConsumer to extract and set commit_ts from ValueEntry, added unit tests
components/test_storage/src/sync_storage.rs Updated to use take_values() for backward compatibility

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

@lcwangchao
*: support need_commit_ts for TiKV command batch_get_command
34de218 
Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
@lcwangchao lcwangchao force-pushed the commit_ts_batch_get_command branch from 93fb15e to 34de218 Compare January 21, 2026 05:22
@lcwangchao
Copy link
Contributor Author

lcwangchao commented Jan 21, 2026

/retest

1 similar comment
@lcwangchao
Copy link
Contributor Author

lcwangchao commented Jan 21, 2026

/retest

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jan 21, 2026
@ti-chi-bot ti-chi-bot bot added the lgtm label Jan 22, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 22, 2026

[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:

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 ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 22, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 22, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-01-21 07:55:46.478926961 +0000 UTC m=+566974.092883816: ☑️ agreed by ekexium.
  • 2026-01-22 07:37:48.728579091 +0000 UTC m=+652296.342535946: ☑️ agreed by cfzjywxk.

@ti-chi-bot ti-chi-bot bot merged commit 1c030e0 into tikv:master Jan 22, 2026
9 checks passed
@ti-chi-bot ti-chi-bot bot added this to the Pool milestone Jan 22, 2026
@lcwangchao lcwangchao deleted the commit_ts_batch_get_command branch January 22, 2026 07:45
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 22, 2026

@lcwangchao: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-unit-test 34de218 link unknown /test pull-unit-test

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

lcwangchao added a commit to lcwangchao/tikv that referenced this pull request Jan 22, 2026
@lcwangchao
*: support need_commit_ts for TiKV command batch_get_command (tikv#…
fe0447a 
…19301)

close tikv#19102

support `need_commit_ts` for TiKV command batch_get_command

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
@lcwangchao lcwangchao mentioned this pull request Jan 22, 2026
Merged
9 tasks
@lcwangchao lcwangchao mentioned this pull request Jan 22, 2026
Open
95 tasks
BornChanger pushed a commit to BornChanger/tikv that referenced this pull request Jan 24, 2026
@lcwangchao @BornChanger
*: support need_commit_ts for TiKV command batch_get_command (tikv#…
95b7645 
…19301)

close tikv#19102

support `need_commit_ts` for TiKV command batch_get_command

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
lcwangchao added a commit to lcwangchao/tikv that referenced this pull request Jan 29, 2026
@lcwangchao
*: support need_commit_ts for TiKV command batch_get_command (tikv#…
ef7f9d6 
…19301)

close tikv#19102

support `need_commit_ts` for TiKV command batch_get_command

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
@lcwangchao lcwangchao mentioned this pull request Jan 29, 2026
Merged
9 tasks
wshwsh12 pushed a commit to wshwsh12/tikv that referenced this pull request Feb 9, 2026
@lcwangchao @wshwsh12
*: support need_commit_ts for TiKV command batch_get_command (tikv#…
bfddc43 
…19301)

close tikv#19102

support `need_commit_ts` for TiKV command batch_get_command

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@cfzjywxk cfzjywxk cfzjywxk approved these changes

@ekexium ekexium ekexium approved these changes

Assignees

No one assigned

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

Pool

Development

Successfully merging this pull request may close these issues.

Support need_commit_ts for Get / BatchGet requests

4 participants

@lcwangchao @cfzjywxk @ekexium