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

HLClient 6.7 does not work with prior 6.x versions #41647

Closed
dadoonet opened this issue Apr 29, 2019 · 10 comments
Closed

HLClient 6.7 does not work with prior 6.x versions #41647

dadoonet opened this issue Apr 29, 2019 · 10 comments
Labels

Comments

@dadoonet
Copy link
Member

The HLClient 6.7 now passes a parameter include_type_name when you for example create an index with (https://github.com/dadoonet/fscrawler/blob/master/elasticsearch-client/elasticsearch-client-v6/src/main/java/fr/pilato/elasticsearch/crawler/fs/client/v6/ElasticsearchClientV6.java#L232-L249):

CreateIndexRequest cir = new CreateIndexRequest(index);
cir.source(indexSettings, XContentType.JSON);
client.indices().create(cir, RequestOptions.DEFAULT);

This is ok when you are running this against a 6.7 cluster but this new parameter is not supported by older elasticsearch versions.

12:46:22,478 DEBUG [f.p.e.c.f.c.FsCrawlerCli] error caught
org.elasticsearch.ElasticsearchStatusException: Elasticsearch exception [type=illegal_argument_exception, reason=request [/hello] contains unrecognized parameter: [include_type_name]]
    at org.elasticsearch.rest.BytesRestResponse.errorFromXContent(BytesRestResponse.java:177) ~[elasticsearch-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.RestHighLevelClient.parseEntity(RestHighLevelClient.java:2053) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.RestHighLevelClient.parseResponseException(RestHighLevelClient.java:2030) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.RestHighLevelClient.internalPerformRequest(RestHighLevelClient.java:1777) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.RestHighLevelClient.performRequest(RestHighLevelClient.java:1734) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.RestHighLevelClient.performRequestAndParseEntity(RestHighLevelClient.java:1696) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at org.elasticsearch.client.IndicesClient.create(IndicesClient.java:191) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
    at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndex(ElasticsearchClientV6.java:240) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
    at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndex(ElasticsearchClientV6.java:603) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
    at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndices(ElasticsearchClientV6.java:436) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
    at fr.pilato.elasticsearch.crawler.fs.FsCrawlerImpl.start(FsCrawlerImpl.java:161) ~[fscrawler-core-2.7-SNAPSHOT.jar:?]
    at fr.pilato.elasticsearch.crawler.fs.cli.FsCrawlerCli.main(FsCrawlerCli.java:270) [fscrawler-cli-2.7-SNAPSHOT.jar:?]
    Suppressed: org.elasticsearch.client.ResponseException: method [PUT], host [http://10.0.2.2:9200], URI [/hello?master_timeout=30s&include_type_name=true&timeout=30s], status line [HTTP/1.1 400 Bad Request]
{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"request [/hello] contains unrecognized parameter: [include_type_name]"}],"type":"illegal_argument_exception","reason":"request [/hello] contains unrecognized parameter: [include_type_name]"},"status":400}
        at org.elasticsearch.client.RestClient$SyncResponseListener.get(RestClient.java:936) ~[elasticsearch-rest-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.RestClient.performRequest(RestClient.java:233) ~[elasticsearch-rest-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.RestHighLevelClient.internalPerformRequest(RestHighLevelClient.java:1764) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.RestHighLevelClient.performRequest(RestHighLevelClient.java:1734) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.RestHighLevelClient.performRequestAndParseEntity(RestHighLevelClient.java:1696) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.IndicesClient.create(IndicesClient.java:191) ~[elasticsearch-rest-high-level-client-6.7.1.jar:6.7.1]
        at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndex(ElasticsearchClientV6.java:240) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
        at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndex(ElasticsearchClientV6.java:603) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
        at fr.pilato.elasticsearch.crawler.fs.client.v6.ElasticsearchClientV6.createIndices(ElasticsearchClientV6.java:436) ~[fscrawler-elasticsearch-client-v6-2.7-SNAPSHOT.jar:?]
        at fr.pilato.elasticsearch.crawler.fs.FsCrawlerImpl.start(FsCrawlerImpl.java:161) ~[fscrawler-core-2.7-SNAPSHOT.jar:?]
        at fr.pilato.elasticsearch.crawler.fs.cli.FsCrawlerCli.main(FsCrawlerCli.java:270) [fscrawler-cli-2.7-SNAPSHOT.jar:?]
    Caused by: org.elasticsearch.client.ResponseException: method [PUT], host [http://10.0.2.2:9200], URI [/hello?master_timeout=30s&include_type_name=true&timeout=30s], status line [HTTP/1.1 400 Bad Request]
{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"request [/hello] contains unrecognized parameter: [include_type_name]"}],"type":"illegal_argument_exception","reason":"request [/hello] contains unrecognized parameter: [include_type_name]"},"status":400}
        at org.elasticsearch.client.RestClient$1.completed(RestClient.java:552) ~[elasticsearch-rest-client-6.7.1.jar:6.7.1]
        at org.elasticsearch.client.RestClient$1.completed(RestClient.java:537) ~[elasticsearch-rest-client-6.7.1.jar:6.7.1]
        at org.apache.http.concurrent.BasicFuture.completed(BasicFuture.java:119) ~[httpcore-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.client.DefaultClientExchangeHandlerImpl.responseCompleted(DefaultClientExchangeHandlerImpl.java:177) ~[httpasyncclient-4.1.2.jar:4.1.2]
        at org.apache.http.nio.protocol.HttpAsyncRequestExecutor.processResponse(HttpAsyncRequestExecutor.java:436) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.nio.protocol.HttpAsyncRequestExecutor.inputReady(HttpAsyncRequestExecutor.java:326) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.DefaultNHttpClientConnection.consumeInput(DefaultNHttpClientConnection.java:265) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.client.InternalIODispatch.onInputReady(InternalIODispatch.java:81) ~[httpasyncclient-4.1.2.jar:4.1.2]
        at org.apache.http.impl.nio.client.InternalIODispatch.onInputReady(InternalIODispatch.java:39) ~[httpasyncclient-4.1.2.jar:4.1.2]
        at org.apache.http.impl.nio.reactor.AbstractIODispatch.inputReady(AbstractIODispatch.java:114) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.BaseIOReactor.readable(BaseIOReactor.java:162) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvent(AbstractIOReactor.java:337) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvents(AbstractIOReactor.java:315) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.AbstractIOReactor.execute(AbstractIOReactor.java:276) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.BaseIOReactor.execute(BaseIOReactor.java:104) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor$Worker.run(AbstractMultiworkerIOReactor.java:588) ~[httpcore-nio-4.4.5.jar:4.4.5]
        at java.lang.Thread.run(Thread.java:748) ~[?:1.8.0_201]

I think that this parameter should be omitted if the elasticsearch server we are running is before 6.7.0 (not sure we are detecting that though).

I thought that a Client 6.x should be compatible to any 6.y server version.
Here the story is that users can't do a smooth upgrade of their application:

  1. package the 6.7 client with their project. Restart their application. Always running a 6.x cluster.
  2. do a rolling upgrade from elasticsearch 6.x to 6.7
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@javanna
Copy link
Member

javanna commented May 1, 2019

Agreed with your analysis, the high-level REST client should not use new parameters added to existing API in a minor version as that breaks backwards compatibility. The underlying low-level client is not aware of the version of the nodes. Maybe a version based node selector could be used when newly added parameters are used, yet the 6.7 client could potentially be used against a cluster with no 6.7 nodes, in which case such calls won't work. I wonder if the parameter can be removed from these calls.

@dadoonet
Copy link
Member Author

dadoonet commented May 1, 2019

I think we should default this parameter to null (using a Boolean instead of a boolean).
That way, unless it's explicitly set, it would not be sent over the wire.

My 2 cents.

@jimczi
Copy link
Contributor

jimczi commented May 6, 2019

The rest client is supposed to be forward compatible, we have a note in the documentation regarding this behavior so I would not call this a bug: https://www.elastic.co/guide/en/elasticsearch/client/java-rest/current/java-rest-high-compatibility.html#java-rest-high-compatibility
There are a lot of scenarios that can break the compatibility of the client when talking to an older node and we don't have rest versioning yet so we could turn the note into a warning in the doc ?

dadoonet added a commit to dadoonet/fscrawler that referenced this issue May 6, 2019
Due to a change in 6.7 HLRest Client, FSCrawler ES6X distribution does not support anymore a version before 6.7.
This probably won't be fixed as per discussion in elastic/elasticsearch#41647 so we need to document this breaking change.

Closes #713
@dadoonet
Copy link
Member Author

dadoonet commented May 6, 2019

Thanks @jimczi. So the right way to upgrade an application in production would be:

  • Do the elasticsearch server rolling upgrade to 6.7.x
  • Then upgrade the application to the 6.7 client

You're right that this is documented. But it looks counter intuitive to me.

The problem I have is that I'm building a 3rd party tool which is used by end users. I can't know in advance which elasticsearch server the end user will be using but I'm always trying to use the latest Elasticsearch REST Client version to benefit from all recent changes/fixes.

When I switched to 6.7 client, that broke the compatibility for any user using an older version of elasticsearch.
As we are not using any rest versioning, I'm already packaging my project with 3 different artifacts:

  • ES 5
  • ES 6
  • ES 7

I don't wish having to package all elasticsearch minor versions TBH.

So my options are for now:

  • build one version per minor release.
  • document that my newest "ES 6" version is only compatible with 6.7+ clusters.
  • implement again (as I was doing previously) my own HLRestClient based on the LLClient.

I chose to document this breaking change. Unless we decide to provide a more flexible client.

Anyway as this is documented, I guess I can just close this issue.

@dadoonet dadoonet closed this as completed May 6, 2019
@javanna
Copy link
Member

javanna commented May 6, 2019

It is true that the most important compatibility aspect for the high level REST client is forward compatibility and that, as we document, it should be the last component to be upgraded. It is expected that e.g. the 6.7 client does not work fine against previous 6.x versions for what concerns APIs that have only been added in 6.7. I think it becomes more annoying if the client starts using in a minor version a new parameter that only 6.7 understands, especially for fundamental APIs like create index. I wonder how the other language clients handled this situation. Also, a different problem to keep in mind for the future: if we have bugfixes in the 6.7 client, we are forcing users to upgrade their cluster to 6.7 before they can upgrade their clients, which is odd. cc @hub-cap

@dadoonet
Copy link
Member Author

dadoonet commented May 6, 2019

I wonder how the other language clients handled this situation.

I believe that optional parameters are not passed to the REST interface. Here include_type_name should be set only if the user explicitly set it IMO.

@linker-c
Copy link

linker-c commented Aug 8, 2019

How do I upgrade from 6.1.1. to 7.3.0 then?
Just a reminder that 6.1.1 has the bug where "_doc" cannot be used as type; therefore, I am using "doc" which needs to be replaced to "_doc".
The rolling upgrade is 5.6 -> 6.8. And 6.8 -> 7.3.
So I have to upgrade in the following procedure:

  1. Upgrade my ES clusters to 6.8.2 first while using older REST.
  2. Wait for code to replace "doc" to "_doc" with new index (monthly index).
  3. Upgrade HLClient to 6.8.2
  4. Then upgrade ES cluster from 6.8.2 to 7.3.0
  5. Upgrade HLClient to 7.3.0

Is this the correct procedure?

@Tasselmi
Copy link

The rest client is supposed to be forward compatible, we have a note in the documentation regarding this behavior so I would not call this a bug: https://www.elastic.co/guide/en/elasticsearch/client/java-rest/current/java-rest-high-compatibility.html#java-rest-high-compatibility
There are a lot of scenarios that can break the compatibility of the client when talking to an older node and we don't have rest versioning yet so we could turn the note into a warning in the doc ?

what sucks is your forward compatibility but not backward compatibility, what do you think ?
when I use ES java client API, there are so many compatibility problems by users.
It is never easy to upgrade the server, but it is easy to upgrade the client.
forward compatibility is totally nonsense and foolish.

@jasontedor
Copy link
Member

@Tasselmi Please use more constructive language. You can communicate without being dismissive and insulting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants