Add support for rest compatibility headers to the HLRC #78490
Conversation
This adds support for the headers necessary for REST version compatibility to the Low Level Rest Client (LLRC). Compatibility mode can be turned on either with the `.setAPICompatibilityMode(true)` when creating the client, or by setting the `ELASTIC_CLIENT_APIVERSIONING` to `true` similar to our other Elasticsearch clients. Resolves elastic#77859
dakrone
added
>enhancement
:Clients/Java Low Level REST Client
elasticmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
client/rest/src/main/java/org/elasticsearch/client/RestClient.java
Outdated
Show resolved
Hide resolved
client/rest/src/main/java/org/elasticsearch/client/RestClient.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We (clients team) have been working on migration scenarios from the now deprecated HLRC and the new Java client. And it turns out that implementing compatibility mode in the LLRC forbids a number of useful scenarios.
A key element for the adoption of the new client is to avoid big-bang migrations and allow an application to use both clients at the same time with no operational overhead beyond adding a library dependency. To avoid operational overhead, an application should be able to have HLRC and the new Java client coexist and share the same LLRC instance, so that we do not double the number of connections to the ES cluster.
Here's a concrete scenario:
- user has a large application using HLRC running with a 7.x cluster.
- they enable compatibility mode on LLRC mode and upgrade the cluster to 8.x
- they want to use some new 8.0-only APIs and start using the new Java client that exposes them, while keeping existing code with HLRC unchanged.
- they setup the new Java client with the same LLRC used by HLRC.
- since that LLRC has compatibility mode enabled, ES rejects 8.0-only calls sent by the new client.
So we really need compatibility mode to be a high-level client setting and have to revisit this PR in that direction. I'm sorry that this realization comes when the PR is almost ready.
Fortunately we don't have to restart from scratch, and can move the code that processes request's Content-Type and Accept headers to RestHighLevelClient.performClientRequest[Async]. Impacts on the test framework should be limited too.
To enable compat mode, we can introduce a RestHighLevelClientBuilder with setApiCompatibilityMode(boolean enabled) and setRestClient(RestClient restClient) setters and the corresponding constructor in HighLevelClient, keeping the existing ones for backwards compatibility.
Note that the setter accepts a fully-formed client and not a RestClientBuilder, to allow the low level client to be created beforehand and shared between HRLC and the new Java client whithout having to get it with RestHighLevelClient.getLowLevelClient().
There was a problem hiding this comment.
Thanks for having updated the PR quickly! It looks good, except for header name comparison which should be case insensitive as per the HTTP specification.
We may want to use fancy casing in tests, e.g. cOntEnt-TypE to make sure all equality tests are case insensitive.
| * Return true if the options contain the given header | ||
| */ | ||
| public boolean containsHeader(String name) { | ||
| return headers.stream().anyMatch(h -> name.equals(h.getName())); |
There was a problem hiding this comment.
HTTP headers are case-insentitive. We should use equalsIgnoreCase here and in removeHeader().
| // Modify any existing "Content-Type" headers on the request to use the version compatibility, if available | ||
| boolean contentTypeModified = false; | ||
| for (Header header : newOptions.getHeaders()) { | ||
| if (headerName.equals(header.getName()) == false) { |
There was a problem hiding this comment.
As for RequestOptions, equals should be equalsIgnoreCase.
|
@elasticmachine update branch |
dakrone
changed the title
This adds support for the headers necessary for REST version compatibility to the High Level Rest Client (HLRC). Compatibility mode can be turned on either with the .setAPICompatibilityMode(true) when creating the client, or by setting the ELASTIC_CLIENT_APIVERSIONING to true similar to our other Elasticsearch clients. Resolves elastic#77859
dakrone
added
:Data Management/Java High Level REST Client
and removed
:Clients/Java Low Level REST Client
This adds support for the headers necessary for REST version compatibility to the High Level Rest Client (HLRC). Compatibility mode can be turned on either with the .setAPICompatibilityMode(true) when creating the client, or by setting the ELASTIC_CLIENT_APIVERSIONING to true similar to our other Elasticsearch clients. Resolves #77859
* master: Fix DataTierTests package and add a validation test (elastic#78880) Fix split package org.elasticsearch.common.xcontent (elastic#78831) Store DataTier Preference directly on IndexMetadata (elastic#78668) [DOCS] Fixes typo in calendar API example (elastic#78867) Improve Node Shutdown Observability (elastic#78727) Convert encrypted snapshot license object to LicensedFeature (elastic#78731) Revert "Make nodePaths() singular (elastic#72514)" (elastic#78801) Fix incorrect generic type in PolicyStepsRegistry (elastic#78628) [DOCS] Fixes ML get calendars API (elastic#78808) Implement GET API for System Feature Upgrades (elastic#78642) [TEST] More MetadataStateFormat tests (elastic#78577) Add support for rest compatibility headers to the HLRC (elastic#78490) Un-ignoring tests after backporting fix (elastic#78830) Add node REPLACE shutdown implementation (elastic#76247) Wrap VersionPropertiesLoader in a BuildService to decouple build logic projects (elastic#78704) Adjust /_cat/templates not to request all metadata (elastic#78829) [DOCS] Fixes ML get scheduled events API (elastic#78809) Enable exit on out of memory error (elastic#71542) # Conflicts: # server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java
* upstream/master: (250 commits) [Transform] HLRC cleanups (elastic#78909) [ML] Make ML indices hidden when the node becomes master (elastic#77416) Introduce a Few Settings Singleton Instances (elastic#78897) Simplify TestCluster extraJar configuration (elastic#78837) Add @OverRide annotations to methods in EnrichPlugin class (elastic#76873) Add v7 restCompat for invalidating API key with the id field (elastic#78664) EQL: Refine repeatable queries (elastic#78895) Fix DataTierTests package and add a validation test (elastic#78880) Fix split package org.elasticsearch.common.xcontent (elastic#78831) Store DataTier Preference directly on IndexMetadata (elastic#78668) [DOCS] Fixes typo in calendar API example (elastic#78867) Improve Node Shutdown Observability (elastic#78727) Convert encrypted snapshot license object to LicensedFeature (elastic#78731) Revert "Make nodePaths() singular (elastic#72514)" (elastic#78801) Fix incorrect generic type in PolicyStepsRegistry (elastic#78628) [DOCS] Fixes ML get calendars API (elastic#78808) Implement GET API for System Feature Upgrades (elastic#78642) [TEST] More MetadataStateFormat tests (elastic#78577) Add support for rest compatibility headers to the HLRC (elastic#78490) Un-ignoring tests after backporting fix (elastic#78830) ... # Conflicts: # server/src/main/java/org/elasticsearch/ingest/IngestService.java # server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java
This adds support for the headers necessary for REST version compatibility to the High Level Rest
Client (HLRC).
Compatibility mode can be turned on either with the
.setAPICompatibilityMode(true)when creatingthe client, or by setting the
ELASTIC_CLIENT_APIVERSIONINGtotruesimilar to our otherElasticsearch clients.
Resolves #77859