pdctl: support show keyspace group primary #6747

lhy1024 · 2023-07-04T11:37:58Z

What problem does this PR solve?

Issue Number: Close #6746

What is changed and how does it work?

Check List

Tests

Unit test

Release note

None.

Signed-off-by: lhy1024 <admin@liudos.us>

ti-chi-bot · 2023-07-04T11:38:00Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JmPotato
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.

codecov · 2023-07-04T11:47:53Z

Codecov Report

Merging #6747 (ddc2ae9) into master (c87782a) will increase coverage by 0.03%.
The diff coverage is 54.54%.

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6747      +/-   ##
==========================================
+ Coverage   74.18%   74.22%   +0.03%     
==========================================
  Files         411      411              
  Lines       43126    43057      -69     
==========================================
- Hits        31992    31957      -35     
+ Misses       8265     8224      -41     
- Partials     2869     2876       +7

Flag	Coverage Δ
unittests	`74.22% <54.54%> (+0.03%)`	⬆️

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

Signed-off-by: lhy1024 <admin@liudos.us>

rleungx · 2023-07-05T02:37:41Z

pkg/keyspace/tso_keyspace_group.go

+	path := fmt.Sprintf("/ms/%d/tso/00000/primary", m.clusterID)
+	if id != utils.DefaultKeyspaceGroupID {
+		path = fmt.Sprintf("/ms/%d/tso/keyspace_groups/election/%05d/primary", m.clusterID, id)


This code is duplicated in many places

I will move them to mcsutil

server/apiv2/handlers/tso_keyspace_group.go

Signed-off-by: lhy1024 <admin@liudos.us>

rleungx · 2023-07-05T03:58:35Z

tools/pd-ctl/pdctl/command/keyspace_group_command.go

@@ -357,7 +357,7 @@ func showKeyspaceGroupPrimaryCommandFunc(cmd *cobra.Command, args []string) {
 		cmd.Printf("Failed to parse the keyspace group ID: %s\n", err)
 		return
 	}
-	r, err := doRequest(cmd, fmt.Sprintf("%s/%s/primary", keyspaceGroupsPrefix, args[0]), http.MethodGet, http.Header{})
+	r, err := doRequest(cmd, fmt.Sprintf("%s/%s?get_param=primary", keyspaceGroupsPrefix, args[0]), http.MethodGet, http.Header{})


I think it should be a bool, right?

"%s/%s?only_primary_member=" so?

Maybe you can google it to find the best practice?

maybe it is better, "%s/%s?fields=primary" and return { id:0, primary:""}
ref:
https://stackoverflow.com/questions/18235546/way-to-specify-resources-fields-list-in-restful-api-request
https://stackoverflow.com/questions/68979696/restful-api-including-additional-fields

Signed-off-by: lhy1024 <admin@liudos.us>

ref #6747, ref #6748, ref #6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

Signed-off-by: lhy1024 <admin@liudos.us>

lhy1024 · 2023-07-10T10:16:37Z

/merge

ti-chi-bot · 2023-07-10T10:16:38Z

@lhy1024: 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-07-10T10:16:40Z

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

Commit hash: 6f12b4c

) ref tikv#6747, ref tikv#6748, ref tikv#6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

close tikv#6746 Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

) ref tikv#6747, ref tikv#6748, ref tikv#6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

close tikv#6746 Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

pdctl: support show keyspace group primary

94f2431

Signed-off-by: lhy1024 <admin@liudos.us>

lhy1024 requested review from JmPotato and rleungx July 4, 2023 11:37

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 4, 2023

fix lint

be30d79

Signed-off-by: lhy1024 <admin@liudos.us>

rleungx reviewed Jul 5, 2023

View reviewed changes

use query

0ab36f4

Signed-off-by: lhy1024 <admin@liudos.us>

rleungx reviewed Jul 5, 2023

View reviewed changes

rleungx mentioned this pull request Jul 5, 2023

*: add test for misusing keyspace ID when creating the client #6754

Merged

update api

ddc2ae9

Signed-off-by: lhy1024 <admin@liudos.us>

ti-chi-bot bot pushed a commit that referenced this pull request Jul 5, 2023

*: add test for misusing keyspace ID when creating the client (#6754)

dc180ca

ref #6747, ref #6748, ref #6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

lhy1024 added 2 commits July 7, 2023 12:21

Merge branch 'master' of http://github.com/tikv/pd into pd-ctl2

4a1f76a

use KeyspaceGroupPrimaryPath

6f12b4c

Signed-off-by: lhy1024 <admin@liudos.us>

lhy1024 requested a review from rleungx July 7, 2023 04:28

rleungx approved these changes Jul 10, 2023

View reviewed changes

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

JmPotato approved these changes Jul 10, 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 Jul 10, 2023

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

Merge branch 'master' into pd-ctl2

ff3da0e

ti-chi-bot bot merged commit 61dfad3 into tikv:master Jul 10, 2023

rleungx added a commit to rleungx/pd that referenced this pull request Aug 2, 2023

*: add test for misusing keyspace ID when creating the client (tikv#6754

ff1e38e

) ref tikv#6747, ref tikv#6748, ref tikv#6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx pushed a commit to rleungx/pd that referenced this pull request Aug 2, 2023

pdctl: support show keyspace group primary (tikv#6747)

ef78515

close tikv#6746 Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

rleungx pushed a commit to rleungx/pd that referenced this pull request Aug 2, 2023

pdctl: support show keyspace group primary (tikv#6747)

af955ef

close tikv#6746 Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

rleungx added a commit to rleungx/pd that referenced this pull request Aug 2, 2023

*: add test for misusing keyspace ID when creating the client (tikv#6754

90b1316

) ref tikv#6747, ref tikv#6748, ref tikv#6749 Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx pushed a commit to rleungx/pd that referenced this pull request Aug 2, 2023

pdctl: support show keyspace group primary (tikv#6747)

56a36cb

close tikv#6746 Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pdctl: support show keyspace group primary #6747

pdctl: support show keyspace group primary #6747

lhy1024 commented Jul 4, 2023

ti-chi-bot bot commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading

rleungx Jul 5, 2023

lhy1024 Jul 5, 2023

lhy1024 Jul 5, 2023

rleungx Jul 5, 2023

lhy1024 Jul 5, 2023

rleungx Jul 5, 2023

lhy1024 Jul 5, 2023

lhy1024 commented Jul 10, 2023

ti-chi-bot bot commented Jul 10, 2023

ti-chi-bot bot commented Jul 10, 2023

pdctl: support show keyspace group primary #6747

pdctl: support show keyspace group primary #6747

Conversation

lhy1024 commented Jul 4, 2023

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Jul 4, 2023 • edited Loading

codecov bot commented Jul 4, 2023 • edited Loading

Codecov Report

rleungx Jul 5, 2023

Choose a reason for hiding this comment

lhy1024 Jul 5, 2023

Choose a reason for hiding this comment

lhy1024 Jul 5, 2023

Choose a reason for hiding this comment

rleungx Jul 5, 2023

Choose a reason for hiding this comment

lhy1024 Jul 5, 2023

Choose a reason for hiding this comment

rleungx Jul 5, 2023

Choose a reason for hiding this comment

lhy1024 Jul 5, 2023

Choose a reason for hiding this comment

lhy1024 commented Jul 10, 2023

ti-chi-bot bot commented Jul 10, 2023

ti-chi-bot bot commented Jul 10, 2023

ti-chi-bot bot commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading