elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.9k stars 24.73k forks source link

Support endpoints added in new version with compatible api #64852

Open pgomulka opened 3 years ago

pgomulka commented 3 years ago

A follow up from a discussion on https://github.com/elastic/elasticsearch/pull/64423#discussion_r517918932

When a user provides an accept header applicaiton/vnd.elasticsearch+json;compatible-with=7 (v7 in short) and is talking to ES v7 (7.0 - 7.x) He should expect no breaking changes in all ES 7 range. New apis might be added, but hence adding is not a breaking change this is ok. Client might be getting a 404 requesting a non existing resource on ES 7.0 but get a 200 when that endpoint was added in 7.1 The response header Content-Type should be always equal to the Accept header on a request (applicaiton/vnd.elasticsearch+json;compatible-with=7)

After upgrading to ES 8, when a client is still using v7 client, he will continue sending applicaiton/vnd.elasticsearch+json;compatible-with=7 Accept header on its requests. For endpoints that did not change - he will continue getting 200 and response’s content-type applicaiton/vnd.elasticsearch+json;compatible-with=7

Question 1: For endpoints newly adedd in ES v8 (non existing in v7 range) should he be getting 200 and `applicaiton/vnd.elasticsearch+json;compatible-with=7 as newly added is always backwards compatible ?

Question 2 What if the newly added API is experimental and it changes in a breaking way in v8.1?

relates https://github.com/elastic/elasticsearch/issues/51816

elasticmachine commented 3 years ago

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

jakelandis commented 3 years ago

There is a question thought what if the newly added API is experimental and it changes in a breaking way in v8.1?

I think we should always make it easy for users to use the most recent APIs. Requiring users to change the headers sent to start consuming any new API I think would run counter to that. This includes experimental and beta APIs. Experimental (and arguably beta) APIs are not constrained by semver by definition. The lack of semver applies outside of compatibility (i.e. an experimental can be breaking in a minor), so we should not try to hold ourselves to a higher bar for compatibility.

As a consumer, i would be surprised to even have access to an experimental 8.x API in a 7.x client. But if I did, I would want to be able to use it with out modifying headers in some non-obvious way. With an 8.x client, if it supported experimental features, I would also expect that the part of the client that interacts with the experimental feature keeps pace with the server side which could mean non-passive changes in a minor.

jakelandis commented 3 years ago

We discussed this in person today and this is essentially should we be stict or lenient for a very specific usage scenario.

To rephrase the original description...

Assume the following scenario:

A custom application that talks to Elasticsearch, but does not use one of the official clients. This application is talking to Elasticsearch v7 and getting ready to upgrade to Elasticsearch v8. They enable Rest API compatibility by configuring their HTTP requests to Elasticsearch to always send the header applicaiton/vnd.elasticsearch+json;compatible-with=7. This will help to decouple the upgrade from some of the work needed in their custom client to continue to use the same code before/during/after the upgrade to v8. After the upgrade to v8 is complete they continue to send the applicaiton/vnd.elasticsearch+json;compatible-with=7. They can see in the logs that compatibility is happening for a couple APIs so they need to continue to sending that header until they can fix their client. So some time passes and v8.2 is released and has this brand new feature (with a new REST API) they want to use, but they haven't gotten around to fixing the client so that compatibility with 7 is still needed for those couple APIs.

The crux of the question here is ... Should Elasticsearch allow that application to consume that brand new feature AND pass the compatibility header to it. A lenient approach says yes, a strict approach says no.

An argument for the strict approach.

If you are requesting compatibility with v7, and that thing never existed in v7 does it makes sense to allow a consumer to consume that thing that never existed ? If we are strict we could require you don't mix metaphors by asking for something brand new but compatible with something old. Semantically the strict approach is more correct.

Being lenient may encourage accumulation of technical debt in custom clients. Any instance where compatibility is actually used it should be considered technical debt to be paid down. By allowing consumption of that new feature AND the compatibility header we are allowing clients to mix new features while still holding that debt. It is also possible that client simply does not send the compatibility header for that endpoint, which would work and (if you like the strict approach) is a good thing since it makes the client aware that this is special case and compatibility here doesn't actually apply.

This is an issue only with custom clients. That new feature in 8.2 won't be retro-fitted to a the official v7 client (unless that feature is back ported) making this a moot point for all official clients. This may provide more reasons to use custom clients or custom/monkey patches of the official clients since with the official clients you need v8 of the client to use that new feature. To use v8 of client you will need to pay off any technical debt represented by the compatibility.

An argument for the lenient approach.

Rest API compatibility is developed to allow for more leniency between the client and server versions. A goal of the REST API compatibility project is to increase likely hood that you can defer making changes to the client when upgrading the server. New APIs that did not exist in 7.x is a not so obvious line to draw for what APIs you can send the header to or not.

Requiring a user to change the header for the new API is a barrier to adoption of that new feature. Cherry picking which API's the client needs to send the header to is more difficult then simply sending the same header to every end point. It is an easier mental model to just send the compatibility header to all endpoint regardless of when the API was introduced.

Technically, the strict approach would require some intelligence of when an API was introduced. Since this is only for the very specific use-case mentioned above, is this even worth the effort to make this strict ? (it will be lenient unless we add code for strictness)

In summary

While there is not a definitive consensus, lenient is the general direction we will likely go. I will remove the team discuss label, but leave this issue open for a bit for additional discussion if needed.

elasticsearchmachine commented 2 years ago

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