Feat(router service): implement grpc function by bufferflies · Pull Request #9723 · tikv/pd

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 <rleungx@gmail.com>

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: 童剑 <1045931706@qq.com>

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

Codecov Report

❌ Patch coverage is 53.21337% with 182 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.60%. Comparing base (a749e77) to head (96f8937).
⚠️ Report is 6 commits behind head on master.

❌ Your patch status has failed because the patch coverage (53.21%) 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   78.68%   78.60%   -0.08%     
==========================================
  Files         492      494       +2     
  Lines       66274    66299      +25     
==========================================
- Hits        52148    52115      -33     
- Misses      10418    10467      +49     
- Partials     3708     3717       +9

Flag	Coverage Δ
unittests	`78.60% <53.21%> (-0.08%)`	⬇️

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.

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/utils/grpcutil/cluster.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: 童剑 <1045931706@qq.com>

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?

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

rleungx · 2025-11-04T08:23:42Z

pkg/utils/grpcutil/metrics.go

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


Suggested change

func RegionRequestCounter(method string, header *pdpb.RequestHeader, err *pdpb.Error, counter *prometheus.CounterVec) {

func RequestCounter(method string, header *pdpb.RequestHeader, err *pdpb.Error, counter *prometheus.CounterVec) {

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

ti-chi-bot · 2025-11-04T13:46:27Z

@bufferflies: 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
non-block/pull-unit-test-next-gen	`050093d`	link	false	`/test pull-unit-test-next-gen`

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.

bufferflies · 2025-11-05T02:28:18Z

/test pull-unit-test-next-gen

bufferflies · 2025-11-05T02:30:03Z

/test pull-integration-realcluster-test

ti-chi-bot · 2025-11-05T02:56:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: okJiang, 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~~ [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-05T02:56:05Z

[LGTM Timeline notifier]

Timeline:

2025-10-29 09:42:44.62911585 +0000 UTC m=+1469670.706368410: ☑️ agreed by okJiang.
2025-11-05 02:56:04.972749804 +0000 UTC m=+238814.415779673: ☑️ agreed by rleungx.

rleungx added 3 commits August 26, 2025 17:32

implement basic framework of router service

f29de7c

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

2c65b97

Signed-off-by: Ryan Leung <rleungx@gmail.com>

reuse pdpb

43650b7

Signed-off-by: Ryan Leung <rleungx@gmail.com>

grpc impl

67aa227

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

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: 童剑 <1045931706@qq.com>

resolve conflict

a42d410

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

pass ut

45b33af

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

resolve confilict

dd9cb15

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

grpc and metrics

796eaa3

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

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: 童剑 <1045931706@qq.com>

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: 童剑 <1045931706@qq.com>

rleungx reviewed Oct 31, 2025

View reviewed changes

move

12845ca

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

bufferflies requested a review from rleungx November 4, 2025 07:52

rleungx reviewed Nov 4, 2025

View reviewed changes

resolve cycle import

96f8937

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

rleungx approved these changes Nov 5, 2025

View reviewed changes

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

bufferflies requested a review from rleungx November 5, 2025 02:56

ti-chi-bot bot merged commit 7f7165e into tikv:master Nov 5, 2025
36 of 40 checks passed

	func RegionRequestCounter(method string, header pdpb.RequestHeader, err pdpb.Error, counter *prometheus.CounterVec) {
	func RequestCounter(method string, header pdpb.RequestHeader, err pdpb.Error, counter *prometheus.CounterVec) {

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!

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!

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 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bufferflies commented Nov 5, 2025

Uh oh!

bufferflies commented Nov 5, 2025

Uh oh!

ti-chi-bot bot commented Nov 5, 2025

Uh oh!

ti-chi-bot bot commented Nov 5, 2025

[LGTM Timeline notifier]

Uh oh!

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

ti-chi-bot bot commented Nov 4, 2025 •

edited

Loading