client: resource manager service discovery by okJiang · Pull Request #9785 · tikv/pd

okJiang · 2025-09-28T08:51:46Z

What problem does this PR solve?

cherry-pick eadad73
cherry-pick 9411339
Enable service discovery in resource_manager_test.go
remove the related Option from resource manager client

Issue Number: Ref #9737

What is changed and how does it work?

Check List

Tests

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

Code changes

Has the configuration change
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

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

Release note

None.

Signed-off-by: disksing <i@disksing.com>

ti-chi-bot · 2025-09-28T08:51:49Z

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

codecov · 2025-09-28T09:00:01Z

Codecov Report

❌ Patch coverage is 84.91379% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.53%. Comparing base (8139aa6) to head (1ed5031).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9785      +/-   ##
==========================================
+ Coverage   78.40%   78.53%   +0.12%     
==========================================
  Files         518      520       +2     
  Lines       69438    69658     +220     
==========================================
+ Hits        54441    54703     +262     
+ Misses      11045    11004      -41     
+ Partials     3952     3951       -1

Flag	Coverage Δ
unittests	`78.53% <84.91%> (+0.12%)`	⬆️

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.

…scovery

Signed-off-by: okjiang <819421878@qq.com>

client/opt/option.go

Signed-off-by: okjiang <819421878@qq.com>

Signed-off-by: disksing <i@disksing.com>

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2025-12-24T08:34:35Z

/retest

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2026-01-05T01:54:06Z

/retest

Signed-off-by: okjiang <819421878@qq.com>

…lient-discovery

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2026-01-05T05:32:11Z

/retest

rleungx · 2026-01-05T06:24:53Z

client/servicediscovery/resource_manager_service_discovery.go

+func (r *ResourceManagerDiscovery) resetConn(url string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	if url == "" {


How about len(url) == 0?

rleungx · 2026-01-05T06:30:16Z

client/servicediscovery/resource_manager_service_discovery.go

+				r.resetConn(url)
+				revision = newRevision
+			}
+		case <-r.updateServiceURLCh:


What is the difference between <-ticker.C and <-r.updateServiceURLCh?

If the request failed, client will trigger updateServiceURLCh to update url actively.

The handle code does not exist any difference.

Then we can reduce the duplicated code?

done it. ptal~

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2026-01-05T07:02:17Z

/retest

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2026-01-05T08:13:47Z

/retest

rleungx · 2026-01-05T08:48:46Z

client/servicediscovery/resource_manager_service_discovery.go

+}
+
+// ScheduleUpateServiceURL schedules an update of the service URL.
+func (r *ResourceManagerDiscovery) ScheduleUpateServiceURL() {


Suggested change

func (r *ResourceManagerDiscovery) ScheduleUpateServiceURL() {

func (r *ResourceManagerDiscovery) ScheduleUpdateServiceURL() {

Signed-off-by: okjiang <819421878@qq.com>

rleungx

The rest LGTM

rleungx · 2026-01-05T10:01:32Z

client/inner_client.go

 	}
 }

+func shouldScheduleResourceManagerServiceURLUpdate(err error) bool {


The function name is too long

updated to shouldUpdateRMURL

Signed-off-by: okjiang <819421878@qq.com>

okJiang · 2026-01-06T03:38:15Z

/retest

okJiang · 2026-01-06T04:02:14Z

/retest

Signed-off-by: okjiang <819421878@qq.com>

ti-chi-bot · 2026-01-06T05:52:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: disksing, niubell, rleungx

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~~ [disksing,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 · 2026-01-06T05:52:39Z

[LGTM Timeline notifier]

Timeline:

2025-12-26 08:25:36.270819475 +0000 UTC m=+2411881.084597037: ☑️ agreed by disksing.
2026-01-06 05:52:37.855253976 +0000 UTC m=+681513.673562418: ☑️ agreed by rleungx.

client: resource manager service discovery (tikv#391)

eadad73

Signed-off-by: disksing <i@disksing.com>

okJiang added 6 commits October 9, 2025 15:17

Merge branch 'master' of https://github.com/tikv/pd into rm-client-di…

52f62c0

…scovery

Merge branch 'master' of github.com:tikv/pd into rm-client-discovery

543be8d

support TestServerPrimaryChange

d3c9b27

Signed-off-by: okjiang <819421878@qq.com>

enable register and member test

012ffc9

Signed-off-by: okjiang <819421878@qq.com>

fallback to call pd server if can't find resource manager

584a0d4

Signed-off-by: okjiang <819421878@qq.com>

save

914d163

Signed-off-by: okjiang <819421878@qq.com>

okJiang marked this pull request as ready for review December 19, 2025 04:17

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 Dec 19, 2025

okJiang added 2 commits December 19, 2025 17:07

revert some test

e0336c4

Signed-off-by: okjiang <819421878@qq.com>

Merge branch 'master' of github.com:tikv/pd into rm-client-discovery

adbac15

Signed-off-by: okjiang <819421878@qq.com>

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

rleungx reviewed Dec 22, 2025

View reviewed changes

client/opt/option.go Outdated Show resolved Hide resolved

okJiang and others added 5 commits December 24, 2025 14:56

add test and remove option

d9ebeea

Signed-off-by: okjiang <819421878@qq.com>

client: update resource manager serivce url after error (tikv#436)

9411339

Signed-off-by: disksing <i@disksing.com>

add comment

5d2fde2

Signed-off-by: okjiang <819421878@qq.com>

update log level

480a01d

Signed-off-by: okjiang <819421878@qq.com>

fix lint

17f134e

Signed-off-by: okjiang <819421878@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 Dec 24, 2025

deploy tso server

52ef981

Signed-off-by: okjiang <819421878@qq.com>

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

okJiang added 6 commits January 5, 2026 10:10

add switch test

5e3cdf1

Signed-off-by: okjiang <819421878@qq.com>

Merge branch 'rm-client-discovery' of github.com:okJiang/pd into rm-c…

933dc3f

…lient-discovery

fix lint

ccf8aca

Signed-off-by: okjiang <819421878@qq.com>

fix lint

94fa471

Signed-off-by: okjiang <819421878@qq.com>

fix comment and enhance unit test

1cea5b6

Signed-off-by: okjiang <819421878@qq.com>

add more check

9687b2b

Signed-off-by: okjiang <819421878@qq.com>

rleungx reviewed Jan 5, 2026

View reviewed changes

fix comment

6b931f4

Signed-off-by: okjiang <819421878@qq.com>

fix typo

7a05484

Signed-off-by: okjiang <819421878@qq.com>

rleungx reviewed Jan 5, 2026

View reviewed changes

add error handling for resource manager service URL updates

f2c0e74

Signed-off-by: okjiang <819421878@qq.com>

rleungx reviewed Jan 5, 2026

View reviewed changes

fix comment: rename

d32d809

Signed-off-by: okjiang <819421878@qq.com>

fix typo

1ed5031

Signed-off-by: okjiang <819421878@qq.com>

rleungx approved these changes Jan 6, 2026

View reviewed changes

ti-chi-bot bot added the lgtm label Jan 6, 2026

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 6, 2026

ti-chi-bot bot merged commit 757acdf into tikv:master Jan 6, 2026
31 checks passed

This was referenced Apr 1, 2026

[Client/Service Discovery] RM service discovery in client (70e5fb6f) #10528

Closed

70e5fb6f resource manager service discovery #10537

Closed

c300ffdf update resource manager service URL after error #10538

Closed

	func (r *ResourceManagerDiscovery) ScheduleUpateServiceURL() {
	func (r *ResourceManagerDiscovery) ScheduleUpdateServiceURL() {

Conversation

okJiang commented Sep 28, 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 Sep 28, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

okJiang commented Dec 24, 2025

Uh oh!

okJiang commented Jan 5, 2026

Uh oh!

okJiang commented Jan 5, 2026

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!

okJiang commented Jan 5, 2026

Uh oh!

okJiang commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rleungx left a comment

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!

okJiang commented Jan 6, 2026

Uh oh!

okJiang commented Jan 6, 2026

Uh oh!

ti-chi-bot bot commented Jan 6, 2026

Uh oh!

ti-chi-bot bot commented Jan 6, 2026

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

okJiang commented Sep 28, 2025 •

edited

Loading

codecov bot commented Sep 28, 2025 •

edited

Loading