Upgrade search app to Elastic Search 5.4 #3373

ericholscher · 2017-12-07T05:54:37Z

This upgrades elastic search, and fixes indexing so that it works on 5.4.

Also integrates the new 5.4 search code in ext. I'm not 100% convinced this shouldn't live in the .org codebase, but just following existing convention of where the docsearch code was.

Test failures are unrelated to this change.

humitos

I don't have the knowledge to judge the ES part, but it seems OK otherwise.

humitos · 2017-12-12T01:32:12Z

readthedocs/restapi/views/footer_views.py

@@ -43,7 +43,7 @@ def get_version_compare_data(project, base_version=None):
    }
    if highest_version_obj:
        ret_val['url'] = highest_version_obj.get_absolute_url()
-        ret_val['slug'] = highest_version_obj.slug,
+        ret_val['slug'] = highest_version_obj.slug


Removing this comma produces tests to fail... I had the same problem and I ended up writing

(highest_version_obj.slug, )

I think the test should be fixed. Its expecting a tuple but it should get the slug in sting

@safwanrahman agreed -- want to PR it to master, and I'll merge it in here?

humitos · 2017-12-12T01:35:11Z

readthedocs/settings/base.py

@@ -116,6 +116,7 @@ def INSTALLED_APPS(self):  # noqa
        if donate:
            apps.append('django_countries')
            apps.append('readthedocsext.donate')
+            apps.append('readthedocsext.search')


Why is this under if donate:?

I fixed that here, actually: https://github.com/rtfd/readthedocs.org/pull/3356/files#diff-5973fe59a4d52278d55f884d145bd5d7R11 :) Waiting on it to get merged.

Merged #3356, this can be rebased

humitos · 2017-12-12T01:37:26Z

readthedocs/urls.py

-    groups.append([
-        url(r'^sustainability/', include('readthedocsext.donate.urls')),
-    ])
+    groups.insert(0,


Why this change?

Seems to be the same, anyway.

humitos · 2017-12-12T01:37:54Z

readthedocs/urls.py

+    groups.insert(0,
+                  [url(r'^sustainability/', include('readthedocsext.donate.urls'))]
+                  )
+    for num, _url in enumerate(rtd_urls):


Same question again. This is only if the donate app is installed, why? I don't find the relationship between donate app and search app.

safwanrahman · 2017-12-12T01:42:33Z

Is it possible to upgrade latest 6.x?

ericholscher · 2017-12-12T01:50:47Z

@safwanrahman that breaks a lot of what we do, specifically removing the ability to have multiple doctypes per index, so I want to keep things here until we have more time to do a full rewrite. This is an incremental step.

safwanrahman · 2017-12-12T01:59:35Z

readthedocs/search/indexes.py

-                    'lang': {'type': 'string', 'index': 'not_analyzed'},
-                    'tags': {'type': 'string', 'index': 'not_analyzed'},
-                    'privacy': {'type': 'string', 'index': 'not_analyzed'},
+                    'id': {'type': 'keyword'},


Is not id is integer or long?

I believe the new ES doesn't let these vary types, or something. I was getting an error, and it doesn't really need to be an ID, so we will just use it as a string like the others.

@ericholscher Seems like they support it
https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html

Without ID, what can be here?

It does seem strange that we're coercing to string here, but I'm not familiar enough with ES to say why token/string is better.

It's not that it's better, it just doesn't work otherwise. I don't fully understand it either.

It throws this error: RequestError: TransportError(400, u'illegal_argument_exception', u'mapper [id] cannot be changed from type [long] to [keyword]')

agjohnson

Looks good. I can't comment much on the ES implementation details, but it seems reasonable. The long -> string coercion does seem like something we want to avoid if possible, but not really sure of the answer there.

agjohnson · 2017-12-12T22:15:01Z

readthedocs/search/indexes.py

-                    'lang': {'type': 'string', 'index': 'not_analyzed'},
-                    'tags': {'type': 'string', 'index': 'not_analyzed'},
-                    'privacy': {'type': 'string', 'index': 'not_analyzed'},
+                    'id': {'type': 'keyword'},


It does seem strange that we're coercing to string here, but I'm not familiar enough with ES to say why token/string is better.

xrmx · 2018-03-07T16:00:08Z

I've rebased this on master here: https://github.com/italia/readthedocs.org/tree/upgrade-es-rebased

It fails for me against ES 5.6.8 with the following:

{u'index':
  {u'status': 400, u'_type': u'page', u'_index': u'readthedocs', u'error': {
    u'caused_by': {
      u'reason': u'For input string: "de3a71b6edf5001ece51f4ea16ca4758"', u'type': u'number_format_exception'
    },
    u'reason': u'failed to parse [id]', u'type': u'mapper_parsing_exception'
  },

xrmx · 2018-03-13T11:22:06Z

I've done some more work here: https://github.com/italia/readthedocs.org/tree/upgrade-es-rebased

Now we are indexing again, updated the search regarding filtering and faceting (now called aggs) to what ES 5 expects.

Still broken:

TransportError(400, u'search_phase_execution_exception', u'Fielddata is disabled on text fields by default. Set fielddata=true on [lang] in order to load fielddata in memory by uninverting the inverted index. Note that this can however use significant memory. Alternatively use a keyword field instead.')

ericholscher · 2018-06-06T20:08:27Z

Closing this, hoping to be replaced by #4183 which uses Elastic Search 6.

ericholscher added 3 commits December 6, 2017 21:54

Upgrade search app to Elastic Search 5.4

f291550

Update ES dep

a922986

Integrate ext search

b7e1189

RichardLitt added the PR: work in progress Pull request is not ready for full review label Dec 7, 2017

ericholscher added 5 commits December 7, 2017 12:40

Don't require ext search

848ee3c

Fix URL insertion

e5c42c0

Merge remote-tracking branch 'origin/master' into upgrade-es

4cf429f

Fix lint

1291961

Fix this here too, for now :)

a4208de

ericholscher removed the PR: work in progress Pull request is not ready for full review label Dec 11, 2017

ericholscher requested review from humitos and agjohnson December 11, 2017 21:43

humitos reviewed Dec 12, 2017

View reviewed changes

ericholscher added 4 commits December 11, 2017 17:43

Use proper URL insert

ec992c3

Merge remote-tracking branch 'origin/master' into upgrade-es

8200055

Use old append

d0a8ce2

Leave out unrelated change

0c538f8

safwanrahman reviewed Dec 12, 2017

View reviewed changes

agjohnson reviewed Dec 12, 2017

View reviewed changes

ericholscher added 2 commits December 12, 2017 14:33

Merge remote-tracking branch 'origin/master' into upgrade-es

75cf46f

Default sections off and fix unicode on index

f2d0758

agjohnson added the PR: work in progress Pull request is not ready for full review label Dec 14, 2017

ericholscher added the Status: blocked Issue is blocked on another issue label Dec 14, 2017

ericholscher added 5 commits December 14, 2017 14:25

Merge remote-tracking branch 'origin/master' into upgrade-es

ede0827

Run task via celery to do concurrency

87c1c9d

Lint reindex_elasticsearch

213a390

Merge remote-tracking branch 'origin/master' into upgrade-es

9dafb8b

Lint utils

07be69d

ericholscher added 3 commits December 19, 2017 11:58

fix up loggin

dedf97f

Make reindexing better

24f9776

Fix lint

ba8c675

humitos mentioned this pull request Dec 21, 2017

Upgrade all packages using pur tool #2916

Merged

humitos mentioned this pull request Dec 28, 2017

Support elastic search 2.x+ #2539

Closed

humitos mentioned this pull request Feb 28, 2018

search: index project tags #3698

Closed

xrmx mentioned this pull request Mar 13, 2018

Upgrade Elasticsearch to 5.X #3787

Closed

stsewd mentioned this pull request Mar 23, 2018

Elasticsearch not working in local installation #3803

Closed

ericholscher closed this Jun 6, 2018

stsewd deleted the upgrade-es branch September 6, 2018 20:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade search app to Elastic Search 5.4 #3373

Upgrade search app to Elastic Search 5.4 #3373

ericholscher commented Dec 7, 2017 •

edited

Loading

humitos left a comment

humitos Dec 12, 2017

safwanrahman Dec 12, 2017

ericholscher Dec 12, 2017

humitos Dec 12, 2017

ericholscher Dec 12, 2017

agjohnson Dec 12, 2017

humitos Dec 12, 2017

humitos Dec 12, 2017

safwanrahman commented Dec 12, 2017

ericholscher commented Dec 12, 2017 •

edited

Loading

safwanrahman Dec 12, 2017 •

edited

Loading

ericholscher Dec 12, 2017

safwanrahman Dec 12, 2017 •

edited

Loading

agjohnson Dec 12, 2017

ericholscher Dec 12, 2017

ericholscher Dec 12, 2017

agjohnson left a comment

agjohnson Dec 12, 2017

xrmx commented Mar 7, 2018

xrmx commented Mar 13, 2018 •

edited

Loading

ericholscher commented Jun 6, 2018 •

edited

Loading

Upgrade search app to Elastic Search 5.4 #3373

Upgrade search app to Elastic Search 5.4 #3373

Conversation

ericholscher commented Dec 7, 2017 • edited Loading

humitos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safwanrahman commented Dec 12, 2017

ericholscher commented Dec 12, 2017 • edited Loading

safwanrahman Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safwanrahman Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agjohnson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xrmx commented Mar 7, 2018

xrmx commented Mar 13, 2018 • edited Loading

ericholscher commented Jun 6, 2018 • edited Loading

ericholscher commented Dec 7, 2017 •

edited

Loading

ericholscher commented Dec 12, 2017 •

edited

Loading

safwanrahman Dec 12, 2017 •

edited

Loading

safwanrahman Dec 12, 2017 •

edited

Loading

xrmx commented Mar 13, 2018 •

edited

Loading

ericholscher commented Jun 6, 2018 •

edited

Loading