Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: correct the explanation of status-host #13315

Closed
wants to merge 2 commits into from
Closed

config: correct the explanation of status-host #13315

wants to merge 2 commits into from

Conversation

ericsyh
Copy link
Contributor

@ericsyh ericsyh commented Nov 9, 2019

What problem does this PR solve?

Revise the PR #13024 which add API example for prometheus and pprof.

What is changed and how it works?

Put the API example to the correct position.

Check List

Tests

  • No code

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch

@codecov
Copy link

codecov bot commented Nov 9, 2019

Codecov Report

Merging #13315 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13315   +/-   ##
===========================================
  Coverage   80.1531%   80.1531%           
===========================================
  Files           469        469           
  Lines        112068     112068           
===========================================
  Hits          89826      89826           
  Misses        15294      15294           
  Partials       6948       6948

@@ -135,12 +135,12 @@ cluster-ssl-key = ""
# If enable status report HTTP service.
report-status = true

# TiDB status host.
status-host = "0.0.0.0"

## status-host is the HTTP address for reporting the internal status of a TiDB server, for example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not only for the status-host but also for status-port.

@@ -135,12 +135,12 @@ cluster-ssl-key = ""
# If enable status report HTTP service.
report-status = true

# TiDB status host.
status-host = "0.0.0.0"

## status-host is the HTTP address for reporting the internal status of a TiDB server, for example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## status-host is the HTTP address for reporting the internal status of a TiDB server, for example:
## status-host and status-port together specify the HTTP address for reporting the internal status of a TiDB server, for example:

## status-host is the HTTP address for reporting the internal status of a TiDB server, for example:
## API for prometheus: http://${status-host}:${status_port}/metrics
## API for pprof: http://${status-host}:${status_port}/debug/pprof
# TiDB status host.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually TiDB status host only have one more information than status-host, which is tidb... I think we can remove this useless comment. So as the tidb status port comment.

We should explain more about the status-host and status-port in the above comment block.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 5, 2020

@ericsyh, please update your pull request.

1 similar comment
@sre-bot
Copy link
Contributor

sre-bot commented Jan 12, 2020

@ericsyh, please update your pull request.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 28, 2020

@ericsyh PR closed due to no update for a long time. Feel free to reopen it anytime.

@sre-bot sre-bot closed this Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants