client: updating keyspace group must check the previous version by bufferflies · Pull Request #9862 · tikv/pd

bufferflies · 2025-10-22T07:50:19Z

What problem does this PR solve?

Issue Number: Close #6770

[Merged]kv proto: #1361

What is changed and how does it work?

'FindGroupByKeyspaceID' must carry the current keyspace group version, and the server needs to check the memory
state isn't stale, and then return the server keyspace group info.

Check List

Tests

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

Code changes

Side effects

Related changes

Need to cherry-pick to the release branch

Release note

None.

ti-chi-bot · 2025-10-22T07:50:21Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: 童剑 <1045931706@qq.com>

ystaticy · 2025-11-06T14:06:25Z

client/servicediscovery/tso_service_discovery.go

 	// urls are the primary/secondary serving URL
 	urls []string
+
+	// version is used to avoid updating stale info


How about adding a comment to clarify where the version is sourced from and briefly explain its usage?

ystaticy · 2025-11-06T14:15:10Z

client/servicediscovery/tso_service_discovery.go

 		primaryURL:    "",
 		secondaryURLs: make([]string, 0),
 		urls:          make([]string, 0),
+		version:       0,


When the TSO client starts up, does it use version=0 for the initial request?

Furthermore, if this initial request is routed to a TSO service with a lower version, is that behavior expected?

No, this version just works for requesting the keyspace group. Most updates don't relate to this client's keyspace ID, so it doesn't need to be updated.

The higher version of the server side doesn't mean that the client's keyspace group has changed.

Signed-off-by: 童剑 <1045931706@qq.com>

ystaticy · 2025-11-12T02:35:24Z

client/servicediscovery/tso_service_discovery.go

+	// version is used to avoid updating stale info
+	// this version is provided by etcd watcher on the server side
+	// and increases monotonically.
+	// If the new version is less than the current version, it means that the requested tso service hasn't applied the


How about clarifying the source of both the new version and the current version?

ystaticy · 2025-11-12T02:43:50Z

pkg/tso/keyspace_group_manager.go

 	// Being merged will cause the group to be removed from this map eventually if the merge is successful.
 	requestedGroups map[uint32]struct{}
+
+	modVersion uint64


Please add comments for modVersion uint64.

ti-chi-bot · 2025-11-14T03:38:58Z

@ystaticy: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

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.

bufferflies · 2025-11-19T01:47:58Z

/ping @rleungx PTAL again

pkg/tso/keyspace_group_manager.go

Signed-off-by: 童剑 <1045931706@qq.com>

codecov · 2025-11-20T14:24:35Z

Codecov Report

❌ Patch coverage is 76.71233% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.45%. Comparing base (d9219d5) to head (a709f84).
⚠️ Report is 70 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9862      +/-   ##
==========================================
- Coverage   78.70%   78.45%   -0.25%     
==========================================
  Files         492      503      +11     
  Lines       66268    67176     +908     
==========================================
+ Hits        52153    52706     +553     
- Misses      10408    10640     +232     
- Partials     3707     3830     +123

Flag	Coverage Δ
unittests	`78.45% <76.71%> (-0.25%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: 童剑 <1045931706@qq.com>

rleungx · 2025-11-24T05:56:04Z

client/servicediscovery/tso_service_discovery.go


 	keyspaceID := c.GetKeyspaceID()
 	var keyspaceGroup *tsopb.KeyspaceGroup
+	var version uint64


How about unifying the naming, using modRevision and curModRevision?

Signed-off-by: 童剑 <1045931706@qq.com>

ti-chi-bot · 2025-11-27T07:54:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: okJiang, rleungx, ystaticy

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~~ [rleungx]

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-11-27T07:54:35Z

[LGTM Timeline notifier]

Timeline:

2025-11-26 05:11:32.909648438 +0000 UTC m=+679656.558842905: ☑️ agreed by rleungx.
2025-11-27 07:54:33.817140933 +0000 UTC m=+775837.466335390: ☑️ agreed by okJiang.

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies · 2025-11-28T03:57:45Z

/retest

bufferflies · 2025-11-28T04:25:06Z

/test pull-unit-test-next-gen

bufferflies · 2025-11-28T07:33:01Z

/test pull-unit-test-next-gen

bufferflies force-pushed the keyspace/version branch 2 times, most recently from 2f5525f to 1460ce5 Compare October 23, 2025 01:17

request with version

3f7b9ee

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies force-pushed the keyspace/version branch from 1460ce5 to 3f7b9ee Compare October 23, 2025 03:54

bufferflies marked this pull request as ready for review October 23, 2025 04:31

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2025

bufferflies changed the title ~~client: keyspace group update must check the version avoid the stale update~~ client: updating keyspace group must check the previous version Oct 23, 2025

bufferflies added 6 commits October 31, 2025 14:15

request with version

66ba782

Signed-off-by: 童剑 <1045931706@qq.com>

add watch failed test

eb0219b

Signed-off-by: 童剑 <1045931706@qq.com>

resolve conflict

eec042f

Signed-off-by: 童剑 <1045931706@qq.com>

remove unused code

727b351

Signed-off-by: 童剑 <1045931706@qq.com>

Merge branch 'master' of github.com:tikv/pd into keyspace/version

d7835d5

link

a05cff6

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies force-pushed the keyspace/version branch from d34bbed to a05cff6 Compare November 4, 2025 02:34

ystaticy reviewed Nov 6, 2025

View reviewed changes

bufferflies requested review from JmPotato and rleungx November 11, 2025 07:20

add comment for the client version

07950e5

Signed-off-by: 童剑 <1045931706@qq.com>

ystaticy reviewed Nov 12, 2025

View reviewed changes

bufferflies force-pushed the keyspace/version branch 2 times, most recently from d6e0452 to 4234bb8 Compare November 12, 2025 07:42

rleungx reviewed Nov 19, 2025

View reviewed changes

pkg/tso/keyspace_group_manager.go Outdated Show resolved Hide resolved

rename modversion

9ee3aac

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies force-pushed the keyspace/version branch from e091432 to 9ee3aac Compare November 20, 2025 14:15

bufferflies requested a review from rleungx November 21, 2025 02:49

rename mod revision

06d1a1d

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies force-pushed the keyspace/version branch from f4ade90 to 06d1a1d Compare November 21, 2025 03:36

update mod

9ab6802

Signed-off-by: 童剑 <1045931706@qq.com>

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 21, 2025

rleungx reviewed Nov 24, 2025

View reviewed changes

revert version

641aad6

Signed-off-by: 童剑 <1045931706@qq.com>

rleungx approved these changes Nov 26, 2025

View reviewed changes

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

okJiang approved these changes Nov 27, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 27, 2025

lint

a709f84

Signed-off-by: 童剑 <1045931706@qq.com>

bufferflies force-pushed the keyspace/version branch from 895b48c to a709f84 Compare November 28, 2025 02:19

ti-chi-bot bot merged commit 06ae8ba into tikv:master Nov 28, 2025
56 of 62 checks passed

bufferflies added the cse label Dec 24, 2025

Conversation

bufferflies commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented Oct 22, 2025

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!

ti-chi-bot bot commented Nov 14, 2025

Uh oh!

bufferflies commented Nov 19, 2025

Uh oh!

Uh oh!

codecov bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Nov 27, 2025

Uh oh!

ti-chi-bot bot commented Nov 27, 2025

[LGTM Timeline notifier]

Uh oh!

bufferflies commented Nov 28, 2025

Uh oh!

bufferflies commented Nov 28, 2025

Uh oh!

bufferflies commented Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bufferflies commented Oct 22, 2025 •

edited

Loading

codecov bot commented Nov 20, 2025 •

edited

Loading