ctl: replace cluster info with HTTP SDK #7605

HuSharp · 2023-12-21T09:19:45Z

What problem does this PR solve?

Issue Number: Ref #7300, wait for #7635 merged

What is changed and how does it work?

want to replace all pd-ctl
support cluster command firstly

$ ./pd-ctl cluster
{
  "id": 7316377368062736100,
  "max_peer_count": 3
}

$ ./pd-ctl --pd http://127.0.0.1:52719 cluster
{
  "id": 7316426302202361416,
  "max_peer_count": 3
}

Check List

Tests

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

Release note

None.

ti-chi-bot · 2023-12-21T09:19:47Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
JmPotato

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-12-21T09:31:15Z

Codecov Report

Merging #7605 (fa30740) into master (335bd1e) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7605      +/-   ##
==========================================
- Coverage   73.89%   73.86%   -0.03%     
==========================================
  Files         429      429              
  Lines       47317    47320       +3     
==========================================
- Hits        34965    34954      -11     
- Misses       9360     9371      +11     
- Partials     2992     2995       +3

Flag	Coverage Δ
unittests	`73.86% <0.00%> (-0.03%)`	⬇️

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

Signed-off-by: husharp <jinhao.hu@pingcap.com>

client/http/types.go

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

go.mod

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp · 2023-12-27T03:44:05Z

@rleungx @CabinfeverB friendly ping :)

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp · 2023-12-28T08:31:09Z

/hold wait #7635 merged

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp · 2024-01-02T02:51:11Z

/unhold

HuSharp · 2024-01-02T02:53:08Z

FYI, after #7635 merged, we do not need to update the server's gomod to import client

@JmPotato @CabinfeverB @rleungx PTAL, thx!

client/http/api.go

JmPotato · 2024-01-03T02:53:35Z

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

@@ -34,12 +35,25 @@ const (
 pingPrefix = "pd/api/v1/ping"
 )

+// PDCli is a pd HTTP client
+var PDCli pd.Client


What about using a sync.Once here?

because we support -i which can reset client addrs, maybe sync.Once is not suitable

JmPotato · 2024-01-03T02:56:15Z

tools/pd-ctl/pdctl/ctl.go

 rootCmd.Println(err)
 return err
 }
+ } else {
+ command.SetNewPDClient(strings.Split(addrs, ","))


What about setting the PD HTTP client unconditionally before we finish the replacement?

we can simplify it after replacing dialClient

Signed-off-by: husharp <jinhao.hu@pingcap.com>

JmPotato · 2024-01-03T06:24:37Z

/merge

ti-chi-bot · 2024-01-03T06:24:39Z

@JmPotato: 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-01-03T06:24:40Z

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

Commit hash: edc498f

ti-chi-bot · 2024-01-03T06:24:54Z

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

ref tikv#7300 Signed-off-by: husharp <jinhao.hu@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: pingandb <songge102@pingan.com.cn>

ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label Dec 21, 2023

ti-chi-bot bot requested review from HunDunDM and rleungx December 21, 2023 09:19

ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 21, 2023

HuSharp removed request for HunDunDM and rleungx December 21, 2023 09:21

HuSharp changed the title ~~ctl: replace cluster info~~ ctl: replace cluster info HTTP SDK Dec 21, 2023

HuSharp changed the title ~~ctl: replace cluster info HTTP SDK~~ ctl: replace cluster info with HTTP SDK Dec 21, 2023

HuSharp force-pushed the replace_ctl_test branch from e48dde1 to 8142844 Compare December 21, 2023 11:48

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 Dec 21, 2023

add cluster

11463ee

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp force-pushed the replace_ctl_test branch from 8142844 to 11463ee Compare December 21, 2023 13:18

JmPotato reviewed Dec 22, 2023

View reviewed changes

client/http/types.go Outdated Show resolved Hide resolved

tools/pd-ctl/pdctl/command/cluster_command.go Outdated Show resolved Hide resolved

rleungx reviewed Dec 22, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

HuSharp added 4 commits December 22, 2023 15:17

seperate client

3184843

Signed-off-by: husharp <jinhao.hu@pingcap.com>

merge master

d103628

Signed-off-by: husharp <jinhao.hu@pingcap.com>

reuse pdcli

3417934

Signed-off-by: husharp <jinhao.hu@pingcap.com>

merge master

0cb4338

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp requested review from CabinfeverB and rleungx December 25, 2023 08:31

HuSharp force-pushed the replace_ctl_test branch 5 times, most recently from e5123ef to c14a305 Compare December 25, 2023 09:18

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 25, 2023

update go mod

2359082

Signed-off-by: husharp <jinhao.hu@pingcap.com>

CabinfeverB approved these changes Dec 27, 2023

View reviewed changes

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

merge master

c97759b

Signed-off-by: husharp <jinhao.hu@pingcap.com>

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 28, 2023

HuSharp added 2 commits December 30, 2023 19:23

merge master

f89b164

Signed-off-by: husharp <jinhao.hu@pingcap.com>

remove client close

fa30740

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp force-pushed the replace_ctl_test branch from 3972b2f to fa30740 Compare January 2, 2024 02:34

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 2, 2024

HuSharp requested review from JmPotato and CabinfeverB January 2, 2024 02:51

JmPotato reviewed Jan 3, 2024

View reviewed changes

rename api

edc498f

Signed-off-by: husharp <jinhao.hu@pingcap.com>

JmPotato approved these changes Jan 3, 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 Jan 3, 2024

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

Merge branch 'master' into replace_ctl_test

5ca044c

ti-chi-bot bot merged commit 5d91220 into tikv:master Jan 3, 2024
22 checks passed

HuSharp deleted the replace_ctl_test branch January 3, 2024 06:54

HuSharp mentioned this pull request Jan 29, 2024

Tracking issue for the HTTP SDK #7300

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ctl: replace cluster info with HTTP SDK #7605

ctl: replace cluster info with HTTP SDK #7605

HuSharp commented Dec 21, 2023 •

edited

Loading

ti-chi-bot bot commented Dec 21, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading

HuSharp commented Dec 27, 2023

HuSharp commented Dec 28, 2023

HuSharp commented Jan 2, 2024

HuSharp commented Jan 2, 2024

JmPotato Jan 3, 2024

HuSharp Jan 3, 2024

JmPotato Jan 3, 2024

HuSharp Jan 3, 2024

JmPotato commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ctl: replace cluster info with HTTP SDK #7605

ctl: replace cluster info with HTTP SDK #7605

Conversation

HuSharp commented Dec 21, 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 Dec 21, 2023 • edited Loading

codecov bot commented Dec 21, 2023 • edited Loading

Codecov Report

HuSharp commented Dec 27, 2023

HuSharp commented Dec 28, 2023

HuSharp commented Jan 2, 2024

HuSharp commented Jan 2, 2024

JmPotato Jan 3, 2024

Choose a reason for hiding this comment

HuSharp Jan 3, 2024

Choose a reason for hiding this comment

JmPotato Jan 3, 2024

Choose a reason for hiding this comment

HuSharp Jan 3, 2024

Choose a reason for hiding this comment

JmPotato commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

HuSharp commented Dec 21, 2023 •

edited

Loading

ti-chi-bot bot commented Dec 21, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading