Log when probe succeeds but full connection fails #51304
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
It is permitted for nodes to accept transport connections at addresses other than their publish address, which allows a good deal of flexibility when configuring discovery. However, it is not unusual for users to misconfigure nodes to pick a publish address which is inaccessible to other nodes. We see this happen a lot if the nodes are on different networks separated by a proxy, or if the nodes are running in Docker with the wrong kind of network config. In this case we offer no useful feedback to the user unless they enable TRACE-level logs. It's particularly tricky to diagnose because if we test connectivity between the nodes (using their discovery addresses) then all will appear well. This commit adds a WARN-level log if this kind of misconfiguration is detected: the probe connection has succeeded (to indicate that we are really talking to a healthy Elasticsearch node) but the followup connection attempt fails. It also tidies up some loose ends in `HandshakingTransportAddressConnector`, removing some TODOs that need not be completed, and registering its accidentally-unregistered timeout settings.
DaveCTurner
added
>enhancement
:Distributed/Network
|
Pinging @elastic/es-distributed (:Distributed/Network) |
| logger.trace("[{}] full connection successful: {}", thisConnectionAttempt, remoteNode); | ||
| listener.onResponse(remoteNode); | ||
| })); | ||
| transportService.connectToNode(remoteNode, ActionListener.wrap(ignored -> { |
There was a problem hiding this comment.
Why move to wrap here? We (mostly Henning :)) are currently trying to remove the number of instances of passing broken listeners to transport APIs that don't handle their own exceptions and this seems like a step in the wrong direction. Can we fix the listener to handle its exception instead?
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jan 23, 2020
It is permitted for nodes to accept transport connections at addresses other than their publish address, which allows a good deal of flexibility when configuring discovery. However, it is not unusual for users to misconfigure nodes to pick a publish address which is inaccessible to other nodes. We see this happen a lot if the nodes are on different networks separated by a proxy, or if the nodes are running in Docker with the wrong kind of network config. In this case we offer no useful feedback to the user unless they enable TRACE-level logs. It's particularly tricky to diagnose because if we test connectivity between the nodes (using their discovery addresses) then all will appear well. This commit adds a WARN-level log if this kind of misconfiguration is detected: the probe connection has succeeded (to indicate that we are really talking to a healthy Elasticsearch node) but the followup connection attempt fails. It also tidies up some loose ends in `HandshakingTransportAddressConnector`, removing some TODOs that need not be completed, and registering its accidentally-unregistered timeout settings.
DaveCTurner
deleted the
2020-01-22-HandshakingTransportAddressConnector-fixes
branch
DaveCTurner
added a commit
that referenced
this pull request
Jan 23, 2020
It is permitted for nodes to accept transport connections at addresses other than their publish address, which allows a good deal of flexibility when configuring discovery. However, it is not unusual for users to misconfigure nodes to pick a publish address which is inaccessible to other nodes. We see this happen a lot if the nodes are on different networks separated by a proxy, or if the nodes are running in Docker with the wrong kind of network config. In this case we offer no useful feedback to the user unless they enable TRACE-level logs. It's particularly tricky to diagnose because if we test connectivity between the nodes (using their discovery addresses) then all will appear well. This commit adds a WARN-level log if this kind of misconfiguration is detected: the probe connection has succeeded (to indicate that we are really talking to a healthy Elasticsearch node) but the followup connection attempt fails. It also tidies up some loose ends in `HandshakingTransportAddressConnector`, removing some TODOs that need not be completed, and registering its accidentally-unregistered timeout settings.
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (elastic#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
DaveCTurner
added a commit
that referenced
this pull request
Jan 28, 2021
The following settings are not exposed to users in 7.6 and earlier: - `discovery.probe.connect_timeout` - `discovery.probe.handshake_timeout` This was addressed in 7.7 (#51304) but the docs for older versions suggest incorrectly that these settings are available. This commit removes the docs for these settings in the affected versions to avoid confusion.
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.
It is permitted for nodes to accept transport connections at addresses other
than their publish address, which allows a good deal of flexibility when
configuring discovery. However, it is not unusual for users to misconfigure
nodes to pick a publish address which is inaccessible to other nodes. We see
this happen a lot if the nodes are on different networks separated by a proxy,
or if the nodes are running in Docker with the wrong kind of network config.
In this case we offer no useful feedback to the user unless they enable
TRACE-level logs. It's particularly tricky to diagnose because if we test
connectivity between the nodes (using their discovery addresses) then all will
appear well.
This commit adds a WARN-level log if this kind of misconfiguration is detected:
the probe connection has succeeded (to indicate that we are really talking to a
healthy Elasticsearch node) but the followup connection attempt fails.
It also tidies up some loose ends in
HandshakingTransportAddressConnector,removing some TODOs that need not be completed, and registering its
accidentally-unregistered timeout settings.