Feat(router service): implement grpc function #9723

bufferflies · 2025-09-05T03:30:34Z

What problem does this PR solve?

Issue Number: Ref #9212

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: Ryan Leung <[email protected]>

ti-chi-bot · 2025-09-05T03:30:37Z

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: 童剑 <[email protected]>

codecov · 2025-10-15T08:56:53Z

Codecov Report

❌ Patch coverage is 52.22482% with 204 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.88%. Comparing base (9003262) to head (050093d).
⚠️ Report is 10 commits behind head on master.

❌ Your patch check has failed because the patch coverage (52.22%) is below the target coverage (74.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9723      +/-   ##
==========================================
+ Coverage   76.82%   76.88%   +0.06%     
==========================================
  Files         491      492       +1     
  Lines       78473    78504      +31     
==========================================
+ Hits        60283    60354      +71     
+ Misses      14486    14457      -29     
+ Partials     3704     3693      -11

Flag	Coverage Δ
unittests	`76.88% <52.22%> (+0.06%)`	⬆️

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.

bufferflies · 2025-10-15T12:01:50Z

/test pull-unit-test-next-gen

bufferflies · 2025-10-23T00:35:40Z

/ping @rleungx @okJiang

okJiang · 2025-10-29T08:14:20Z

server/grpc_service.go

-		PendingPeers: region.GetPendingPeers(),
-		Buckets:      buckets,
-	}, nil
+	request.NeedBuckets = !*followerHandle && rc.GetStoreConfig().IsEnableRegionBucket() && request.GetNeedBuckets()


How about wrap a function?

The three variables from different structures, it seems so strange if a new function needs three parameters.

ti-chi-bot · 2025-10-29T09:42:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: okJiang
Once this PR has been reviewed and has the lgtm label, please assign husharp for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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-10-29T09:42:44Z

[LGTM Timeline notifier]

Timeline:

2025-10-29 09:42:44.62911585 +0000 UTC m=+1469670.706368410: ☑️ agreed by okJiang.

rleungx · 2025-10-16T07:23:37Z

pkg/grpc/helper.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package grpc


Can we choose a better package name?

how about move this into the pkg/utils/grpcutil/grpcutil.go?

rleungx · 2025-10-16T07:25:55Z

pkg/grpc/helper.go

+)
+
+// IncRegionRequestCounter increments the region request counter with the given method, header, error, and counter.
+func IncRegionRequestCounter(method string, header *pdpb.RequestHeader, err *pdpb.Error, counter *prometheus.CounterVec) {


Suggest moving back to metrics.go

rleungx · 2025-10-16T07:44:34Z

pkg/grpc/helper.go

+}
+
+// BatchScanRegions implements gRPC PDServer.
+func BatchScanRegions(rc *core.BasicCluster, request *pdpb.BatchScanRegionsRequest, isFollower bool) (resp *pdpb.BatchScanRegionsResponse, err error) {


Seems no need to use a new package?

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot · 2025-10-31T03:49:36Z

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

Test name	Commit	Details	Required	Rerun command
non-block/pull-unit-test-next-gen	`050093d`	link	false	`/test pull-unit-test-next-gen`
pull-build-next-gen	`5d56496`	link	true	`/test pull-build-next-gen`
pull-build	`5d56496`	link	true	`/test pull-build`
pull-integration-realcluster-test	`5d56496`	link	true	`/test pull-integration-realcluster-test`
pull-unit-test-next-gen	`5d56496`	link	true	`/test pull-unit-test-next-gen`

Full PR test history. Your PR dashboard.

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.

rleungx · 2025-10-31T04:27:57Z

pkg/metering/utils.go

+)
+
+// IncRegionRequestCounter increments the region request counter with the given method, header, error, and counter.
+func IncRegionRequestCounter(method string, header *pdpb.RequestHeader, err *pdpb.Error, counter *prometheus.CounterVec) {


in metering?

rleungx added 3 commits August 26, 2025 17:32

implement basic framework of router service

f29de7c

Signed-off-by: Ryan Leung <[email protected]>

address the comment

2c65b97

Signed-off-by: Ryan Leung <[email protected]>

reuse pdpb

43650b7

Signed-off-by: Ryan Leung <[email protected]>

grpc impl

67aa227

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feat/router_grpc_impl branch from 328c657 to 67aa227 Compare September 10, 2025 12:08

bufferflies added 5 commits September 11, 2025 10:27

refactor code

a4c32de

Signed-off-by: 童剑 <[email protected]>

resolve conflict

a42d410

Signed-off-by: 童剑 <[email protected]>

pass ut

45b33af

Signed-off-by: 童剑 <[email protected]>

resolve confilict

dd9cb15

Signed-off-by: 童剑 <[email protected]>

grpc and metrics

796eaa3

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Oct 14, 2025

bufferflies force-pushed the feat/router_grpc_impl branch 3 times, most recently from 44f7459 to 54a2392 Compare October 14, 2025 12:33

bufferflies marked this pull request as ready for review October 14, 2025 12:33

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 14, 2025

bufferflies force-pushed the feat/router_grpc_impl branch 5 times, most recently from e14be22 to 238b1b4 Compare October 15, 2025 04:41

lint

9a85795

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feat/router_grpc_impl branch from 238b1b4 to 9a85795 Compare October 15, 2025 08:48

Merge branch 'master' of github.com:tikv/pd into feat/router_grpc_impl

050093d

okJiang reviewed Oct 29, 2025

View reviewed changes

okJiang approved these changes Oct 29, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 29, 2025

bufferflies requested review from lhy1024 and rleungx October 31, 2025 02:18

Merge branch 'master' of github.com:tikv/pd into feat/router_grpc_impl

1705d8f

rleungx reviewed Oct 31, 2025

View reviewed changes

move IncRegionRequestCounter to metering

5d56496

Signed-off-by: 童剑 <[email protected]>

rleungx reviewed Oct 31, 2025

View reviewed changes

Feat(router service): implement grpc function #9723

Are you sure you want to change the base?

Feat(router service): implement grpc function #9723

Uh oh!

Conversation

bufferflies commented Sep 5, 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 5, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

bufferflies commented Oct 15, 2025

Uh oh!

bufferflies commented Oct 23, 2025

Uh oh!

okJiang Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

bufferflies Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Oct 29, 2025

Uh oh!

ti-chi-bot bot commented Oct 29, 2025

[LGTM Timeline notifier]

Uh oh!

rleungx Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

bufferflies Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

rleungx Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

rleungx Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Oct 31, 2025

Uh oh!

rleungx Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bufferflies commented Sep 5, 2025 •

edited

Loading

codecov bot commented Oct 15, 2025 •

edited

Loading