resource_manager: implement the independent primary election #6086

JmPotato · 2023-03-03T04:33:14Z

What problem does this PR solve?

Issue Number: ref #5766.

What is changed and how does it work?

Implement the independent resource manager primary election.

Check List

Tests

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

Release note

None.

Signed-off-by: JmPotato <ghzpotato@gmail.com>

ti-chi-bot · 2023-03-03T04:33:17Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
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-03-03T04:33:17Z

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: JmPotato <ghzpotato@gmail.com>

codecov · 2023-03-03T08:33:16Z

Codecov Report

Patch coverage: 72.22% and project coverage change: +0.12 🎉

Comparison is base (312e5b4) 73.95% compared to head (841a20a) 74.07%.

❗ Current head 841a20a differs from pull request most recent head 8c495a5. Consider uploading reports for the commit 8c495a5 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6086      +/-   ##
==========================================
+ Coverage   73.95%   74.07%   +0.12%     
==========================================
  Files         383      385       +2     
  Lines       37882    37958      +76     
==========================================
+ Hits        28015    28119     +104     
+ Misses       7409     7361      -48     
- Partials     2458     2478      +20

Flag	Coverage Δ
unittests	`74.07% <72.22%> (+0.12%)`	⬆️

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

Impacted Files	Coverage Δ
pkg/mcs/resource_manager/server/server.go	`54.09% <66.66%> (+5.32%)`	⬆️
pkg/mcs/resource_manager/server/testutil.go	`86.66% <86.66%> (ø)`
pkg/mcs/resource_manager/server/config.go	`71.23% <100.00%> (+55.95%)`	⬆️
pkg/mcs/resource_manager/server/metrics.go	`100.00% <100.00%> (ø)`
pkg/mcs/tso/server/server.go	`44.67% <100.00%> (-0.19%)`	⬇️
pkg/utils/memberutil/member.go	`100.00% <100.00%> (ø)`
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/id/id.go	`83.05% <0.00%> (-6.78%)`	⬇️
pkg/storage/endpoint/meta.go	`63.63% <0.00%> (-4.55%)`	⬇️
server/region_syncer/client.go	`85.82% <0.00%> (-2.24%)`	⬇️
... and 28 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

rleungx · 2023-03-03T08:53:47Z

pkg/mcs/resource_manager/server/server.go

+	// resourceManagerKeyspaceGroupPrimaryElectionPrefix defines the key prefix for keyspace group primary election.
+	// The entire key is in the format of "/pd/<cluster-id>/microservice/resource_manager/keyspace-group-XXXXX/primary"
+	// in which XXXXX is 5 digits integer with leading zeros. For now we use 0 as the default cluster id.
+	resourceManagerKeyspaceGroupPrimaryElectionPrefix = "/pd/0/microservice/resource_manager/keyspace-group-"


Will the resource manager have the concept of a keyspace group?

Yes, in the assumption of the future, we may separate the resource manager service into several to serve different keyspace groups.

tests/mcs/resource_manager/server_test.go

Signed-off-by: JmPotato <ghzpotato@gmail.com>

pkg/mcs/resource_manager/server/server.go

HuSharp · 2023-03-06T02:18:30Z

pkg/mcs/resource_manager/server/config.go

@@ -32,13 +32,15 @@ import (

 const (
 	defaultName              = "Resource Manager"
-	defaultBackendEndpoints  = "127.0.0.1:2379"
+	defaultBackendEndpoints  = "http://127.0.0.1:3379"


maybe conflict with tso's defaultListenAddr

BTW, I think 2379 is better.

Signed-off-by: JmPotato <ghzpotato@gmail.com>

nolouch · 2023-03-06T04:15:26Z

PTAL @CabinfeverB

pkg/mcs/resource_manager/server/config.go

Signed-off-by: JmPotato <ghzpotato@gmail.com>

nolouch

rest lgtm

pkg/mcs/resource_manager/server/server.go

Signed-off-by: JmPotato <ghzpotato@gmail.com>

nolouch · 2023-03-06T10:04:47Z

/merge

ti-chi-bot · 2023-03-06T10:04:49Z

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

/run-all-tests

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-03-06T10:04:50Z

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

Commit hash: 3e15ec4

Implement the independent resource manager primary election

1dad889

Signed-off-by: JmPotato <ghzpotato@gmail.com>

JmPotato added the component/resource-control label Mar 3, 2023

JmPotato requested review from nolouch, lhy1024, CabinfeverB and rleungx March 3, 2023 04:33

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

Finish the test

e7d2bfa

Signed-off-by: JmPotato <ghzpotato@gmail.com>

JmPotato marked this pull request as ready for review March 3, 2023 08:50

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

rleungx reviewed Mar 3, 2023

View reviewed changes

tests/mcs/resource_manager/server_test.go Outdated Show resolved Hide resolved

Fix the necessary test

2f3d845

Signed-off-by: JmPotato <ghzpotato@gmail.com>

nolouch reviewed Mar 3, 2023

View reviewed changes

pkg/mcs/resource_manager/server/server.go Outdated Show resolved Hide resolved

HuSharp reviewed Mar 6, 2023

View reviewed changes

Address the comments

e8d9c05

Signed-off-by: JmPotato <ghzpotato@gmail.com>

JmPotato force-pushed the independent_resource_manager branch from 2aa3863 to e8d9c05 Compare March 6, 2023 02:49

JmPotato requested review from nolouch, rleungx and HuSharp March 6, 2023 03:13

rleungx reviewed Mar 6, 2023

View reviewed changes

pkg/mcs/resource_manager/server/config.go Outdated Show resolved Hide resolved

Unify the URL addr config format

841a20a

Signed-off-by: JmPotato <ghzpotato@gmail.com>

rleungx approved these changes Mar 6, 2023

View reviewed changes

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

nolouch reviewed Mar 6, 2023

View reviewed changes

pkg/mcs/resource_manager/server/server.go Outdated Show resolved Hide resolved

Address the comment

3e15ec4

Signed-off-by: JmPotato <ghzpotato@gmail.com>

CabinfeverB approved these changes Mar 6, 2023

View reviewed changes

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

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

Merge branch 'master' into independent_resource_manager

8c495a5

ti-chi-bot merged commit bc5234c into tikv:master Mar 6, 2023

JmPotato deleted the independent_resource_manager branch March 6, 2023 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource_manager: implement the independent primary election #6086

resource_manager: implement the independent primary election #6086

JmPotato commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023 •

edited

Loading

ti-chi-bot commented Mar 3, 2023

codecov bot commented Mar 3, 2023 •

edited

Loading

rleungx Mar 3, 2023

JmPotato Mar 3, 2023

HuSharp Mar 6, 2023

rleungx Mar 6, 2023

nolouch commented Mar 6, 2023

nolouch left a comment

nolouch commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

resource_manager: implement the independent primary election #6086

resource_manager: implement the independent primary election #6086

Conversation

JmPotato commented Mar 3, 2023

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Mar 3, 2023 • edited Loading

ti-chi-bot commented Mar 3, 2023

codecov bot commented Mar 3, 2023 • edited Loading

Codecov Report

rleungx Mar 3, 2023

Choose a reason for hiding this comment

JmPotato Mar 3, 2023

Choose a reason for hiding this comment

HuSharp Mar 6, 2023

Choose a reason for hiding this comment

rleungx Mar 6, 2023

Choose a reason for hiding this comment

nolouch commented Mar 6, 2023

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

ti-chi-bot commented Mar 3, 2023 •

edited

Loading

codecov bot commented Mar 3, 2023 •

edited

Loading