New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 5 users

Issue metadata

Status: Released
Closed: Jun 2018

Sign in to add a comment

Issue 6094: Add support for Elasticsearch version 5.x

Reported by, Apr 27 2017 Project Member

Issue description

*****                                                       *****
*****                                                       *****
*****                                                       *****
*****                                                       *****

Affected Version:2.14+

What steps will reproduce the problem?
1. Using elasticsearch 5.x does not work with gerrit

What is the expected output?
For elasticsearch 5 to work

What do you see instead?
I see errors

Please provide any additional information below.
Elasticsearch should either be updated or we should be able to configure which version of the lib we can use.

Comment 1 by, Apr 27 2017

Project Member
Or we could use curl

and remove the dependancy on elasticsearch dep.

Comment 2 Deleted

Comment 3 Deleted

Comment 5 by, Jul 28 2017

Project Member
Status: Accepted (was: New)
Summary: Update elasticsearch to 5.x in gerrit (was: Update elasticsearch to 5.x in gerrit 2.14)

Comment 6 by, Mar 12 2018

Project Member
Components: Elasticsearch

Comment 7 Deleted

Comment 8 by, Mar 12 2018

Project Member
We should switch to an api that does not depend on the lucent version. 

Which would allow us to support elasticsearch 5, 6, 7 and so on.

Namely using an api that uses the url to fetch it like curl

Comment 9 by, Mar 13 2018

IIRC the API doesn't depend on the Lucene version, the only reason we are stuck to a specific Lucene is because the Elasticsearch server requires it and we use that in the tests.

There was an attempt to isolate the older Lucene dependency to the tests, but that didn't work in Eclipse because it just puts everything on the classpath.

Comment 10 by, Mar 13 2018

Project Member
Strange as in phabricator we support versions 2 and 5. I doin't think i understand how elastcsearch lucene version is causing us to be blocked.

Comment 11 by, Mar 14 2018

> I see errors

Please include the errors in this issue report.

Comment 12 by, Mar 14 2018

 Issue 7856  has been merged into this issue.

Comment 13 by, Mar 14 2018

Project Member
So, the upgrade path could be: upgrade Lucene, upgrade ES and adjust the tests.

Comment 14 by, Mar 14 2018

Seems we do have a dep on Lucene from ElasticQueryBuilder, which imports:

Comment 15 by, Mar 14 2018

Project Member
My understanding is, that ES depends on Lucene, so that we have
to use the same versions and we could only upgrade to the max.
Lucene version, that is supported by ES.

Comment 16 by, Mar 14 2018

Yes, but before I thought the only dependency on Lucene was due to using ES during the tests. 

That might have been the case in earlier versions and the runtime dependency in the querybuilder was added later.

Comment 17 by, Apr 12 2018

Project Member
Status: Started (was: Accepted)
Adding both Davids in cc, as not sure about Monorail's behaviour here; no annoyance intended then.

Started working on this overall, by means of this initial change first:

Not setting myself as Owner, at least not yet, given the initial nature of this work. Same goes for not using the ChangeUnderReview status -yet. But please do feel free to amend, at will.

Comment 18 by, Apr 13 2018

Project Member
'Elasticsearch BUILD: remove unneeded dependencies' <=>

-Only slightly related to this issue, but still.
That change also contributes, by means of removing extra dependencies, such as lucene-analyzers-common (which is of interest here).

Comment 19 by, Apr 13 2018

Project Member
<=> lib/jest for Elasticsearch: refactor dependencies.

Comment 20 by, Apr 13 2018

Project Member
> Seems we do have a dep on Lucene from ElasticQueryBuilder, which imports:

I don't see, why this catch block is needed. What happens if we remove it: [1]?

* [1]

Comment 21 by, Apr 13 2018

Project Member
Ah, I see this is a runtime exception:

  /** Thrown when an attempt is made to add more than {@link
   * #getMaxClauseCount()} clauses. This typically happens if
   * a PrefixQuery, FuzzyQuery, WildcardQuery, or TermRangeQuery 
   * is expanded to many terms during search. 
  public static class TooManyClauses extends RuntimeException {
    public TooManyClauses() {
      super("maxClauseCount is set to " + maxClauseCount);

Comment 22 by, Apr 13 2018

Project Member
<=> lib/elasticsearch: remove unnecessary dependencies.

Now I think the work can start on gradually bumping our hereby versions, also joining the other thread right above.

Comment 23 by, Apr 16 2018

Project Member
<=> lib/elasticsearch: restore jackson_dataformat_smile
(just for the comprehensive record, still).

Comment 24 by, Apr 16 2018

Project Member
So the tests are currently using the Elasticsearch (native, embedded) Java API -[1].

Production code too, although mostly relying on Jest [2] or REST API use -towards an Elasticsearch cluster.

Since then, Elastic have been recommending a newer REST API instead, coming in two paths:

We could then maybe aim at halting our usage of the native Java API [1,5], as well as Jest's ([2]).
All this in light of the [3,5]-listed shortcomings with (mostly) that legacy Java API.

As we potentially do so, we could go incremental with the chosen version(s), given our other Lucene deps in Gerrit.
We would then also start bumping Lucene versions, driven by (based on) such Elastic API changes but also Gerrit.
This would lead us into choosing specific Elasticsearch versions [6] for production too.

More thoughts on how to pursue this further though?

Comment 25 by, Apr 25 2018

Project Member
Resuming our work on this now for the coming weeks.

Comment 26 by, May 11 2018

Project Member
I just replied to [1] about our current status regarding the WIP for this issue, if anyone has been wondering about this.


Comment 27 by, May 18 2018

Project Member
The change [1] to remove the lucene dependency from the elasticsearch client in prod has just been pushed for review:


Comment 28 by, May 28 2018

Project Member
In case anyone interested in this issue would need to know: more changes were recently merged on top of the aforementioned [1], about also removing such dependencies for the test code.

Next steps are then to adapt production and test (Elasticsearch client) code to more/later Elasticsearch server versions; WIP.

Comment 29 by, May 30 2018

Summary: Add support for Elasticsearch version 5.x (was: Update elasticsearch to 5.x in gerrit)

Comment 31 by, May 30 2018

Tracking support for ES 6.x in  issue 9112 

Comment 32 by, May 31 2018

Project Member
Status: ChangeUnderReview (was: Started)

Comment 33 by, Jun 1 2018

Alternative solution uploaded:

With this all the tests pass, although there is still a deprecation warning:

  "Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]"

This seems to be the same as mentioned here:

Comment 34 by, Jun 1 2018

Project Member
Currently working on fixing that warning; will let you know -thx for noticing it.

Comment 36 by, Jun 4 2018

Labels: FixedIn-2.14.9
Status: Submitted (was: ChangeUnderReview)

Comment 37 by, Nov 20

Project Member
Labels: FixedIn-2.16
Status: Released (was: Submitted)

Sign in to add a comment