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

Issue 6094 link

Starred by 5 users

Issue metadata

Status: Released
Closed: Jun 4

Sign in to add a comment

Add support for Elasticsearch version 5.x

Project Member Reported by, Apr 27 2017

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.
Project Member

Comment 1 by, Apr 27 2017

Or we could use curl

and remove the dependancy on elasticsearch dep.

Comment 2 Deleted

Comment 3 Deleted

Project Member

Comment 5 by, Jul 28 2017

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

Comment 6 by, Mar 12 2018

Components: Elasticsearch

Comment 7 Deleted

Project Member

Comment 8 by, Mar 12 2018

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
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.
Project Member

Comment 10 by, Mar 13 2018

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.
> I see errors

Please include the errors in this issue report.

 Issue 7856  has been merged into this issue.
Project Member

Comment 13 by, Mar 14 2018

So, the upgrade path could be: upgrade Lucene, upgrade ES and adjust the tests. 
Seems we do have a dep on Lucene from ElasticQueryBuilder, which imports:

Project Member

Comment 15 by, Mar 14 2018

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.
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.

Project Member

Comment 17 by, Apr 12 2018

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.
Project Member

Comment 18 by, Apr 13 2018

'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).
Project Member

Comment 19 by, Apr 13 2018
<=> lib/jest for Elasticsearch: refactor dependencies.

Project Member

Comment 20 by, Apr 13 2018

> 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]
Project Member

Comment 21 by, Apr 13 2018

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);

Project Member

Comment 22 by, Apr 13 2018
<=> 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.
Project Member

Comment 23 by, Apr 16 2018
<=> lib/elasticsearch: restore jackson_dataformat_smile
(just for the comprehensive record, still).
Project Member

Comment 24 by, Apr 16 2018

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?
Project Member

Comment 25 by, Apr 25 2018

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

Comment 26 by, May 11 2018

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


Project Member

Comment 27 by, May 18 2018

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


Project Member

Comment 28 by, May 28 2018

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.
Summary: Add support for Elasticsearch version 5.x (was: Update elasticsearch to 5.x in gerrit)
Tracking support for ES 6.x in  issue 9112 
Project Member

Comment 32 by, May 31 2018

Status: ChangeUnderReview (was: Started)

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:

Project Member

Comment 34 by, Jun 1

Currently working on fixing that warning; will let you know -thx for noticing it.
Labels: FixedIn-2.14.9
Status: Submitted (was: ChangeUnderReview)
Project Member

Comment 37 by, Nov 20

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

Sign in to add a comment