*: follower support to handle `GetRegion` and other region api #7432

CabinfeverB · 2023-11-24T01:36:17Z

What problem does this PR solve?

Issue Number: ref #7431
should be merged after #7489 #7611

What is changed and how does it work?

Check List

Tests

Integration test
Manual test (add detailed scripts or steps below)

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

PD allows followers to handle gRPC API about region info such as `GetRegion` and `ScanRegions`. It can increase the maximum carrying capability of the PD routing service.

ti-chi-bot · 2023-11-24T01:36:21Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2023-11-24T01:36: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

CabinfeverB · 2023-11-24T07:21:48Z

cc @niubell

codecov · 2023-11-24T07:26:47Z

Codecov Report

Merging #7432 (c742353) into master (cf22c04) will increase coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 46.32%.

❗ Current head c742353 differs from pull request most recent head ff657fd. Consider uploading reports for the commit ff657fd to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7432      +/-   ##
==========================================
+ Coverage   74.79%   74.80%   +0.01%     
==========================================
  Files         459      459              
  Lines       50978    51062      +84     
==========================================
+ Hits        38128    38196      +68     
- Misses       9489     9493       +4     
- Partials     3361     3373      +12

Flag	Coverage Δ
unittests	`74.80% <46.32%> (+0.01%)`	⬆️

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

Signed-off-by: Cabinfever_B <[email protected]>

…_forward

Signed-off-by: Cabinfever_B <[email protected]>

rleungx · 2023-12-28T06:20:06Z

server/grpc_service.go

 func (s *GrpcServer) validateRequest(header *pdpb.RequestHeader) error {
-	if s.IsClosed() || !s.member.IsLeader() {
-		return ErrNotLeader
+	return s.validateRoleInRequest(context.TODO(), header, nil)


Why not just name it as validateRequest?

Some code needs the original validateRequest which only has one param and is easy to use, so I keep it.

rleungx · 2023-12-28T06:22:49Z

server/grpc_service.go

+	var region *core.RegionInfo
+	if *followerHandle {
+		rc = s.cluster
+		if !s.cluster.GetRegionSyncer().IsRunning() {


Suggested change

if !s.cluster.GetRegionSyncer().IsRunning() {

if !rc.GetRegionSyncer().IsRunning() {

Signed-off-by: Cabinfever_B <[email protected]>

nolouch · 2023-12-29T09:24:48Z

/merge

ti-chi-bot · 2023-12-29T09:24:50Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-12-29T09:24:51Z

This pull request has been accepted and is ready to merge.

Commit hash: c742353

ti-chi-bot · 2023-12-29T09:27:40Z

@CabinfeverB: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.

…7432) ref tikv#7431 Signed-off-by: Cabinfever_B <[email protected]> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: pingandb <[email protected]>

lidaohang · 2024-12-17T08:52:03Z

mark

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Nov 24, 2023

ti-chi-bot bot requested review from disksing and rleungx November 24, 2023 01:36

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 24, 2023

CabinfeverB marked this pull request as ready for review November 24, 2023 07:16

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 Nov 24, 2023

CabinfeverB changed the title ~~add follower handle~~ *: support follower handle Nov 24, 2023

CabinfeverB changed the title *: support follower handle *: follower support handle GetRegion and other region api Nov 24, 2023

CabinfeverB changed the title *: follower support handle GetRegion and other region api *: follower support to handle GetRegion and other region api Nov 24, 2023

CabinfeverB requested review from nolouch and removed request for disksing November 24, 2023 07:18

CabinfeverB added 2 commits December 4, 2023 11:04

add service client

7e725fa

Signed-off-by: Cabinfever_B <[email protected]>

add service client

889c0eb

Signed-off-by: Cabinfever_B <[email protected]>

CabinfeverB marked this pull request as draft December 4, 2023 03:28

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2023

CabinfeverB added 9 commits December 4, 2023 13:47

address comment

6315793

Signed-off-by: Cabinfever_B <[email protected]>

Merge remote-tracking branch 'upstream/master' into follower/refactor…

fa0f510

…_forward

address comment

6535c6e

Signed-off-by: Cabinfever_B <[email protected]>

address comment

423717c

Signed-off-by: Cabinfever_B <[email protected]>

Merge branch 'master' into follower/service_clinet

013fbce

Merge branch 'follower/service_clinet' into follower/refactor_forward

943331e

address comment

0891cf1

Signed-off-by: Cabinfever_B <[email protected]>

Merge branch 'follower/service_clinet' into follower/refactor_forward

9322e4e

impl follower region

739deba

Signed-off-by: Cabinfever_B <[email protected]>

CabinfeverB force-pushed the follower/refactor_forward branch from c074c18 to 739deba Compare December 5, 2023 09:37

CabinfeverB added 2 commits December 26, 2023 14:08

merge master

d5af768

Signed-off-by: Cabinfever_B <[email protected]>

merge master

5ef3716

Signed-off-by: Cabinfever_B <[email protected]>

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 26, 2023

merge master

d5ca4c6

Signed-off-by: Cabinfever_B <[email protected]>

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 27, 2023

CabinfeverB requested review from lhy1024, bufferflies and HuSharp December 27, 2023 02:21

Merge branch 'master' into follower/refactor_forward

fad91de

rleungx reviewed Dec 28, 2023

View reviewed changes

CabinfeverB added 2 commits December 28, 2023 14:29

Merge branch 'master' into follower/refactor_forward

c1878d2

address comment

c742353

Signed-off-by: Cabinfever_B <[email protected]>

CabinfeverB mentioned this pull request Dec 28, 2023

Activate Follower-Handle #7431

Closed

3 tasks

rleungx approved these changes Dec 28, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 28, 2023

nolouch approved these changes Dec 29, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 29, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 29, 2023

ti-chi-bot bot added 2 commits December 29, 2023 09:24

Merge branch 'master' into follower/refactor_forward

4de4d48

Merge branch 'master' into follower/refactor_forward

ff657fd

ti-chi-bot bot merged commit a67ccbb into tikv:master Dec 29, 2023
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: follower support to handle `GetRegion` and other region api #7432

*: follower support to handle `GetRegion` and other region api #7432

CabinfeverB commented Nov 24, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 24, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 24, 2023

CabinfeverB commented Nov 24, 2023

codecov bot commented Nov 24, 2023 •

edited

Loading

rleungx Dec 28, 2023

CabinfeverB Dec 28, 2023 •

edited

Loading

rleungx Dec 28, 2023

nolouch commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

lidaohang commented Dec 17, 2024

	if !s.cluster.GetRegionSyncer().IsRunning() {
	if !rc.GetRegionSyncer().IsRunning() {

*: follower support to handle GetRegion and other region api #7432

*: follower support to handle GetRegion and other region api #7432

Conversation

CabinfeverB commented Nov 24, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Nov 24, 2023 • edited Loading

ti-chi-bot bot commented Nov 24, 2023

CabinfeverB commented Nov 24, 2023

codecov bot commented Nov 24, 2023 • edited Loading

Codecov Report

rleungx Dec 28, 2023

Choose a reason for hiding this comment

CabinfeverB Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

rleungx Dec 28, 2023

Choose a reason for hiding this comment

nolouch commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

ti-chi-bot bot commented Dec 29, 2023

lidaohang commented Dec 17, 2024

*: follower support to handle `GetRegion` and other region api #7432

*: follower support to handle `GetRegion` and other region api #7432

CabinfeverB commented Nov 24, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 24, 2023 •

edited

Loading

codecov bot commented Nov 24, 2023 •

edited

Loading

CabinfeverB Dec 28, 2023 •

edited

Loading