[ZEPPELIN-1618] [WIP] Support ElasticSearch 5.0

[1ambda]

What is this PR for?

ElasticSearch was released in Oct 26 and It requires 5.x client driver but zeppelin currently use 2.x only.

• we need 2 different profiles for 2.x and 5.x (there are no 3.x, 4.x)
• 5.x client require different codebase. In this case we should keep 2 version of code at the same time like the spark code.
• 5.x client requires JAVA 8 at least

What type of PR is it?

Improvement

Todos

• - create profiles for 2.x and 5.x
• - setup JAVA 8 validation for 5.x
• - update default elasticsearch.version to 2.4.x (the most recent for 2.x)
• - update license
• - refactor
• - create the 5.x interpreter
• - loading dynamically
• - hide unused classes using maven profile
• - write tests for 5.x
• - update docs (build, usage, upgrade)

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1618

How should this be tested?

Some tests will be included.

• mvn clean test -pl 'elasticsearch' -Pelasticsearch-2
• mvn clean test -pl 'elasticsearch' -Pelasticsearch-5

Screenshots (if appropriate)

Questions:

• Does the licenses files need update? - YES
• Is there breaking changes for older versions? - NO
• Does this needs documentation? - YES

[1ambda]

Ref - https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking_50_java_api_changes.html

• isFound https://github.com/elastic/elasticsearch/pull/19645/files
• setExtraSource https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking_50_java_api_changes.html#_searchrequest
• NodeBuilder Remove NodeBuilder elastic/elasticsearch#15354

2 APIs make little difficult to finish this PR. They are incompatible at all :(
We have to use reflection to support both versions.

But 2.x is broadly used at this point, I think we have to support 2.x in zeppelin 0.7 at least

public boolean isFound() { ... } 
// should be
RestStatus.NOT_FOUND != response.status())

public WriteRequestBuilder setRefresh(true) { ... }
// should be
setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)

• even more. They are using log4j 2
  Introduce Log4j 2 elastic/elasticsearch#20235

Caused by: java.lang.ClassNotFoundException: org.apache.logging.log4j.Logger
    at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    ... 25 more

• Embeded elasticsearch API was changed.
  Undocumented side effects from fixing PreBuiltTransportClientsTests PR #19803 elastic/elasticsearch#19903
• setExtraSource
  SearchRequestBuilder.wrapperquery can't set stringified JSON query (5.0.0) elastic/elasticsearch#21452

[1ambda]

2+ Solutions.

1. Use just reflection

• Pros: We can have only 1 interpreter ElasticsearchInterpreter
• Cons: but too much reflection code :(

2. Create Elasticsearch5Interpreter for version 5.x

• Pros: We don't need reflection by selecting the proper interpreter based on the maven profile
• Cons: We have 2 interpreters :(
  Also, Code might be duplicated but we can extract them as an interface. For example, ElasticsearchInterpeterBase

Final Thought

• Refactor the whole ElasticsearchInterpreter to extract abstract class which can be shared by 2.x, 5.x
• Write 2.x, 5.x interpreters separately and load them at runtime (Class.forName)
• Hide unused classes usign maven profiles

That would work

[1ambda]

Blocked until 5.1 release due to too many undocumented breaking changes

[bzz]

Great work @1ambda ! Do you know if there are any ETA for Elasticsearch 5.1?

\cc @bbonnin who contributed ES2.x interpreter support for a review

[bbonnin]

I have just one comment: it's about some duplicate lines of codes. I think some lines that we can find in both ElasticSearchXConnectors could be put in the base class, in order to keep only the specific part ES v2 and V5 in the sub-classes. An example is the method executeDeleteQuery that is almost the same in both classes.
Otherwise, looks good to me.

[1ambda]

@bbonnin Thanks for the quick review :)

1. Duplicated code in executeDeleteQuery is due to incompatible API between 2.x and 5.x

// 2.x
public boolean isFound() { ... } 
// 5.x
RestStatus.NOT_FOUND != response.status())

But we can extract duplicated validation logic to the base class as you mentioned.

2. @bzz, @bbonnin Regarding to update docs,

Are there more files I should update? I am asking because

• new maven profiles are added elasticsearch-2 (activated by default), elasticsearch-5
• elasticsearch-5 profile requires java 8 +

3. The final hurdle is,

• Figure out how to replace setExtraSource (SearchRequestBuilder.wrapperquery can't set stringified JSON query (5.0.0) elastic/elasticsearch#21452)

Hi @1ambda, wrapper query should work for the query part of the search request. Can you post more info on how it doesn't work for you please? Unfortunately I don't think we have the same for aggregations and the rest of a search request.
In general, I'd recommend to use the java api builders to create search requests that are already in the binary format, so no parsing is needed. Otherwise, I would suggest to have a look at the newly released Java REST client that doesn't have builders yet but allows to provide strings.

We need REST API client

• Enhance test cases for agg, search. They only checks InterpreterResult.Success not the response currently

[stevenmccord]

@1ambda were you still working through this one? I was going to give it a test run if this is something that might be getting in. Noticed it hasn't been updated with master in a while, so just wanted to check to see if it might be worthwhile to give it a try. Thanks!

[1ambda]

@stevenmccord Hi :)

I am trying to fix other urgent issue. So it's ok to close this PR if you want to implement.
The reason I am keeping this issue is sharing info about ES 5.0

[jaspinderdineout]

Is it live? Does zeppelin support elasricsearch 5.0 ???

[zjffdu]

\cc @1ambda

[1ambda]

@jaspinderdineout Hi :)

Zeppelin doesn't support es 5.0 currently as i know.
I will close this PR. This issue can be easily handled after #1902.

[brbrown25]

@1ambda recently elastico deprecated http and requires https now, is there any plan to upgraded this connector?