[Feature] Support set rpc port config #212
Merged
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
|
LGTM. Thanks! |
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.
[BUG]/[FEATURE] title
What changes were proposed in this pull request?
Add worker rpc port config, replace the master & worker arguments port's default value with config from RssConf.
Why are the changes needed?
For better monitor the rpc port workload of worker.
And in previous Master, it will use the value from MasterArguments to create master's rpcEnv, but if user also set the "rss.master.port" or "rss.master.address", it may cause the RssHARetryClient use different port value with Master. Currently if user set master's port by -p/--port in command line when launching master, it may cause RssHARetryClient could not find the real rpc port of master, thus i think may need to find a replacement of current RssHARetryClient's constructor logic or forbid user to set master's port by -p/--port.
What are the items that need reviewer attention?
masterPort used by RssHARetryClient's constructor may conflict with Master's real port.
Related issues.
#211
Related pull requests.
How was this patch tested?
/cc @related-reviewer
/assign @main-reviewer