mcs: change the get member return value #7819

rleungx · 2024-02-18T06:53:03Z

What problem does this PR solve?

Issue Number: Ref #5839.

What is changed and how does it work?

tidb-dashborad needs more information besides the address, it's better to use the member api.

Check List

Tests

Unit test

Release note

None.

ti-chi-bot · 2024-02-18T06:53:05Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

HuSharp
lhy1024

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.

HuSharp

most LGTM, maybe need to change tidb-operator and inform testbed as well

pingcap/tidb-operator#5547

rleungx · 2024-02-18T07:47:50Z

most LGTM, maybe need to change tidb-operator and inform testbed as well

pingcap/tidb-operator#5547

Will do it later.

lhy1024 · 2024-02-19T02:51:57Z

client/http/types.go

@@ -592,3 +592,12 @@ type MembersInfo struct {
 	Leader     *pdpb.Member         `json:"leader,omitempty"`
 	EtcdLeader *pdpb.Member         `json:"etcd_leader,omitempty"`
 }
+
+// MicroServiceMember is the member info of a micro service.
+type MicroServiceMember struct {


ServiceRegistryEntry and MicroServiceMember should be always the same? We need to add some comments to avoid inconsistent after late modification.

make their name unified

codecov · 2024-02-19T04:19:42Z

Codecov Report

Merging #7819 (685db6b) into master (5a37b80) will increase coverage by 0.07%.
Report is 1 commits behind head on master.
The diff coverage is 83.33%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7819      +/-   ##
==========================================
+ Coverage   73.39%   73.46%   +0.07%     
==========================================
  Files         432      432              
  Lines       47879    47896      +17     
==========================================
+ Hits        35141    35188      +47     
+ Misses       9687     9659      -28     
+ Partials     3051     3049       -2

Flag	Coverage Δ
unittests	`73.46% <83.33%> (+0.07%)`	⬆️

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

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

lhy1024

the rest LGTM

lhy1024 · 2024-02-20T01:42:09Z

pkg/mcs/scheduling/server/server.go

-	s.serviceID = &discovery.ServiceRegistryEntry{ServiceAddr: s.cfg.AdvertiseListenAddr}
+	deployPath, err := os.Executable()
+	if err != nil {
+		deployPath = ""


Do we need to add a log about it?

maybe later if needed

lhy1024 · 2024-02-20T01:42:26Z

pkg/mcs/tso/server/server.go

-	s.serviceID = &discovery.ServiceRegistryEntry{ServiceAddr: s.cfg.AdvertiseListenAddr}
+	deployPath, err := os.Executable()
+	if err != nil {
+		deployPath = ""


HuSharp · 2024-02-20T03:54:37Z

most LGTM, maybe need to change tidb-operator and inform testbed as well

pingcap/tidb-operator#5547

Passed the test with pingcap/tidb-operator#5547

rleungx · 2024-02-20T09:00:10Z

/merge

ti-chi-bot · 2024-02-20T09:00:12Z

@rleungx: 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 · 2024-02-20T09:00:13Z

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

Commit hash: 685db6b

ti-chi-bot · 2024-02-20T09:00:27Z

@rleungx: 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.

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels Feb 18, 2024

ti-chi-bot bot requested review from lhy1024 and Yisaer February 18, 2024 06:53

ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 18, 2024

rleungx requested review from HuSharp and removed request for Yisaer February 18, 2024 06:53

HuSharp mentioned this pull request Feb 18, 2024

pdms: change get ms members return value pingcap/tidb-operator#5547

Merged

4 tasks

HuSharp reviewed Feb 18, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Feb 18, 2024

rleungx force-pushed the refactor-interface branch from f5dd49b to dccb195 Compare February 18, 2024 10:01

lhy1024 approved these changes Feb 19, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 19, 2024

HuSharp approved these changes Feb 19, 2024

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 Feb 19, 2024

rleungx added 2 commits February 19, 2024 16:17

change the get member return value

34a28a5

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

fix tests

685db6b

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

rleungx force-pushed the refactor-interface branch from f98f97f to 763d50a Compare February 19, 2024 08:17

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 19, 2024

rleungx requested review from lhy1024 and HuSharp February 19, 2024 08:41

rleungx force-pushed the refactor-interface branch 4 times, most recently from 5232ff9 to 76ef427 Compare February 19, 2024 08:55

rleungx force-pushed the refactor-interface branch from 626782e to 685db6b Compare February 19, 2024 09:26

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 19, 2024

lhy1024 approved these changes Feb 20, 2024

View reviewed changes

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 20, 2024

Merge branch 'master' into refactor-interface

7fe9899

ti-chi-bot bot merged commit 46fd313 into tikv:master Feb 20, 2024
22 checks passed

rleungx deleted the refactor-interface branch February 20, 2024 09:29

HuSharp mentioned this pull request Mar 5, 2024

pdms: randomly run pdms in e2e test pingcap/tidb-operator#5559

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mcs: change the get member return value #7819

mcs: change the get member return value #7819

rleungx commented Feb 18, 2024 •

edited

Loading

ti-chi-bot bot commented Feb 18, 2024 •

edited

Loading

HuSharp left a comment

rleungx commented Feb 18, 2024

lhy1024 Feb 19, 2024

rleungx Feb 19, 2024 •

edited

Loading

codecov bot commented Feb 19, 2024 •

edited

Loading

lhy1024 left a comment

lhy1024 Feb 20, 2024

rleungx Feb 20, 2024

lhy1024 Feb 20, 2024

HuSharp commented Feb 20, 2024

rleungx commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

mcs: change the get member return value #7819

mcs: change the get member return value #7819

Conversation

rleungx commented Feb 18, 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 Feb 18, 2024 • edited Loading

HuSharp left a comment

Choose a reason for hiding this comment

rleungx commented Feb 18, 2024

lhy1024 Feb 19, 2024

Choose a reason for hiding this comment

rleungx Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Feb 19, 2024 • edited Loading

Codecov Report

lhy1024 left a comment

Choose a reason for hiding this comment

lhy1024 Feb 20, 2024

Choose a reason for hiding this comment

rleungx Feb 20, 2024

Choose a reason for hiding this comment

lhy1024 Feb 20, 2024

Choose a reason for hiding this comment

HuSharp commented Feb 20, 2024

rleungx commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

ti-chi-bot bot commented Feb 20, 2024

rleungx commented Feb 18, 2024 •

edited

Loading

ti-chi-bot bot commented Feb 18, 2024 •

edited

Loading

rleungx Feb 19, 2024 •

edited

Loading

codecov bot commented Feb 19, 2024 •

edited

Loading