Negotiate disordered transport handshakes #123397
Merged
Conversation
This file contains hidden or 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
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications.
DaveCTurner
added
>non-issue
:Core/Infra/Transport API
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
rjernst
approved these changes
Feb 27, 2025
| return this; | ||
| } | ||
| TransportVersion bestSoFar = TransportVersions.ZERO; | ||
| for (final var knownVersion : VersionsHolder.ALL_VERSIONS_MAP.values()) { |
There was a problem hiding this comment.
At one point we had the map as a NavigableMap, which would allow quickly finding the closest known version. Is the linear performance here "ok" because handshakes are only done on initial connection?
There was a problem hiding this comment.
Yep we hardly ever open new connections.
| final var bestKnownVersion = remoteTransportVersion.bestKnownVersion(); | ||
| if (bestKnownVersion.equals(TransportVersions.ZERO) == false) { | ||
| if (bestKnownVersion.equals(remoteTransportVersion) == false) { | ||
| // Remote is older than us, and we do not know its exact transport protocol version, so we choose a still-older version |
There was a problem hiding this comment.
nit: older is ambiguous, this should be numerically-older right? Also, maybe use semantically-older since we generally talk about these as semantic versions?
There was a problem hiding this comment.
++ updated in 184ada9
DaveCTurner
added
auto-merge-without-approval
💔 Backport failed
You can use sqren/backport to manually backport by running |
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Feb 28, 2025
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of elastic#123397 to 9.0
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Feb 28, 2025
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of elastic#123397 to 8.x
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Feb 28, 2025
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of elastic#123397 to 8.18
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Mar 1, 2025
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of elastic#123397 to 8.x
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 1, 2025
Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of #123397 to 9.0
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 1, 2025
* Negotiate disordered transport handshakes Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of #123397 to 8.x * Negotiate disordered transport handshakes Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of #123397 to 8.x * Fix test * Fix for pre-8.9 versions
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 1, 2025
* Negotiate disordered transport handshakes Previously we'd reject transport handshakes from numerically-older nodes with a chronologically-newer release date. With this commit we instead negotiate to use the latest known version for future communications. Backport of #123397 to 8.18 * Fix test * Fix for pre-8.9 versions
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Mar 1, 2025
In backporting elastic#123397 to 8.x we discovered that we needed to generalize `TransportVersion#bestKnownVersion` slightly to handle older versions. This commit forward-ports this change to keep the branches aligned.
DaveCTurner
added a commit
that referenced
this pull request
Mar 4, 2025
In backporting #123397 to 8.x we discovered that we needed to generalize `TransportVersion#bestKnownVersion` slightly to handle older versions. This commit forward-ports this change to keep the branches aligned.
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Mar 4, 2025
In backporting elastic#123397 to 8.x we discovered that we needed to generalize `TransportVersion#bestKnownVersion` slightly to handle older versions. This commit forward-ports this change to keep the branches aligned.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 4, 2025
In backporting #123397 to 8.x we discovered that we needed to generalize `TransportVersion#bestKnownVersion` slightly to handle older versions. This commit forward-ports this change to keep the branches aligned.
georgewallace
pushed a commit
to georgewallace/elasticsearch
that referenced
this pull request
Mar 11, 2025
In backporting elastic#123397 to 8.x we discovered that we needed to generalize `TransportVersion#bestKnownVersion` slightly to handle older versions. This commit forward-ports this change to keep the branches aligned.
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates elastic#123397
DaveCTurner
added a commit
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates #123397
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates elastic#123397
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates elastic#123397 Backport of elastic#129904 to `8.19`
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates #123397
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates #123397 Backport of #129904 to `8.19`
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates elastic#123397 Backport of elastic#129904 to `8.19`
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 24, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates #123397 Backport of #129904 to `8.19`
mridula-s109
pushed a commit
to mridula-s109/elasticsearch
that referenced
this pull request
Jun 25, 2025
The comment in `TransportHandshaker` indicates (correctly) that we emit a warning when talking to a chronologically-newer-yet-numerically-older version, but the wording of the warning message is inverted and says that the remote is chronologically-older-yet-numerically-newer. This commit straightens out the message to match the situation it is describing. Relates elastic#123397
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Previously we'd reject transport handshakes from numerically-older nodes
with a chronologically-newer release date. With this commit we instead
negotiate to use the latest known version for future communications.