[BD-19] Transition to the new Elasticsearch libs version for cs_comments_service #327

idegtiarov · 2020-08-26T08:12:31Z

Description:

Updates versions of the ES and the ES DSL libs from V1.5.2 to V7.8.0
Splits content index with comment and comment_thread doc_types to comment and comment_thread indices.

Was done:

update ES libs version;
rewrite index documents for compatibility with ES7;
update make commands;
update travis ci.

Current code coverage: 95.78%.

Reviewers:
@mikix
@dianakhuang

Post merge:

Delete working branch

openedx-webhooks · 2020-08-26T08:12:36Z

Thanks for the pull request, @idegtiarov! I've created BLENDED-561 to keep track of it in Jira. More details are on the BD-19 project page.

When this pull request is ready, tag your edX technical lead.

…model

Golub-Sergey · 2020-09-08T15:10:44Z

When we search by title or body ES allow to make 2 mistakes by default (max_edits option), do we need increase amount of edits?
Doc link

Notes: before start to test forum locally you have to rebuild forum indices. It can be made by command bin/rake search:rebuild_indices. It can be used once during running forum service.

davidjoy · 2020-09-22T21:11:56Z

Edit: The below is resolved. It's unclear what fixed it for me, unfortunately.

So I'm currently seeing the following error when I try to create a new post:

Elasticsearch::Transport::Transport::Errors::BadRequest at /api/v1/i4x-edx-eiorguegnru-course-foobarbaz/threads
[400] {"error":"MapperParsingException[mapping [_doc]]; nested: InvalidTypeNameException[mapping type name [_doc] can't start with '_']; ","status":400}

Not quite sure what that's about - I'm seeing documentation on Elasticsearch's site about the removal of types in Elasticsearch 7.

https://www.elastic.co/guide/en/elasticsearch/reference/7.x/removal-of-types.html

I'm not sure how this affects the declarations of mappings in comment.rb and comment_thread.rb / how it may work with the ruby elasticsearch library.

Does creating a post work for you? Curious if there's just something I'm missing. (To be clear: I've rebuilt my indices as described in your last comment)

davidjoy · 2020-09-23T19:36:49Z

.travis/elasticsearch.yml

@@ -0,0 +1,5 @@
+cluster.name: "docker-cluster"


Is the cluster.name necessary here? I think "docker-cluster" may be the default...

cluster.name is not necessary, by default cluster.name is elasticsearch, link to the doc.
Done

davidjoy · 2020-09-24T21:09:05Z

.travis/docker-compose-travis.yml

+      - discovery.type=single-node
+      - bootstrap.memory_lock=true
+      - "ES_JAVA_OPTS=-Xms512m -Xmx512m"
+    ulimits:


For what it's worth, the container I'm using from devstack doesn't include these entire entries:

ulimits

ports

And seems to work fine. Are these particularly necessary for travis or something?

Done. It is not necessary

davidjoy · 2020-09-24T21:09:34Z

.travis/docker-compose-travis.yml

+
+volumes:
+  data01:
+    driver: local


I also don't seem to need this driver: local line either, far as I can tell.

davidjoy · 2020-09-24T21:11:14Z

README.rst


 .. code-block:: bash

    $ bin/rake search:initialize

-To validate the 'content' alias exists and contains the proper mappings:
+To validate indices existence and content of the proper mappings:


"To validate indices exist and contain the proper mappings:" would be with the spirit of the original sentence, I think.

lib/task_helpers.rb

davidjoy · 2020-09-24T21:17:03Z

lib/task_helpers.rb

-        LOG.info "Unable to delete non-existent index: #{name}."
-      end
+    def self.delete_indices
+      Elasticsearch::Model.client.indices.delete(index: INDEX_NAMES, ignore_unavailable: true)


Nice simplification.

davidjoy · 2020-09-24T21:17:38Z

lib/task_helpers.rb

-      end
+    def self.delete_indices
+      Elasticsearch::Model.client.indices.delete(index: INDEX_NAMES, ignore_unavailable: true)
+      LOG.info "Delete indices."


This should maybe be "deleted" instead of "delete", since it's saying it's been completed, rather than starting.

davidjoy · 2020-09-24T21:18:40Z

lib/task_helpers.rb

-            LOG.error "Error indexing. Response was: #{response}"
-        end
-        LOG.info "Imported batch #{batch_number} into the index"
-        sleep(sleep_time)


Why is it we no longer need to sleep here? (I'm pre-supposing there was a good reason, but maybe there wasn't?)

When we load data to the index ES is acknowledged that index was updated. ES periodically refresh index.
Indices refresh description doc.

davidjoy · 2020-09-24T21:19:48Z

lib/task_helpers.rb

-    def self.exists_alias(alias_name)
-      Elasticsearch::Model.client.indices.exists_alias(name: alias_name)
+    def self.exists_indices
+      Elasticsearch::Model.client.indices.exists(index: INDEX_NAMES)


This indices.exists function can accept an array? What does it do if one exists but the other doesn't?

I'm having trouble finding any documentation that these functions like exists and refresh take an array as their index option...

Ahh, okay, I think I found it:

https://www.rubydoc.info/gems/elasticsearch-api/Elasticsearch/API/Indices/Actions#exists-instance_method

davidjoy · 2020-09-24T21:44:34Z

api/search.rb

+    ],
+    query: {
+      bool: {
+        must: must,


Has this changed the search behavior, splitting the filters from the old code into must and should? From line 41 on the other side, I would have expected they were all "musts" before.

Maybe I'll just generalize that question to ask - have the changes to this file resulted in any differences in how we perform our queries/what results they return? Can you tell me a little about what needed to change here? It's hard to follow from the diff alone.

As I see from test results we get expected result of search behaviour.
The filtered query is replaced by the bool query. Instead of the following. I use should to optimise filter query which look like, for example:

filter: { bool: { must: filters } }

where filters

{ bool: { should: [ {:not => {:exists => {:field => :group_id}}}, {:terms => {:group_id => group_ids}} ] } }

And ES haven't critical difference between should and filter clauses. Useful doc link which explain should and filter difference.
Now search query looks simpler, I think.

davidjoy

In general I think this all looks good - I have some questions I just wanted to talk through before Approving and moving forward. Thanks!

davidjoy · 2020-09-25T17:15:41Z

I'm hearing from our team that sharding of the index is something we should be thinking about, as the older version of Elasticsearch handled it automatically, but the newer one doesn't. Admittedly, I don't know much about this, so I'm not going to be a great resource. I'd suggest if we need some help deciding what needs to be done here, we query the group in our shared Slack channel.

Golub-Sergey · 2020-09-30T19:51:05Z

@davidjoy pr to the rg forum which can be useful if the alias functionality should be returned.

Review can go on, thanks for your helps, comments and questions !)

…e-alias-functionality returns move alias functionality

lib/tasks/search.rake

lib/task_helpers.rb

Golub-Sergey · 2020-10-06T21:49:15Z

@davidjoy I returned move_alias at the end of rebuild indices, as it should be. Thanks for your comments!

Use a different SEARCH_SERVER variable for ES7 code.

lib/tasks/search.rake

davidjoy

I think this is ready to go! Tests all pass for me, and everything I'm able to test through the UI and the rake commands works too. Great job!

openedx-webhooks added blended PR is managed through 2U's blended developmnt program needs triage labels Aug 26, 2020

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from 1be5be7 to 0b4ceb4 Compare August 31, 2020 07:07

Golub-Sergey added 10 commits August 31, 2020 10:21

OeX_ES-39: updates requirements

9c48e8a

OeX_ES-39: updates indices mappings description, adds index name per …

54126d8

…model

OeX_ES-39: split doc_types into indexes, removes doc_types

a1dc0ac

OeX_ES-39: updates search query building

2ce3649

OeX_ES-39: updates commands

0e3002d

OeX_ES-39: updates tests

2bc2ccf

OeX_ES-39: updates docker containers for testing

78b0fe6

OeX_ES-39: updates README description

ec8359e

OeX_ES-39: resolves pr issues

bc03b4b

OeX_ES-39: returns deleted code

e92abfb

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from 0b4ceb4 to e92abfb Compare August 31, 2020 07:22

OeX_ES-39: adds title suggester, title field to the comment mappings

370e16c

davidjoy reviewed Sep 23, 2020

View reviewed changes

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from fd188ce to 77f3eba Compare September 24, 2020 13:00

OeX_ES-39: removes redundant setting from elasticsearch configs

7dca2df

davidjoy reviewed Sep 24, 2020

View reviewed changes

lib/task_helpers.rb Show resolved Hide resolved

davidjoy reviewed Sep 24, 2020

View reviewed changes

lib/task_helpers.rb Show resolved Hide resolved

davidjoy reviewed Sep 24, 2020

View reviewed changes

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch 2 times, most recently from 027656e to d46589e Compare September 25, 2020 16:16

Golub-Sergey mentioned this pull request Sep 30, 2020

Golub sergey/bd 19/returns move alias functionality raccoongang/cs_comments_service#7

Merged

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from d46589e to fb39d1e Compare October 1, 2020 09:46

OeX_ES-39: resolves pr issues

b18045f

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from fb39d1e to b18045f Compare October 1, 2020 09:51

Golub-Sergey added 3 commits October 5, 2020 12:10

OEX-39: returns aliases functionality

6fdd719

OEX-39: updates tests

19eb514

Merge pull request #7 from raccoongang/golub-sergey/BD-19/returns-mov…

3d1f02a

…e-alias-functionality returns move alias functionality

davidjoy reviewed Oct 5, 2020

View reviewed changes

lib/tasks/search.rake Outdated Show resolved Hide resolved

davidjoy reviewed Oct 5, 2020

View reviewed changes

lib/task_helpers.rb Outdated Show resolved Hide resolved

dianakhuang and others added 2 commits October 6, 2020 13:44

Use a different SEARCH_SERVER variable for ES7 code.

881aa06

OEX-39: edit forum's aliases functionality

56b48bb

Merge pull request #8 from edx/diana/update-ES-variables

efd39dc

Use a different SEARCH_SERVER variable for ES7 code.

davidjoy reviewed Oct 8, 2020

View reviewed changes

lib/tasks/search.rake Outdated Show resolved Hide resolved

Golub-Sergey added 3 commits October 8, 2020 23:54

OEX-39: fixes travis ci job failure

8d8821d

OEX-39: edits README description

6fbb136

OEX-39: edits docker compose file for travis and edit rake command

baf9e7f

Golub-Sergey force-pushed the golub-sergey/OeX_ElasticSearch/feature/transition-from-ES1.5-to-ES7 branch from 5cdb90f to baf9e7f Compare October 8, 2020 22:26

Golub-Sergey added 2 commits October 11, 2020 22:28

OEX-39: fixes tests of the rake command

0761a42

OEX-39: edit test settings for avoiding huge log during each test

af1dbf1

davidjoy approved these changes Oct 13, 2020

View reviewed changes

dianakhuang merged commit 573567e into openedx:master Oct 14, 2020

openedx-webhooks added merged and removed needs triage labels Oct 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-19] Transition to the new Elasticsearch libs version for cs_comments_service #327

[BD-19] Transition to the new Elasticsearch libs version for cs_comments_service #327

idegtiarov commented Aug 26, 2020

openedx-webhooks commented Aug 26, 2020 •

edited

Loading

Golub-Sergey commented Sep 8, 2020 •

edited

Loading

davidjoy commented Sep 22, 2020 •

edited

Loading

davidjoy Sep 23, 2020

Golub-Sergey Sep 24, 2020 •

edited

Loading

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020 •

edited

Loading

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy Sep 24, 2020

davidjoy Sep 24, 2020

davidjoy Sep 24, 2020

davidjoy Sep 24, 2020

davidjoy Sep 24, 2020

Golub-Sergey Sep 25, 2020

davidjoy left a comment

davidjoy commented Sep 25, 2020

Golub-Sergey commented Sep 30, 2020

Golub-Sergey commented Oct 6, 2020

davidjoy left a comment

[BD-19] Transition to the new Elasticsearch libs version for cs_comments_service #327

[BD-19] Transition to the new Elasticsearch libs version for cs_comments_service #327

Conversation

idegtiarov commented Aug 26, 2020

openedx-webhooks commented Aug 26, 2020 • edited Loading

Golub-Sergey commented Sep 8, 2020 • edited Loading

davidjoy commented Sep 22, 2020 • edited Loading

Choose a reason for hiding this comment

Golub-Sergey Sep 24, 2020 • 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

Golub-Sergey Sep 25, 2020 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidjoy left a comment

Choose a reason for hiding this comment

davidjoy commented Sep 25, 2020

Golub-Sergey commented Sep 30, 2020

Golub-Sergey commented Oct 6, 2020

davidjoy left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Aug 26, 2020 •

edited

Loading

Golub-Sergey commented Sep 8, 2020 •

edited

Loading

davidjoy commented Sep 22, 2020 •

edited

Loading

Golub-Sergey Sep 24, 2020 •

edited

Loading

Golub-Sergey Sep 25, 2020 •

edited

Loading