grpc_service: add `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. #8266

LykxSassinator · 2024-06-06T11:47:17Z

What problem does this PR solve?

Issue Number: Ref tikv/tikv#15292 proto: pingcap/kvproto#1250

What is changed and how does it work?

Make `grpc_service` support manually add / remove peers by `ManuallyChangePeer` and `ManuallyChangeMultiPeers`.

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

Add extra apis `ManuallyChangePeer` and `ManuallyChangeMultiPeers` for `grpc_service` to support manually add / remove peers.

make `grpc_service` support manually add / remove peers by `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot · 2024-06-06T11:47:20Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

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.

LykxSassinator · 2024-06-06T11:47:52Z

/cc @nolouch

LykxSassinator · 2024-06-06T11:51:30Z

client/go.mod

@@ -2,6 +2,8 @@ module github.com/tikv/pd/client

 go 1.21

+replace github.com/pingcap/kvproto => github.com/LykxSassinator/kvproto v0.0.0-20240605160713-4dfe4594d6ca


Will be cleared after pingcap/kvproto#1250 is merged.

LykxSassinator · 2024-06-06T11:51:38Z

go.mod

@@ -6,6 +6,7 @@ go 1.21
 // kvproto at the same time. You can run `go mod tidy` to make it replaced with go-mod style specification.
 // After the PR to kvproto is merged, remember to comment this out and run `go mod tidy`.
 // replace github.com/pingcap/kvproto => github.com/$YourPrivateRepo $YourPrivateBranch
+replace github.com/pingcap/kvproto => github.com/LykxSassinator/kvproto v0.0.0-20240605160713-4dfe4594d6ca


LykxSassinator · 2024-06-06T11:51:47Z

tools/go.mod

@@ -3,6 +3,7 @@ module github.com/tikv/pd/tools
 go 1.21

 replace (
+	github.com/pingcap/kvproto => github.com/LykxSassinator/kvproto v0.0.0-20240605160713-4dfe4594d6ca


overvenus · 2024-06-06T13:53:25Z

server/grpc_service.go

@@ -3000,6 +3001,78 @@ func (s *GrpcServer) GetExternalTimestamp(ctx context.Context, request *pdpb.Get
 	}, nil
 }

+// ManuallyChangePeer implements gRPC PDServer.
+func (s *GrpcServer) ManuallyChangePeer(ctx context.Context, request *pdpb.ChangePeerRequest) (*pdpb.ChangePeerResponse, error) {


Any input about the RPC name? @nolouch @rleungx

Can we only report the purpose then PD generates operators, like call interface as "ReportDamagedPeer"?

Do u mean using GetDamagedRegionsId?

Not appropriate. The DamagedRegionsId just records the abnormal region, and it's used to delete all of the peers of this region.

But what we need now is "add / remove" a single peer.

I mean the TiKV tells PD which peer is damaged or needs to be cleaned. then PD will log it(better with the
caller source) and add an operator to add/remove it.

Do not directly guide PD on how to do it, tell PD your purpose, and then PD makes the decisions. it prevents some unknown source from abusing it.

It just needs a simple way to let PD trigger the operator on add or remove peer here. And we've already have Restful API used for add & remove peer, gRPC API is intended to be used as the same purpose here.

ti-chi-bot · 2024-06-11T03:40:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lhy1024 for approval. For more information see the Code Review Process.

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

rleungx · 2024-06-12T04:00:11Z

client/client.go

@@ -133,6 +134,11 @@ type RPCClient interface {
 	// SetExternalTimestamp sets external timestamp
 	SetExternalTimestamp(ctx context.Context, timestamp uint64) error

+	// ManuallyChangePeer manually changes the peer of a region.
+	ManuallyChangePeer(ctx context.Context, regionID uint64, changeType eraftpb.ConfChangeType, peer *metapb.Peer) error


Is it necessary? Why not let TiKV handle it? If we are going to use RPC to solve the snapshot problem, we still can not ensure that the operator is executed successfully.

It's used to make the operation on add peer / remove peer managed by PD uniformly.

If let TiKV handle it directly, the scheduling on add peer or remove peer will not be perceived by PD, and the check of the scheduler cannot be validated by the store limit and other checking mechanisms.

As for

ensure that the operator is executed successfully.

TiKV will retry the remove peer RPC if the operator does not make senses until this peer is cleared.

ti-chi-bot · 2024-06-13T05:09:40Z

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-linked-issue label, please provide the linked issue number on one line in the PR body, for example: Issue Number: close #123 or Issue Number: ref #456, multiple issues should use full syntax for each issue and be separated by a comma, like: Issue Number: close #123, ref #456.

_{📖 For more info, you can check the "Linking issues" section in the CONTRIBUTING.md.}

LykxSassinator · 2024-06-13T09:12:26Z

This pr can be closed as this RPC is not necessary.

grpc_service: add ManuallyChangePeer and ManuallyChangeMultiPeers.

361e455

make `grpc_service` support manually add / remove peers by `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-triage-completed dco-signoff: yes release-note The PR should write the release note. labels Jun 6, 2024

Merge branch 'master' into change_peer_api

5270d79

ti-chi-bot bot requested review from HuSharp and rleungx June 6, 2024 11:47

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 6, 2024

ti-chi-bot bot requested a review from nolouch June 6, 2024 11:47

LykxSassinator commented Jun 6, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Jun 6, 2024

LykxSassinator mentioned this pull request Jun 6, 2024

raftstore: gc abnormal snapshots and destroy peer if failed to apply snapshots. tikv/tikv#16992

Merged

9 tasks

overvenus reviewed Jun 6, 2024

View reviewed changes

Merge branch 'master' into change_peer_api

37a9dba

Merge branch 'master' into change_peer_api

6c7098e

rleungx reviewed Jun 12, 2024

View reviewed changes

LykxSassinator added 2 commits June 13, 2024 10:28

Merge branch 'master' into change_peer_api

774fda1

Merge branch 'master' into change_peer_api

f55f5fe

LykxSassinator closed this Jun 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

grpc_service: add `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. #8266

grpc_service: add `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. #8266

LykxSassinator commented Jun 6, 2024 •

edited

Loading

ti-chi-bot bot commented Jun 6, 2024

LykxSassinator commented Jun 6, 2024

LykxSassinator Jun 6, 2024

LykxSassinator Jun 6, 2024

LykxSassinator Jun 6, 2024

overvenus Jun 6, 2024

nolouch Jun 12, 2024 •

edited

Loading

LykxSassinator Jun 12, 2024

nolouch Jun 13, 2024 •

edited

Loading

LykxSassinator Jun 13, 2024

ti-chi-bot bot commented Jun 11, 2024

rleungx Jun 12, 2024

LykxSassinator Jun 12, 2024 •

edited

Loading

ti-chi-bot bot commented Jun 13, 2024

LykxSassinator commented Jun 13, 2024

		@@ -2,6 +2,8 @@ module github.com/tikv/pd/client

		go 1.21

		replace github.com/pingcap/kvproto => github.com/LykxSassinator/kvproto v0.0.0-20240605160713-4dfe4594d6ca

grpc_service: add ManuallyChangePeer and ManuallyChangeMultiPeers. #8266

grpc_service: add ManuallyChangePeer and ManuallyChangeMultiPeers. #8266

Conversation

LykxSassinator commented Jun 6, 2024 • 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 Jun 6, 2024

LykxSassinator commented Jun 6, 2024

LykxSassinator Jun 6, 2024

Choose a reason for hiding this comment

LykxSassinator Jun 6, 2024

Choose a reason for hiding this comment

LykxSassinator Jun 6, 2024

Choose a reason for hiding this comment

overvenus Jun 6, 2024

Choose a reason for hiding this comment

nolouch Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

LykxSassinator Jun 12, 2024

Choose a reason for hiding this comment

nolouch Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

LykxSassinator Jun 13, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jun 11, 2024

rleungx Jun 12, 2024

Choose a reason for hiding this comment

LykxSassinator Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented Jun 13, 2024

LykxSassinator commented Jun 13, 2024

grpc_service: add `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. #8266

grpc_service: add `ManuallyChangePeer` and `ManuallyChangeMultiPeers`. #8266

LykxSassinator commented Jun 6, 2024 •

edited

Loading

nolouch Jun 12, 2024 •

edited

Loading

nolouch Jun 13, 2024 •

edited

Loading

LykxSassinator Jun 12, 2024 •

edited

Loading