*: add service subcommand #5971

rleungx · 2023-02-13T06:25:57Z

What problem does this PR solve?

Issue Number: Ref #5836.

What is changed and how does it work?

This PR has a similar functionality to #5964 but uses the independent config for starting the TSO server. Besides, we can use the same function to init log for all binaries now.
Update:
Replace the original implementation with the cobra.

Check List

Tests

Unit test

Release note

None

ti-chi-bot · 2023-02-13T06:25:58Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

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

ti-chi-bot · 2023-02-13T06:25:58Z

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

codecov · 2023-02-13T06:55:50Z

Codecov Report

Base: 75.07% // Head: 75.04% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (a4f317c) compared to base (29520a3).
Patch coverage: 65.43% of modified lines in pull request are covered.

❗ Current head a4f317c differs from pull request most recent head 7bbde8c. Consider uploading reports for the commit 7bbde8c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5971      +/-   ##
==========================================
- Coverage   75.07%   75.04%   -0.03%     
==========================================
  Files         362      362              
  Lines       36283    36300      +17     
==========================================
+ Hits        27238    27243       +5     
- Misses       6657     6673      +16     
+ Partials     2388     2384       -4

Flag	Coverage Δ
unittests	`75.04% <65.43%> (-0.03%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/tso/config.go	`0.00% <0.00%> (ø)`
server/config/config.go	`75.74% <74.07%> (+0.33%)`	⬆️
pkg/utils/logutil/log.go	`81.57% <75.00%> (-1.76%)`	⬇️
tests/config.go	`93.65% <91.30%> (-1.48%)`	⬇️
server/server.go	`74.74% <100.00%> (+1.42%)`	⬆️
server/testutil.go	`90.62% <100.00%> (ø)`
tests/cluster.go	`79.27% <100.00%> (ø)`
server/api/diagnostic.go	`76.92% <0.00%> (-23.08%)`	⬇️
pkg/mcs/discovery/register.go	`53.48% <0.00%> (-18.61%)`	⬇️
server/region_syncer/server.go	`82.96% <0.00%> (-4.40%)`	⬇️
... and 24 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.

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

lhy1024 · 2023-02-13T08:46:35Z

/merge

ti-chi-bot · 2023-02-13T08:46:36Z

@lhy1024: 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-02-13T08:46:38Z

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

Commit hash: e849dd5

rleungx · 2023-02-13T08:52:30Z

/hold

rleungx · 2023-02-15T07:07:24Z

/hold cancel

rleungx · 2023-02-15T07:08:25Z

Since the PR has been rewritten, need two more LGTM before merging.

binshi-bing · 2023-02-15T07:20:40Z

cmd/pd-server/main.go

+	cmd := &cobra.Command{
+		Use:   "tso [args]",
+		Short: "run a tso service",
+		Run:   tso.CreateServerWrapper,


so this is why we removed the return data types and have to duplicate server run() and exit code. LGTM

binshi-bing

Looks like with Cobra, we have to duplicate server.run and exit code. LGTM

ti-chi-bot · 2023-02-15T07:21:31Z

@binshi-bing: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

Looks like with Cobra, we have to duplicate server.run and exit code. LGTM

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.

lhy1024 · 2023-02-15T07:28:40Z

Looks like with Cobra, we have to duplicate server.run and exit code. LGTM

I think we can will move some functions to util.go after ensuring todo in tso server

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

rleungx · 2023-02-15T08:59:04Z

@JmPotato @nolouch Please take a look when you are available.

nolouch · 2023-02-15T10:38:38Z

/merge

ti-chi-bot · 2023-02-15T10:38:39Z

@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-02-15T10:38:40Z

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

Commit hash: d131cf5

ti-chi-bot · 2023-02-15T11:14:13Z

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

ref #5836, ref #5971 Signed-off-by: Ryan Leung <rleungx@gmail.com>

ref tikv#5836, ref tikv#5964 Signed-off-by: Ryan Leung <rleungx@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Co-authored-by: lhy1024 <admin@liudos.us>

ref tikv#5836, ref tikv#5971 Signed-off-by: Ryan Leung <rleungx@gmail.com>

ti-chi-bot requested review from lhy1024 and nolouch February 13, 2023 06:26

rleungx force-pushed the cmd branch from c4687c1 to e93169e Compare February 13, 2023 06:32

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2023

rleungx force-pushed the cmd branch from e93169e to 55cb240 Compare February 13, 2023 06:41

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 13, 2023

rleungx marked this pull request as ready for review February 13, 2023 06:44

ti-chi-bot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-linked-issue labels Feb 13, 2023

rleungx requested a review from JmPotato February 13, 2023 07:01

JmPotato approved these changes Feb 13, 2023

View reviewed changes

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

lhy1024 approved these changes Feb 13, 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 Feb 13, 2023

lhy1024 added a commit to lhy1024/pd that referenced this pull request Feb 13, 2023

use tikv#5971 and move client

53eedb5

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

rleungx mentioned this pull request Feb 13, 2023

Separate TSO into an independent service #5836

Closed

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

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

rleungx removed the status/LGT2 Indicates that a PR has LGTM 2. label Feb 15, 2023

ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 15, 2023

lhy1024 approved these changes Feb 15, 2023

View reviewed changes

binshi-bing reviewed Feb 15, 2023

View reviewed changes

binshi-bing approved these changes Feb 15, 2023

View reviewed changes

use cobra to manage command line

d131cf5

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

rleungx force-pushed the cmd branch from a4f317c to d131cf5 Compare February 15, 2023 07:37

JmPotato approved these changes Feb 15, 2023

View reviewed changes

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

ti-chi-bot and others added 3 commits February 15, 2023 18:38

Merge branch 'master' into cmd

83811cb

Merge branch 'master' into cmd

fa9dea8

Merge branch 'master' into cmd

7bbde8c

ti-chi-bot merged commit f1e3bf5 into tikv:master Feb 15, 2023

purelind mentioned this pull request Feb 15, 2023

fix: update pd start cmd args PingCAP-QE/ci#1790

Merged

rleungx deleted the cmd branch February 16, 2023 02:26

rleungx mentioned this pull request Feb 16, 2023

*: fix the default value of peer-urls #5996

Merged

ti-chi-bot pushed a commit that referenced this pull request Feb 16, 2023

*: fix the default value of peer-urls (#5996)

c499e01

ref #5836, ref #5971 Signed-off-by: Ryan Leung <rleungx@gmail.com>

purelind mentioned this pull request Feb 16, 2023

fix: pd use the complete flag with -- tikv/copr-test#166

Merged

nolouch pushed a commit to nolouch/pd that referenced this pull request Feb 24, 2023

*: fix the default value of peer-urls (tikv#5996)

b896543

ref tikv#5836, ref tikv#5971 Signed-off-by: Ryan Leung <rleungx@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add service subcommand #5971

*: add service subcommand #5971

rleungx commented Feb 13, 2023 •

edited

Loading

ti-chi-bot commented Feb 13, 2023 •

edited

Loading

ti-chi-bot commented Feb 13, 2023

codecov bot commented Feb 13, 2023 •

edited

Loading

lhy1024 commented Feb 13, 2023

ti-chi-bot commented Feb 13, 2023

ti-chi-bot commented Feb 13, 2023

rleungx commented Feb 13, 2023

rleungx commented Feb 15, 2023

rleungx commented Feb 15, 2023

binshi-bing Feb 15, 2023

binshi-bing left a comment

ti-chi-bot commented Feb 15, 2023

lhy1024 commented Feb 15, 2023

rleungx commented Feb 15, 2023

nolouch commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

*: add service subcommand #5971

*: add service subcommand #5971

Conversation

rleungx commented Feb 13, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Feb 13, 2023 • edited Loading

ti-chi-bot commented Feb 13, 2023

codecov bot commented Feb 13, 2023 • edited Loading

Codecov Report

lhy1024 commented Feb 13, 2023

ti-chi-bot commented Feb 13, 2023

ti-chi-bot commented Feb 13, 2023

rleungx commented Feb 13, 2023

rleungx commented Feb 15, 2023

rleungx commented Feb 15, 2023

binshi-bing Feb 15, 2023

Choose a reason for hiding this comment

binshi-bing left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Feb 15, 2023

lhy1024 commented Feb 15, 2023

rleungx commented Feb 15, 2023

nolouch commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

rleungx commented Feb 13, 2023 •

edited

Loading

ti-chi-bot commented Feb 13, 2023 •

edited

Loading

codecov bot commented Feb 13, 2023 •

edited

Loading