Fix PD --peer-urls flag to use listen_host variable #949
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #949 +/- ##
==========================================
- Coverage 55.86% 52.05% -3.82%
==========================================
Files 263 263
Lines 19513 19513
==========================================
- Hits 10901 10157 -744
- Misses 6883 7713 +830
+ Partials 1729 1643 -86
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
AstroProfundis
approved these changes
Nov 30, 2020
AstroProfundis
added
need-more-test
status/need-doc
ti-chi-bot
added
the
size/XS
fln
force-pushed
the
fix_pd_peer_urls
branch
from
6c6d184 to
c3a66cd
Compare
This commit updates run_pd.sh template to use `listen_host` for --peer-urls flag instead of `host`. This allows us to use server hostname instead of IP addresse. Without this change if PD service is declared using hostname it fails to start, reporting an error that it can not bind to given interface. Metadata variable `listen_host` was introduced in pingcap#495 and is already used for the --client-urls flag. This should resolve pingcap#337 and partially implement pingcap#691.
fln
force-pushed
the
fix_pd_peer_urls
branch
from
c3a66cd to
174b11e
Compare
|
/merge |
ti-chi-bot
added
the
status/can-merge
|
Can merge label has been added. Git tree hash: 6df2551
|
|
@fln: Your PR has out-of-dated and I have automatically updated it for you. 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 tidb-community-bots/ti-community-prow repository. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
This allows us to use server host name instead of IP address. Without
this change if PD service is declared using host name it fails to start,
reporting an error that it can not bind to given interface.
Metadata variable
listen_hostwas introduced in #495 and is already used forthe
--client-urlsflag.This should resolve #337 and partially implement #691 .
What is changed and how it works?
This commit updates
run_pd.shandrun_pd_scale.sh.tpltemplates to uselisten_hostfor--peer-urlsflag insteadof host.Check List
Tests
No code. Tested with integration tests in #950.
Side effects
Breaking backward compatibility
If existing installations relied on host field to be used as address for accepting peer connections this change could be treated as breaking change. Default value of listen_host is 0.0.0.0 so it should not break existing clusters.
Some might want to use different listen addresses for peer and client connections. However I think this would add unnecessary complexity and not worth supporting.
Related changes
Need to update the documentation
Not sure where metadata file is properly documented. So far I have used source code to understand what is expected in the metadata.yaml file.
Release notes: