-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Align docs etc with new discovery setting names #38492
Align docs etc with new discovery setting names #38492
Conversation
In elastic#38333 and elastic#38350 we moved away from the `discovery.zen` settings namespace since these settings have an effect even though Zen Discovery itself is being phased out. This change aligns the documentation and the names of related classes and methods with the newly-introduced naming conventions.
Pinging @elastic/es-distributed |
@@ -14,8 +15,8 @@ include::install_remove.asciidoc[] | |||
[[discovery-azure-classic-usage]] | |||
==== Azure Virtual Machine Discovery | |||
|
|||
Azure VM discovery allows to use the azure APIs to perform automatic discovery (similar to multicast in non hostile | |||
multicast environments). Here is a simple sample configuration: | |||
Azure VM discovery allows to use the azure APIs to perform automatic discovery. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/azure APIs/Azure APIs/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep c1f25d0
@@ -3,6 +3,73 @@ | |||
|
|||
Discovery and cluster formation are affected by the following settings: | |||
|
|||
`discovery.seed_hosts`:: | |||
|
|||
Provides a list of master-eligible nodes in the cluster. The list contains |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just remove the The list contains either an array of hosts or a comma-delimited string
sentence. I think it causes more confusion and distracts from the main message, namely what this setting is about. Linking to an example usage of this setting with the array syntax (which is preferred) is probably more useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sentence deleted and there is a link to the "important settings" page with a full example.
either an array of hosts or a comma-delimited string. Each value has the | ||
format `host:port` or `host`, where `port` defaults to the setting | ||
`transport.profiles.default.port`. Note that IPv6 hosts must be bracketed. | ||
The default value is `127.0.0.1, [::1]`. See <<unicast.hosts>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<<unicast.hosts>>
-> should we rename that as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This renders correctly as See discovery.seed_hosts
so it's only the name of the internal cross-ref that's referring to the old name. I'm not sure what extra hoops we need to jump through to rename these cross-refs so I've left them alone.
@@ -229,7 +233,6 @@ Prefer https://aws.amazon.com/amazon-linux-ami/[Amazon Linux AMIs] as since Elas | |||
|
|||
===== Networking | |||
* Networking throttling takes place on smaller instance types in both the form of https://lab.getbase.com/how-we-discovered-limitations-on-the-aws-tcp-stack/[bandwidth and number of connections]. Therefore if large number of connections are needed and networking is becoming a bottleneck, avoid https://aws.amazon.com/ec2/instance-types/[instance types] with networking labeled as `Moderate` or `Low`. | |||
* Multicast is not supported, even when in an VPC; the aws cloud plugin which joins by performing a security group lookup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it's technically correct that multicast is not supported in EC2 discovery, it's confusing to call it out specifically here: multicast isn't supported anywhere any more. We've been unicast-only for so long that it's getting kinda confusing even to mention it, so this PR removes many instances of the words "unicast" and "multicast".
Sadly not all of them: the file that file-based discovery uses is called unicast_hosts.txt
.
In #38333 and #38350 we moved away from the
discovery.zen
settings namespacesince these settings have an effect even though Zen Discovery itself is being
phased out. This change aligns the documentation and the names of related
classes and methods with the newly-introduced naming conventions.