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

Upgrade search app to Elastic Search 5.4 #3373

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions readthedocs/core/management/commands/reindex_elasticsearch.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
"""Reindex Elastic Search indexes"""
# -*- coding: utf-8 -*-
"""Reindex Elastic Search indexes."""

from __future__ import (
absolute_import, division, print_function, unicode_literals)

from __future__ import absolute_import
import logging
import socket
from optparse import make_option

from django.core.management.base import BaseCommand
from django.core.management.base import CommandError
from django.conf import settings
from django.core.management.base import BaseCommand, CommandError

from readthedocs.builds.constants import LATEST
from readthedocs.builds.models import Version
Expand All @@ -23,34 +25,42 @@ class Command(BaseCommand):
dest='project',
default='',
help='Project to index'),
make_option('-l',
dest='only_latest',
default=False,
action='store_true',
help='Only index latest'),
)

def handle(self, *args, **options):
"""Build/index all versions or a single project's version"""
"""Build/index all versions or a single project's version."""
project = options['project']
only_latest = options['only_latest']

queryset = Version.objects.all()
queryset = Version.objects.filter(active=True)

if project:
queryset = queryset.filter(project__slug=project)
if not queryset.exists():
raise CommandError(
'No project with slug: {slug}'.format(slug=project))
log.info("Building all versions for %s", project)
elif getattr(settings, 'INDEX_ONLY_LATEST', True):
u'No project with slug: {slug}'.format(slug=project))
log.info(u'Building all versions for %s', project)
if only_latest:
log.warning('Indexing only latest')
queryset = queryset.filter(slug=LATEST)

for version in queryset:
log.info("Reindexing %s", version)
try:
commit = version.project.vcs_repo(version.slug).commit
except: # pylint: disable=bare-except
# An exception can be thrown here in production, but it's not
# documented what the exception here is
commit = None

for version_pk, version_slug, project_slug in queryset.values_list(
'pk', 'slug', 'project__slug'):
log.info(u'Reindexing %s:%s' % (project_slug, version_slug))
try:
update_search(version.pk, commit,
delete_non_commit_files=False)
update_search.apply_async(
kwargs=dict(
version_pk=version_pk,
commit='reindex',
delete_non_commit_files=False
),
priority=0,
queue=socket.gethostname()
)
except Exception:
log.exception('Reindex failed for {}'.format(version))
log.exception(u'Reindexing failed for %s:%s' % (project_slug, version_slug))
50 changes: 50 additions & 0 deletions readthedocs/projects/migrations/0022_add-view-data.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.9.12 on 2017-12-11 13:05
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('projects', '0021_add-webhook-deprecation-feature'),
]

operations = [
migrations.AlterField(
model_name='domain',
name='canonical',
field=models.BooleanField(default=False, help_text='This Domain is the primary one where the documentation is served from'),
),
migrations.AlterField(
model_name='domain',
name='count',
field=models.IntegerField(default=0, help_text='Number of times this domain has been hit'),
),
migrations.AlterField(
model_name='project',
name='allow_promos',
field=models.BooleanField(default=True, help_text='If unchecked, users will still see community ads.', verbose_name='Allow paid advertising'),
),
migrations.AlterField(
model_name='project',
name='comment_moderation',
field=models.BooleanField(default=False, verbose_name='Comment Moderation'),
),
migrations.AlterField(
model_name='project',
name='conf_py_file',
field=models.CharField(blank=True, default=b'', help_text='Path from project root to <code>conf.py</code> file (ex. <code>docs/conf.py</code>). Leave blank if you want us to find it for you.', max_length=255, verbose_name='Python configuration file'),
),
migrations.AlterField(
model_name='project',
name='has_valid_webhook',
field=models.BooleanField(default=False, help_text='This project has been built with a webhook'),
),
migrations.AlterField(
model_name='project',
name='programming_language',
field=models.CharField(blank=True, choices=[('words', 'Only Words'), ('py', 'Python'), ('js', 'JavaScript'), ('php', 'PHP'), ('ruby', 'Ruby'), ('perl', 'Perl'), ('java', 'Java'), ('go', 'Go'), ('julia', 'Julia'), ('c', 'C'), ('csharp', 'C#'), ('cpp', 'C++'), ('objc', 'Objective-C'), ('other', 'Other')], default=b'words', help_text='The primary programming language the project is written in.', max_length=20, verbose_name='Programming Language'),
),
]
2 changes: 1 addition & 1 deletion readthedocs/restapi/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
url(r'index_search/',
search_views.index_search,
name='index_search'),
url(r'search/$', views.search_views.search, name='api_search'),
url(r'^search/$', views.search_views.search, name='api_search'),
url(r'search/project/$',
search_views.project_search,
name='api_project_search'),
Expand Down
40 changes: 21 additions & 19 deletions readthedocs/restapi/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# -*- coding: utf-8 -*-
"""Utility functions that are used by both views and celery tasks."""

from __future__ import absolute_import
from __future__ import (
absolute_import, division, print_function, unicode_literals)

import hashlib
import logging

Expand Down Expand Up @@ -37,7 +40,7 @@ def sync_versions(project, versions, type): # pylint: disable=redefined-builtin
type=type,
machine=False,
)
log.info("(Sync Versions) Updated Version: [%s=%s] ",
log.info('(Sync Versions) Updated Version: [%s=%s] ',
version['verbose_name'], version['identifier'])
else:
# New Version
Expand All @@ -49,7 +52,7 @@ def sync_versions(project, versions, type): # pylint: disable=redefined-builtin
)
added.add(created_version.slug)
if added:
log.info("(Sync Versions) Added Versions: [%s] ", ' '.join(added))
log.info('(Sync Versions) Added Versions: [%s] ', ' '.join(added))
return added


Expand All @@ -70,14 +73,14 @@ def delete_versions(project, version_data):

if to_delete_qs.count():
ret_val = {obj.slug for obj in to_delete_qs}
log.info("(Sync Versions) Deleted Versions: [%s]", ' '.join(ret_val))
log.info('(Sync Versions) Deleted Versions: [%s]', ' '.join(ret_val))
to_delete_qs.delete()
return ret_val
return set()


def index_search_request(version, page_list, commit, project_scale, page_scale,
section=True, delete=True):
section=False, delete=True):
"""
Update search indexes with build output JSON.

Expand All @@ -89,7 +92,7 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
project = version.project

log_msg = ' '.join([page['path'] for page in page_list])
log.info("Updating search index: project=%s pages=[%s]",
log.info('Updating search index: project=%s pages=[%s]',
project.slug, log_msg)

project_obj = ProjectIndex()
Expand All @@ -112,7 +115,7 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
routes = [project.slug]
routes.extend([p.parent.slug for p in project.superprojects.all()])
for page in page_list:
log.debug("Indexing page: %s:%s", project.slug, page['path'])
log.debug('Indexing page: %s:%s', project.slug, page['path'])
to_hash = '-'.join([project.slug, version.slug, page['path']])
page_id = hashlib.md5(to_hash.encode('utf-8')).hexdigest()
index_list.append({
Expand Down Expand Up @@ -142,25 +145,24 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
'weight': page_scale,
})
for route in routes:
section_obj.bulk_index(section_index_list, parent=page_id,
routing=route)
section_obj.bulk_index(section_index_list, routing=route)

for route in routes:
page_obj.bulk_index(index_list, parent=project.slug, routing=route)
page_obj.bulk_index(index_list, routing=route)

if delete:
log.info("Deleting files not in commit: %s", commit)
log.info('Deleting files not in commit: %s', commit)
# TODO: AK Make sure this works
delete_query = {
"query": {
"bool": {
"must": [
{"term": {"project": project.slug, }},
{"term": {"version": version.slug, }},
'query': {
'bool': {
'must': [
{'term': {'project': project.slug, }},
{'term': {'version': version.slug, }},
],
"must_not": {
"term": {
"commit": commit
'must_not': {
'term': {
'commit': commit
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/restapi/views/search_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def index_search(request):

utils.index_search_request(
version=version, page_list=data['page_list'], commit=commit,
project_scale=project_scale, page_scale=page_scale)
project_scale=project_scale, page_scale=page_scale, section=False)

return Response({'indexed': True})

Expand Down
75 changes: 36 additions & 39 deletions readthedocs/search/indexes.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import datetime

from elasticsearch import Elasticsearch, exceptions
from elasticsearch.helpers import bulk_index
from elasticsearch.helpers import bulk

from django.conf import settings

Expand Down Expand Up @@ -48,8 +48,6 @@ def get_settings(self, settings_override=None):
'number_of_replicas': settings.ES_DEFAULT_NUM_REPLICAS,
'number_of_shards': settings.ES_DEFAULT_NUM_SHARDS,
'refresh_interval': '5s',
'store.compress.tv': True,
'store.compress.stored': True,
'analysis': self.get_analysis(),
}
if settings_override:
Expand All @@ -76,7 +74,7 @@ def get_analysis(self):
analyzers['default_icu'] = {
'type': 'custom',
'tokenizer': 'icu_tokenizer',
'filter': ['word_delimiter', 'icu_folding', 'icu_normalizer'],
'filter': ['custom_word_delimiter', 'icu_folding', 'icu_normalizer', 'lowercase'],
}

# Customize the word_delimiter filter to set various options.
Expand Down Expand Up @@ -139,7 +137,7 @@ def bulk_index(self, data, index=None, chunk_size=500, parent=None,
docs.append(doc)

# TODO: This doesn't work with the new ES setup.
bulk_index(self.es, docs, chunk_size=chunk_size)
bulk(self.es, docs, chunk_size=chunk_size)

def index_document(self, data, index=None, parent=None, routing=None):
doc = self.extract_document(data)
Expand Down Expand Up @@ -220,25 +218,24 @@ def get_mapping(self):
# Disable _all field to reduce index size.
'_all': {'enabled': False},
'properties': {
'id': {'type': 'long'},
'name': {'type': 'string', 'analyzer': 'default_icu'},
'description': {'type': 'string', 'analyzer': 'default_icu'},

'slug': {'type': 'string', 'index': 'not_analyzed'},
'lang': {'type': 'string', 'index': 'not_analyzed'},
'tags': {'type': 'string', 'index': 'not_analyzed'},
'privacy': {'type': 'string', 'index': 'not_analyzed'},
'id': {'type': 'keyword'},
Copy link
Member

@safwanrahman safwanrahman Dec 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is not id is integer or long?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@safwanrahman safwanrahman Dec 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Without ID, what can be here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

'name': {'type': 'text', 'analyzer': 'default_icu'},
'description': {'type': 'text', 'analyzer': 'default_icu'},

'slug': {'type': 'keyword'},
'lang': {'type': 'keyword'},
'tags': {'type': 'keyword'},
'privacy': {'type': 'keyword'},
'author': {
'type': 'string',
'type': 'text',
'analyzer': 'default_icu',
'fields': {
'raw': {
'type': 'string',
'index': 'not_analyzed',
'type': 'keyword',
},
},
},
'url': {'type': 'string', 'index': 'not_analyzed'},
'url': {'type': 'keyword'},
# Add a weight field to enhance relevancy scoring.
'weight': {'type': 'float'},
}
Expand Down Expand Up @@ -273,19 +270,19 @@ def get_mapping(self):
# Disable _all field to reduce index size.
'_all': {'enabled': False},
# Associate a page with a project.
'_parent': {'type': self._parent},
# '_parent': {'type': self._parent},
'properties': {
'id': {'type': 'string', 'index': 'not_analyzed'},
'sha': {'type': 'string', 'index': 'not_analyzed'},
'project': {'type': 'string', 'index': 'not_analyzed'},
'version': {'type': 'string', 'index': 'not_analyzed'},
'path': {'type': 'string', 'index': 'not_analyzed'},
'taxonomy': {'type': 'string', 'index': 'not_analyzed'},
'commit': {'type': 'string', 'index': 'not_analyzed'},

'title': {'type': 'string', 'analyzer': 'default_icu'},
'headers': {'type': 'string', 'analyzer': 'default_icu'},
'content': {'type': 'string', 'analyzer': 'default_icu'},
'id': {'type': 'keyword'},
'sha': {'type': 'keyword'},
'project': {'type': 'keyword'},
'version': {'type': 'keyword'},
'path': {'type': 'keyword'},
'taxonomy': {'type': 'keyword'},
'commit': {'type': 'keyword'},

'title': {'type': 'text', 'analyzer': 'default_icu'},
'headers': {'type': 'text', 'analyzer': 'default_icu'},
'content': {'type': 'text', 'analyzer': 'default_icu'},
# Add a weight field to enhance relevancy scoring.
'weight': {'type': 'float'},
}
Expand Down Expand Up @@ -321,7 +318,7 @@ def get_mapping(self):
# Disable _all field to reduce index size.
'_all': {'enabled': False},
# Associate a section with a page.
'_parent': {'type': self._parent},
# '_parent': {'type': self._parent},
# Commenting this out until we need it.
# 'suggest': {
# "type": "completion",
Expand All @@ -330,18 +327,18 @@ def get_mapping(self):
# "payloads": True,
# },
'properties': {
'id': {'type': 'string', 'index': 'not_analyzed'},
'project': {'type': 'string', 'index': 'not_analyzed'},
'version': {'type': 'string', 'index': 'not_analyzed'},
'path': {'type': 'string', 'index': 'not_analyzed'},
'page_id': {'type': 'string', 'index': 'not_analyzed'},
'commit': {'type': 'string', 'index': 'not_analyzed'},
'title': {'type': 'string', 'analyzer': 'default_icu'},
'content': {'type': 'string', 'analyzer': 'default_icu'},
'id': {'type': 'keyword'},
'project': {'type': 'keyword'},
'version': {'type': 'keyword'},
'path': {'type': 'keyword'},
'page_id': {'type': 'keyword'},
'commit': {'type': 'keyword'},
'title': {'type': 'text', 'analyzer': 'default_icu'},
'content': {'type': 'text', 'analyzer': 'default_icu'},
'blocks': {
'type': 'object',
'properties': {
'code': {'type': 'string', 'analyzer': 'default_icu'}
'code': {'type': 'text', 'analyzer': 'default_icu'}
}
},
# Add a weight field to enhance relevancy scoring.
Expand Down
1 change: 1 addition & 0 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ def INSTALLED_APPS(self): # noqa
if ext:
apps.append('django_countries')
apps.append('readthedocsext.donate')
apps.append('readthedocsext.search')
apps.append('readthedocsext.embed')
return apps

Expand Down
Loading