Respond with same compression scheme received #76372
Conversation
Tim-Brooks
added
>enhancement
:Distributed Coordination/Network
elasticmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
LGTM.
I think we should add the compression scheme setting to docs for remote clusters: (edit: now saw the other PR that is already merged, thanks!)
https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-remote-clusters.html#remote-cluster-settings
(follow-up is fine, perhaps together with removing experimental flag on the setting(s)).
| } | ||
|
|
||
| private static boolean uncompressedOrSchemeDefinited(Header header) { | ||
| return header.isCompressed() == false || header.getCompressionScheme() != null; |
There was a problem hiding this comment.
Can this be tightened to:
| return header.isCompressed() == false || header.getCompressionScheme() != null; | |
| return header.isCompressed() == (header.getCompressionScheme() != null); |
| final Compression.Scheme compressionScheme = header.getCompressionScheme(); | ||
| if (header.isCompressed()) { | ||
| assertNotNull(compressionScheme); | ||
| } |
There was a problem hiding this comment.
Can we add else assertNull(compressionScheme); ? (or embed it into the other assertion).
| final TransportResponse response, final Compression.Scheme compressionScheme, final boolean isHandshake) | ||
| throws IOException { | ||
| Version version = Version.min(this.version, nodeVersion); | ||
| final Compression.Scheme compressionScheme; |
There was a problem hiding this comment.
I wonder if we can now assert that if compressionScheme == LZ4 we are on a new enough version. Something like:
assert compressionScheme != Compression.Scheme.LZ4 || version.onOrAfter(Compression.Scheme.LZ4_VERSION);
I am sure this will break a test or two and I am OK with deferring this if it adds too much trouble.
This is related to elastic#73497. Currently, we only use the configured transport.compression_scheme setting when compressing a request or a response. Additionally, the cluster.remote.*.compression_scheme setting is ignored. This commit fixes this behavior by respecting the per-cluster setting. Additionally, it resolves confusion around inbound and outbound connections by always responding with the same scheme that was received. This allows remote connections to have different schemes than local connections.
4 participants
This is related to #73497. Currently, we only use the configured
transport.compression_schemesetting when compressing a request or aresponse. Additionally, the
cluster.remote.*.compression_schemesetting is ignored. This commit fixes this behavior by respecting the
per-cluster setting. Additionally, it resolves confusion around inbound
and outbound connections by always responding with the same scheme that
was received. This allows remote connections to have different schemes
than local connections.