Add deprecation warning to TransportHandshaker #123188

JVerwolf · 2025-02-21T20:33:47Z

Jira: ES-10547

This PR adds a deprecation warning log to the TransportHandshaker when connecting with nodes < v8.18.

elasticsearchmachine · 2025-02-21T20:34:38Z

Hi @JVerwolf, I've created a changelog YAML for you. Note that since this PR is labelled >deprecation, you need to update the changelog YAML to fill out the extended information sections.

…thub.com:JVerwolf/elasticsearch into deprication/add-deprication-warning-to-handshake

DaveCTurner · 2025-02-22T07:14:42Z

To fix the TransportHandshakerRawMessageTests I would recommend emitting this deprecation in a context that doesn't propagate response headers back over the wire (e.g. under a stashContext() I think?). There's no point in sending the deprecation warning back to the caller IMO - opening a transport connection is a very internal thing. Also, keeping the handshake as simple as possible will make future changes easier if ever a future change is needed.

JVerwolf · 2025-02-24T16:05:32Z

I would recommend emitting this deprecation in a context that doesn't propagate response headers back over the wire

Thanks @DaveCTurner. I'm unfamiliar with how the response headers are logged/reported on the sending node. I'm curious - since this work was motivated by CCR/CCS use-cases, would sending the headers back to the node that was the source of the request be useful here? Or, even if it is, would the headers still be adding complexity to the handshake to the point that this complexity outweigh the benefits?

DaveCTurner · 2025-02-24T16:19:47Z

would sending the headers back to the node that was the source of the request be useful here?

This PR doesn't directly relate to an actual CCR/CCS request, it's just about the transport handshake. It's true that the transport handshake might under some circumstances be triggered by a cross-cluster request, but often it's just going to be a background thing opening this connection. And most of the time a cross-cluster request will be using a connection that is already established so it won't get the warning anyway.

If you want to see warnings on each CCR/CCS request, you probably need to implement them in the source cluster's RemoteClusterService (by looking at the TransportVersion of the underlying connection). Or maybe in org.elasticsearch.transport.RemoteConnectionManager#wrapConnectionWithRemoteClusterInfo? But tbh this sounds like unnecessary work to me, I have never heard of anyone looking at the warning headers returned to clients, and there are lots of other ways that we have folks hitting deprecated functionality without triggering such a headers, so IMO the important thing is to have the deprecation recorded in the deprecation logs.

…hake

…ion/add-deprication-warning-to-handshake

elasticsearchmachine · 2025-02-24T18:07:07Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

JVerwolf · 2025-02-26T15:52:00Z

Thanks for the info/context @DaveCTurner. That make sense, I've removed the warning header from the response.

rjernst

Looks fine to me, but please let @DaveCTurner have a final look.

rjernst · 2025-02-26T15:57:39Z

docs/changelog/123188.yaml

+deprecation:
+  title: Add deprecation warning to `TransportHandshaker`
+  area: REST API
+  details: This PR adds a deprecation log to versions < v8.18.


nit: the details are more what you have in impact. The impact is that users should upgrade any clusters/nodes older than 8.18 in preparation for upgrading to 9.0.

Yes I think this needs more detail. Whatever we write here gets converted into fairly prominent text in the release notes, we should spell out in detail what this change means to avoid causing any confusion to users.

TIL, thanks! I've updated the notes, WDYT?

DaveCTurner

Just a few nits. Also this needs to go into 8.x as well as 8.18.

DaveCTurner · 2025-02-26T16:18:43Z

docs/changelog/123188.yaml

+deprecation:
+  title: Add deprecation warning to `TransportHandshaker`
+  area: REST API
+  details: This PR adds a deprecation log to versions < v8.18.


Yes I think this needs more detail. Whatever we write here gets converted into fairly prominent text in the release notes, we should spell out in detail what this change means to avoid causing any confusion to users.

DaveCTurner · 2025-02-26T16:22:20Z

server/src/test/java/org/elasticsearch/transport/TransportHandshakerTests.java


-        MockLog.assertThatLogger(
+        MockLog.awaitLogger(


Hmm why did this need to be changed from an immediate assertion?

In a prior iteration it was failing when the deprecation log was hit first (I had thought this was because the loggers share the same name, as appears to be convention via other examples in the codebase). However, this is no longer happening - I set this up in a debugger to be sure. I'm not aware of what caused the behaviour difference, but I will revert this to assertThatLogger now that this is no longer causing an issue.

DaveCTurner · 2025-02-26T16:23:28Z

server/src/test/java/org/elasticsearch/transport/TransportHandshakerTests.java

    }

-    private static TransportVersion getRandomIncompatibleTransportVersion() {
-        return randomBoolean()
+    private static TransportVersion getRandomIncompatibleTransportVersion(boolean olderThanMinCompatible) {


Is this new parameter needed? It looks like we just pass in a randomBoolean() and don't otherwise care?

This is a leftover from a prior iteration, I'll remove it, thanks!

…thub.com:JVerwolf/elasticsearch into deprication/add-deprication-warning-to-handshake

…hake

DaveCTurner

Editorial nits only :)

docs/changelog/123188.yaml

DaveCTurner · 2025-02-26T19:46:30Z

docs/changelog/123188.yaml

+    Users will now see a deprecation log when a remote node, who's version is < v8.18, performs a transport handshake
+    request with nodes >= v8.18 (e.g. during a CCS/CCR request). The remote cluster will need to be upgraded to >= v8.18
+    to maintain compatibility with clusters that have been upgraded to v9.


Maybe something like this?

Suggested change

Users will now see a deprecation log when a remote node, who's version is < v8.18, performs a transport handshake

request with nodes >= v8.18 (e.g. during a CCS/CCR request). The remote cluster will need to be upgraded to >= v8.18

to maintain compatibility with clusters that have been upgraded to v9.

Versions 9.0.0 and later of {es} will not support communication with nodes of versions earlier than 8.18.0,

so the ability to connect to nodes of earlier versions is deprecated in this version. This applies both to

communication within a cluster and communication across clusters (e.g. for CCS or CCR).

{es} will report in its deprecation logs each time it opens a connection to a node that will not be supported

from version 9.0.0 onwards. You must upgrade all your clusters to version 8.18.0 or later before upgrading

any of your clusters to 9.0.0 or later.

Bonus points for turning across clusters, CCS, CCR and deprecation logs into links to the relevant docs.

Thank you!

Bonus points for turning across clusters, CCS, CCR and deprecation logs into links to the relevant docs.

Are you familiar with how to add a link here? (I skimmed the change logs and found no examples). Is this converted to markdown? Will the regular [link](url) apply?

This is Asciidoc so [xpack-ccr] should link here for instance.

Bah sorry wrong syntax, I meant <<xpack-ccr,{ccr}>>

Much appreciated!

docs/changelog/123188.yaml

Co-authored-by: David Turner <david.turner@elastic.co>

…hake

JVerwolf · 2025-02-26T22:11:54Z

@elasticmachine run elasticsearch-ci/part-2

…hake

elasticsearchmachine · 2025-02-27T16:53:09Z

💚 Backport successful

Status	Branch	Result
✅	8.x

This PR adds a deprecation warning log to the TransportHandshaker when connecting with nodes < v8.18.

This PR adds a deprecation warning log to the TransportHandshaker when connecting with nodes < v8.19.

JVerwolf and others added 5 commits February 21, 2025 12:30

Add deprecation warning to handshaker

d7eb582

Update docs/changelog/123168.yaml

b2814b4

Update 123168.yaml

4e99ab7

update changelog

653998d

Refactor and fix tests

Loading
Loading status checks…

e289a6f

JVerwolf added :Core/Infra/Core >deprecation labels Feb 21, 2025

JVerwolf requested review from rjernst, DaveCTurner and a team February 21, 2025 20:33

elasticsearchmachine added the v8.18.1 label Feb 21, 2025

Update docs/changelog/123188.yaml

Loading
Loading status checks…

56993a4

JVerwolf mentioned this pull request Feb 21, 2025

Add deprecation warning to TransportHandshaker #123168

Closed

JVerwolf and others added 3 commits February 21, 2025 12:37

Update 123188.yaml

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

b00fd1e

remove yaml file

83cd121

Merge branch 'deprication/add-deprication-warning-to-handshake' of gi…

Loading
Loading status checks…

124d88a

…thub.com:JVerwolf/elasticsearch into deprication/add-deprication-warning-to-handshake

JVerwolf and others added 3 commits February 24, 2025 08:48

PR feedback: remove deprecation header

Loading
Loading status checks…

1f66e84

Merge branch '8.18' into deprication/add-deprication-warning-to-hands…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

5339a70

…hake

Merge branch '8.18' of github.com:elastic/elasticsearch into depricat…

54f6089

…ion/add-deprication-warning-to-handshake

JVerwolf marked this pull request as ready for review February 24, 2025 18:06

elasticsearchmachine added the Team:Core/Infra label Feb 24, 2025

rjernst approved these changes Feb 26, 2025

View reviewed changes

DaveCTurner reviewed Feb 26, 2025

View reviewed changes

JVerwolf added the auto-backport label Feb 26, 2025

JVerwolf added 3 commits February 26, 2025 10:51

PR feedback: update changelog

7562f95

PR feedback: update tests

06dce91

Merge branch 'deprication/add-deprication-warning-to-handshake' of gi…

Loading
Loading status checks…

e25ee9f

…thub.com:JVerwolf/elasticsearch into deprication/add-deprication-warning-to-handshake

JVerwolf added the v8.19.0 label Feb 26, 2025

Merge branch '8.18' into deprication/add-deprication-warning-to-hands…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

b32106b

…hake

JVerwolf requested a review from DaveCTurner February 26, 2025 19:12

DaveCTurner approved these changes Feb 26, 2025

View reviewed changes

JVerwolf and others added 5 commits February 26, 2025 11:50

PR feedback: update release notes

Loading
Loading status checks…

f29dc09

Merge branch '8.18' into deprication/add-deprication-warning-to-hands…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

8892e10

…hake

Merge branch '8.18' into deprication/add-deprication-warning-to-hands…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

0909e1e

…hake

JVerwolf merged commit 04917e1 into elastic:8.18 Feb 27, 2025
15 checks passed

JVerwolf mentioned this pull request Feb 27, 2025

[8.x] Add deprecation warning to TransportHandshaker (#123188) #123627

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deprecation warning to TransportHandshaker #123188

Add deprecation warning to TransportHandshaker #123188

JVerwolf commented Feb 21, 2025 •

edited

Loading

elasticsearchmachine commented Feb 21, 2025

DaveCTurner commented Feb 22, 2025

JVerwolf commented Feb 24, 2025

DaveCTurner commented Feb 24, 2025

elasticsearchmachine commented Feb 24, 2025

JVerwolf commented Feb 26, 2025

rjernst left a comment

rjernst Feb 26, 2025

DaveCTurner Feb 26, 2025

JVerwolf Feb 26, 2025

DaveCTurner left a comment

DaveCTurner Feb 26, 2025

DaveCTurner Feb 26, 2025

JVerwolf Feb 26, 2025 •

edited

Loading

DaveCTurner Feb 26, 2025

JVerwolf Feb 26, 2025

DaveCTurner left a comment

DaveCTurner Feb 26, 2025

JVerwolf Feb 26, 2025 •

edited

Loading

DaveCTurner Feb 26, 2025

DaveCTurner Feb 26, 2025

JVerwolf Feb 26, 2025

JVerwolf commented Feb 26, 2025

elasticsearchmachine commented Feb 27, 2025

-    Users will now see a deprecation log when a remote node, who's version is < v8.18, performs a transport handshake
-    request with nodes >= v8.18 (e.g. during a CCS/CCR request). The remote cluster will need to be upgraded to >= v8.18
-    to maintain compatibility with clusters that have been upgraded to v9.
+    Versions 9.0.0 and later of {es} will not support communication with nodes of versions earlier than 8.18.0,
+    so the ability to connect to nodes of earlier versions is deprecated in this version. This applies both to
+    communication within a cluster and communication across clusters (e.g. for CCS or CCR).
+    {es} will report in its deprecation logs each time it opens a connection to a node that will not be supported
+    from version 9.0.0 onwards. You must upgrade all your clusters to version 8.18.0 or later before upgrading
+    any of your clusters to 9.0.0 or later.

Add deprecation warning to TransportHandshaker #123188

Add deprecation warning to TransportHandshaker #123188

Conversation

JVerwolf commented Feb 21, 2025 • edited Loading

elasticsearchmachine commented Feb 21, 2025

DaveCTurner commented Feb 22, 2025

JVerwolf commented Feb 24, 2025

DaveCTurner commented Feb 24, 2025

elasticsearchmachine commented Feb 24, 2025

JVerwolf commented Feb 26, 2025

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JVerwolf Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JVerwolf Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JVerwolf commented Feb 26, 2025

elasticsearchmachine commented Feb 27, 2025

💚 Backport successful

JVerwolf commented Feb 21, 2025 •

edited

Loading

JVerwolf Feb 26, 2025 •

edited

Loading

JVerwolf Feb 26, 2025 •

edited

Loading