Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Breaking changes with opensearch.http.override_main_response_version=true #3023

Closed
dblock opened this issue Apr 21, 2022 · 23 comments · Fixed by #3031
Closed

[BUG] Breaking changes with opensearch.http.override_main_response_version=true #3023

dblock opened this issue Apr 21, 2022 · 23 comments · Fixed by #3031
Labels
backwards-compatibility bug Something isn't working v2.0.0 Version 2.0.0

Comments

@dblock
Copy link
Member

dblock commented Apr 21, 2022

There are a number of breaking changes in 2.0 RESTful APIs. I suspect (still need to confirm) that breaks opensearch.http.override_main_response_version=true (https://opensearch.org/docs/latest/clients/agents-and-ingestion-tools/index/).

I think we have two options.

a) Remove opensearch.http.override_main_response_version=true.
b) Ensure backwards compatibility.

This is tangentially related to opensearch-project/opensearch-clients#17.

@dblock dblock added bug Something isn't working v2.0.0 Version 2.0.0 untriaged labels Apr 21, 2022
@dblock
Copy link
Member Author

dblock commented Apr 21, 2022

@shwetathareja Could you help find a repro case where this is true (using override_main_response_version works in 1.x and is broken in 2.0)?

@dblock
Copy link
Member Author

dblock commented Apr 21, 2022

@mch2 @saratvemulapalli I believe we were originally planning to do (a) for 2.0, is that accurate? do we no longer want to?

@dblock dblock removed the untriaged label Apr 21, 2022
@peternied
Copy link
Member

While 2.0.0 introduces breaking changes to the rest interface, the clients abstraction the rest calls. Is that abstraction enough that a 1.x client is still logically comparable via the abstraction layer?

Assuming it is, we add support for OpenSearch 2.X in a new OpenSearch client 1.X release on inspection of the api version, it would be valuable to allow rolling backends forward while front ends only need the minor update.

@anasalkouz
Copy link
Member

We have similar case on Inclusive naming and the plan to make sure you can do rolling upgrade without any issues. I would avoid doing the upgrade on server and client on the same time.

@dlvenable
Copy link
Member

OpenSearch 2.0 removed the classic mapping templates. Clients may be expecting these are available if OpenSearch is advertising itself as a 7.10.2 cluster.

I pushed for the addition of this property (override_main_response_version) initially to help users migrate existing ingestion tools such as Logstash. Often these tools would use the version to decide how to create templates. Hopefully they were not using mapping types. But, if they are, then OpenSearch 2.0 would fail.

@saratvemulapalli
Copy link
Member

saratvemulapalli commented Apr 21, 2022

@mch2 @saratvemulapalli I believe we were originally planning to do (a) for 2.0, is that accurate? do we no longer want to?

This is how I understand it.
We no longer want to have this change in 2.0.
We originally put in the override because technically Elasticsearch 7.10 is the same as OpenSearch 1.0 (i.e we didnt change any of our Rest API signatures).
For clients which work with 7.10.2 did not work with OpenSearch because of new version checks added and this change helps customers use existing clients and make it work.

Now us being at 2.0, this override is not needed anymore. Because the clients on older major (i.e 1.x) cannot talk to OpenSearch 2.0 as we have many breaking changes in our Rest APIs.
Also note, this change only effects Rest API responses and not transport (i.e communication between nodes).

@nknize
Copy link
Collaborator

nknize commented Apr 21, 2022

Because the clients on older major (i.e 1.x) cannot talk to OpenSearch 2.0 as we have many breaking changes in our Rest APIs

Not entirely correct. Since OpenSearch Core 1.0.0 GA is nothing more than refactored, reversioned Legacy 7.10.x OpenSearch 2.0 is wire compatible back to legacy 7.10.0. We need to retain spoofing to ensure compatibility w/ 7.10.0 nodes. OpenSearch 3.0 will remove any/all compatibility w/ legacy versions so spoofing can be removed at that time.

@nknize
Copy link
Collaborator

nknize commented Apr 21, 2022

Hopefully they were not using mapping types. But, if they are, then OpenSearch 2.0 would fail.

Correct. Types were deprecated in legacy 7.x (Nov 1, 2018); ergo they were removed since clients had over three years to stop depending on types.

@shdubsh
Copy link
Contributor

shdubsh commented Apr 21, 2022

We use a couple supporting tools that still depend on the 7.x ES client libraries. We look forward to disabling the 7.10.2 spoofing as soon as possible to remove our reliance on this workaround.

I based our plans on the answer I received at the community meeting on 2021-10-19: "Do we expect the ES version compatibility flag to be removed in 2.0?" Kyle's answer was to expect the version compatibility flag to be re-evaluated prior to 3.0. If removed, it would be removed no earlier than 3.0.

If this is no longer possible and it becomes necessary for us to remain on 1.x until we no longer rely on the workaround, will 1.x continue to get security patches for while?

@saratvemulapalli
Copy link
Member

saratvemulapalli commented Apr 21, 2022

Not entirely correct. Since OpenSearch Core 1.0.0 GA is nothing more than refactored, reversioned Legacy 7.10.x OpenSearch 2.0 is wire compatible back to legacy 7.10.0. We need to retain spoofing to ensure compatibility w/ 7.10.0 nodes. OpenSearch 3.0 will remove any/all compatibility w/ legacy versions so spoofing can be removed at that time.

Interesting.. thanks for letting me know.
So there could be Rest clients which support 7.10 and we do want to support them for 2.0?

@reta
Copy link
Collaborator

reta commented Apr 21, 2022

@saratvemulapalli I think it is reversed: Rest clients which support 2.0 and we do want to support them for 7.10. That would be the upgrade path: upgrade client to 2.0, it should work for both versions, than start upgrading clusters.

@shwetathareja
Copy link
Member

shwetathareja commented Apr 21, 2022

Because the clients on older major (i.e 1.x) cannot talk to OpenSearch 2.0 as we have many breaking changes in our Rest APIs

Not entirely correct. Since OpenSearch Core 1.0.0 GA is nothing more than refactored, reversioned Legacy 7.10.x OpenSearch 2.0 is wire compatible back to legacy 7.10.0. We need to retain spoofing to ensure compatibility w/ 7.10.0 nodes. OpenSearch 3.0 will remove any/all compatibility w/ legacy versions so spoofing can be removed at that time.

Every major version is expected to be backward compatible with last minor version of majorVersion - 1. So OpenSearch 2.0 should be backward compatible with ES 7.10. But this flag - opensearch.http.override_main_response_version=true was introduced to ensure OpenSearch pretends to be ES 7.10 for the rest clients and I think OpenSearch 2.0 is not really ES 7.10 and it shouldn't do that. Like @dlvenable mentioned if they use 7.10 rest client against OpenSearch 2.0 cluster, things would break for them like legacy mapping templates. We should recommend customers to upgrade their client first like @reta said and then upgrade their clusters afterwards.

@nknize this flag opensearch.http.override_main_response_version=true doesn't impact the node connectivity with ES 7.10, that would still work and only impact the version in localhost:9200 main response.

@stockholmux
Copy link
Member

I think there is some dissonance between OpenSearch 2.0.0 saying it's 7.10.2 yet not having the same API (after the removal of mapping types).

It would be completely logical for someone to have a tool or script that tests for 7.10.2 and continues to use mapping types (maybe only complaining in the logs or something). Now if they move to OpenSearch 2.0.0 w/ override_main_response_version it will fail at runtime - that's a total bummer. The nightmare scenario is if it only failed in a corner case or something, so they discover the failure in an emergency.

I'm not sure the project should be shy about 2.0.0 having breaking changes - that's what major versions are for after all. If put myself in the mindset of a user, I'd rather the software prevent something nasty at the start than it cropping up in a running process.

@reta
Copy link
Collaborator

reta commented Apr 21, 2022

The main purpose of the override_main_response_version was to allow Opensearch to work with native Elasticsearch clients (so users could migrate mostly seamlessly). AFAIK, it does not work for Elasticsearch client 7.15.+, so the usefulness of this property became less valuable. Most projects which support Elasticsearch are already using more recent clients or switched to native Opensearch clients.

Keeping this flag for 2.0 would probably mean to return not 7.10 but 8.0, to match Elasticsearch breaking API changes, but none of the Elasticsearch native clients would support that. I suspect there is small number of projects that prefer to use raw HTTP and not rely on any Elasticsearch / Opensearch clients at all, like [1], some of those could check for version in response.

I think removing opensearch.http.override_main_response_version=true for 2.0 would be better than trying to mimic Elasticsearch responses.

[1] akka/alpakka#2850

@nknize
Copy link
Collaborator

nknize commented Apr 21, 2022

That would be the upgrade path: upgrade client to 2.0, it should work for both versions, than start upgrading clusters.

That is the correct upgrade path for clients depending on the HLRC.

opensearch.http.override_main_response_version=true doesn't impact the node connectivity with ES 7.10

Correct. opensearch.http.override_main_response_version=true does not impact internal node connectivity it was only added to ensure compatibility w/ legacy 7.x clients. I'm sorry if that wasn't clear. My point is that 2.0.0 supports Transport API compatibility back to 7.10.0. The HLRC (client facing API) does not include Version.onOrAfter logic so it doesn't subscribe to the same compatibility guarantees; it follows the same deprecation / removal path as features which is why the types endpoints were removed (after being deprecated in 7.x/1.x).

I'm not sure the project should be shy about 2.0.0 having breaking changes

💯

there is some dissonance between OpenSearch 2.0.0 saying it's 7.10.2 yet not having the same API

Agree. I've slept since we removed the transport client December of last year so we don't need to retain transport client compatibility (nor should the REST client spoofing matter here).

+1 for removing the override_main_response_version property and forcing client upgrades.

@dblock
Copy link
Member Author

dblock commented Apr 21, 2022

It looks like we have some consensus on removing override_main_response_version for 2.0, so we should do it in RC1. Can anyone pick this up?

#3031

@nknize
Copy link
Collaborator

nknize commented Apr 21, 2022

Can anyone pick this up?

#3031

@saratvemulapalli
Copy link
Member

I've opened up an issue to remove the documentation for this feature from 2.0 and up.
Ref: opensearch-project/documentation-website#517

@rursprung
Copy link
Contributor

That is the correct upgrade path for clients depending on the HLRC.

but is the upgrade path from ES 7.10.2 HLRC and/or OpenSearch 1.x HLRC to OpenSearch 2.x HLRC or to opensearch-java? elastic said their elasticsearch-java is now the official java client and should be used instead and e.g. spring-data-elasticsearch is now refactoring to support both clients (and eventually also an OpenSearch client): spring-projects/spring-data-elasticsearch#1973 & spring-projects/spring-data-elasticsearch#1853

upgrading from one HLRC to another HLRC is probably easiest (for those projects using it directly) but moving directly to opensearch-java is probably the more long-term approach. and since opensearch-java is not part of OpenSearch itself i would presume it would lend itself much more to being compatible over multiple OpenSearch major versions than the HLRC which is an integral part of OpenSearch and makes use of internal OpenSearch libraries?

@nknize
Copy link
Collaborator

nknize commented Jun 7, 2022

Many users of Beats and Logstash have switched to OpenSearch and Elastic has made it clear that these products do not intend to support OpenSearch. There are explicit version checks to verify compatibility with Elasticsearch that were circumvented using the spoofing mechanism. Without the spoof users of some filebeat processors are now DOA after upgrading to 2.0.

I have opened a PR to revert the removal of spoofing so these affected users can continue to function by explicitly setting the version. Yes, breaking compatibility with upgraded clients might be a side effect if users explicitly set this setting to function with older versions of Beats and Logstash. For this reason we need the REST API Versioning mechanism so this will, hopefully, be a temporary situation. Longer term we need a plan to either fork beats and logstash or replace it with an alternative like FluentBit.

@minutolc
Copy link

Hello everyone

I'm currently using org.elasticsearch : elasticsearch-hadoop and elasticsearch-hadoop-mr since there is no opensearch-hadoop library for JAVA
In Opensearch 1.X.X I use this option override_main_response_version in order to solve this error : No type found; Types are required when writing in ES version 6 and bellow.

Now with Opensearch 2.0.0 i cannot use anymore this option, so i 'm currently blocked .
I cannot use the hadoop connector for elasticsearch, and there is no clear roadmap on the hadoop connector for opensearch

Anyone has an idea on a workaround ?

@mattweber
Copy link
Contributor

@minutolc,

I was in same boat and found that If all you need is support for indexing, it is a single line change:
https://github.com/elastic/elasticsearch-hadoop/blob/main/mr/src/main/java/org/elasticsearch/hadoop/rest/Resource.java#L121

Just change that line to:

typed = !UNDERSCORE_DOC.equals(type);

Then just use _doc as the type as things should start working. I am using this with spark and its working just fine.

Hope this helps.

@rursprung
Copy link
Contributor

Now with Opensearch 2.0.0 i cannot use anymore this option, so i 'm currently blocked .

OpenSearch 2.0.1 has been released a few days (weeks?) ago which has the option back in, see the release notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-compatibility bug Something isn't working v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.