Make the the number of ports to scan when trying to join a cluster configurable, and set the default to 2.#8833
Make the the number of ports to scan when trying to join a cluster configurable, and set the default to 2.#8833ejain wants to merge 1 commit intoelastic:masterfrom zenobase:master
Conversation
configurable, and set the default to 2. Closes #7090
|
@ejain sorry for the long wait. Will you be willing to pick this one up again? It looks good,but misses a test (maybe expose configuredTargetNode as a package private getter and add a unit test UnicastZenPingTests). |
|
I'd be happy to add a test, but since this isn't a trivial unit test, and I'm not familiar with elasticsearch's test scaffolding, it might be a while before I get to have a look. |
|
@ejain I think we can make it a basic unit test (inheriting from ElasticsearchTestCase ), with my suggestion above:
That means you just need to instantiate UnicastZenPing with two different settings and see that number of hosts is good, plus that the ports is what you expect. Let me know if you don't have time for this (it has been a while). I can pick it up , but please do sign the CLA so I can take your PR as a base (see https://www.elastic.co/contributing-to-elasticsearch ) |
|
Signed the CLA. Feel free to pick this up, could be a few weeks before I'm able to get back to this. |
|
I've just followed half a dozen issues through to this one which appears to be the leading edge of this problem. Since the current state of play has not been changed in several years, can you at least correct the public documentation which still clearly describes using port ranges although everyone on the ES side knows they are ignored. |
|
@drekbour agreed the documentation is confusing. Parsing ranges is mostly there to be able to work out of the box with the default transport settings which use 9300-9400 . Not sure much for external configuration. Agreed it's super confusing now. Do you mind opening a new issue as this PR tries to do something else, which is make it configurable. I believe that's the wrong approach. We should document what we do - not support ranges when specified and default to the first 5 local ports when not configured. |
|
The networking code has changed considerably, and the documentation has been updated to reflect the current state. Closing |
Closes #7090