- Notifications
You must be signed in to change notification settings - Fork 25.6k
Remove PeerFinder request timeout #134365
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
Remove PeerFinder request timeout #134365
Conversation
There's really no need to time out so enthusiastically here, we can wait for as long as it takes to receive a list of other discovery targets from the remote peer. This commit removes the timeout, deferring failure detection down to the TCP level (e.g. keepalives) as managed by the OS. Relates elastic#132713, elastic#123568
| Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
ywangd left a comment
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.
Changes look good to me. I have just a logistic question.
docs/reference/elasticsearch/configuration-reference/discovery-cluster-formation-settings.md Outdated Show resolved Hide resolved
| TimeValue.timeValueMillis(1), | ||
| Setting.Property.NodeScope | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Deprecated |
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 is a deprecation and strictly speaking behaviour change. Do we want to use a different label other than >non-issue?
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 I marked it >deprecation and added a changelog.
…-cluster-formation-settings.md Co-authored-by: Yang Wang <ywangd@gmail.com>
| Hi @DaveCTurner, I've created a changelog YAML for you. Note that since this PR is labelled |
ywangd left a comment
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.
LGTM
There's really no need to time out so enthusiastically here, we can wait for as long as it takes to receive a list of other discovery targets from the remote peer. This commit removes the timeout, deferring failure detection down to the TCP level (e.g. keepalives) as managed by the OS. Relates elastic#132713, elastic#123568
There's really no need to time out so enthusiastically here, we can wait for as long as it takes to receive a list of other discovery targets from the remote peer. This commit removes the timeout, deferring failure detection down to the TCP level (e.g. keepalives) as managed by the OS. Relates elastic#132713, elastic#123568
There's really no need to time out so enthusiastically here, we can wait
for as long as it takes to receive a list of other discovery targets
from the remote peer. This commit removes the timeout, deferring failure
detection down to the TCP level (e.g. keepalives) as managed by the OS.
Relates #132713, #123568