Update ElasticSearch and Elastica to 6.x #1880

anvit · 2024-10-09T20:07:12Z

Update ElasticSearch and Elastica and address issues introduced by breaking changes in ElasticSearch 6.x.

The most notable changes are:

Dropped support for multiple mapping types: ES 6.x no longer allows using multiple indices, They suggest handling this by creating multiple indices, each with its own type.
Mapping changes: The include_in_all mapping parameter is disallowed and the _all field is disabled by default.

Changes in Elastica:

Elastica only has a addDocuments, deleteDocuments, and updateDocuments methods in 6.x and there are no longer addDocument, deleteDocument, updateDocument methods for handling a singular documents.

jraddaoui

Looking great @anvit!

I have not been able to look at everything in detail yet, but I have added a few comments that I think are important and may require some work already. Let me know your thoughts ;)

apps/qubit/modules/search/actions/autocompleteAction.class.php

lib/QubitLftSyncer.class.php

plugins/arElasticSearchPlugin/lib/arElasticSearchIndexDecorator.class.php

plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php

Update ElasticSearch to 6.8.23 in the Dockerfile and update Elastica to 6.x in composer.

- 'inline' scripts are deprecated, changed to 'source'

Update arElasticSearchPlugin to use multiple indices instead of multiple types since ES 6.x removed being able to add multiple types. Also update mapping to remove include_in_all and add that to the _all field using copy_to instead since include_in_all was also removed in ES 6.x

Update autocompleteAction to use multiple indices

Add the appropriate index types for ES requests that did not need to specify explicit types in ES 5.x

Add missing document type to partialUpdate in arElasticSearchPlugin

Update condition in arElasticSearchPluginUtil to only get string fields included in _all if copy_to is explicitly set to _all in the mappping.

Renamed arElasticSearchIndexDecorator as arElasticSearchMultiIndexWrapper for clarity and added a few comments about changes needed for ElasticSearch/Elastica 7.x.

Rename getType from arElasticSearchMultiIndexWrapper to getIndex for clarity.

Add a dummy ElasticSearch index type since one is still required for ES 6.x, instead of having a seperate type name for each of the multiple indices.

jraddaoui

Thanks @anvit! This looks a lot clearer to me now.

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php

plugins/arElasticSearchPlugin/config/mapping.yml

Update any references to ElasticSearch index type to index name. Also add getType as an alias for getIndex for backwards compatibility with custom themes that still might be making references to ElasticSearch's getType.

Removed copy_to _all from all fields that previously had include_in_all set to false.

Remove unnecessary $type ElasticSearch variables in QubitLftSyncer, arElasticSearchPlugin, and updatePublicationStatusTask

Remove the index prefix from arElasticSearchMultiIndexWrapper and use AtoM class names internally instead of using prefix to refer to them.

jraddaoui

Thanks @anvit! Getting there, just a few minor tweaks!

apps/qubit/modules/search/actions/autocompleteAction.class.php

lib/QubitLftSyncer.class.php

plugins/arElasticSearchPlugin/lib/arElasticSearchMultiIndexWrapper.class.php

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php

jraddaoui · 2024-11-22T10:59:57Z

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php

        $this->log('Types that will be indexed:');

-        foreach ($this->mappings as $typeName => $typeProperties) {
-            if (!in_array(strtolower($typeName), $excludeTypes)) {
-                $this->log(' - '.$typeName);
+        foreach ($this->mappings as $indexName => $indexProperties) {
+            if (!in_array(strtolower($indexName), $excludeTypes)) {
+                $this->log(' - '.$indexName);
                ++$typeCount;
            }
        }


We may want to change this too, "Indices that will be updated." and use the prefixed index name.

Okay, maybe not here. These need to match the exclude-types argument from the search:populate task.

Now that I think about that, we should probably revisit how is that done with multiple indexes instead of types.

plugins/arElasticSearchPlugin/config/mapping.yml

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php

jraddaoui · 2024-11-22T13:21:23Z

plugins/arElasticSearchPlugin/lib/arElasticSearchMultiIndexWrapper.class.php

+    public function delete()
+    {
+        foreach ($this->indices as $index) {
+            $index->delete();
+        }
+    }


Given all the feedback above, this would be the only function without a $name parameter in the wrapper (apart from the constructor). And, when we revisit the exclude-types option I think we'll add that parameter.

It makes a lot of sense to me that all methods from this class require a name parameter now that it's a multi index wrapper instead of an index decorator.

It would be even clearer if $index was called $indicesWrapper in arElasticSearchPlugin, but preventing issues in custom code is a stronger point than clarity.

Just a note.

Fix bug in search autocompleteAction which was showing duplicate results for actors under repositories. Also address some of the CR feedback and update/add methods for add, update, and deleteById in the arElasticSearchMultiIndexWrapper.

Remove unnecessary foreach loop in arElasticSearchPlugin. Also move code for adding filter for stripping md tags into a seperate method to make the initialization code more readable.

jraddaoui

Nice, thanks @anvit!

apps/qubit/modules/search/actions/autocompleteAction.class.php

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php

plugins/arElasticSearchPlugin/config/mapping.yml

plugins/arElasticSearchPlugin/lib/arElasticSearchMultiIndexWrapper.class.php

Update the index refresh method to accept an optional name param that only refreshes that particular index in arElasticSearchMultiIndexWrapper. Also address some small CR feedback.

anvit added the work-in-progress label Oct 9, 2024

anvit force-pushed the dev/upgrade-elasticsearch branch from 9f05a24 to 1e9114f Compare October 10, 2024 00:48

anvit removed the work-in-progress label Oct 10, 2024

anvit self-assigned this Oct 10, 2024

anvit requested a review from a team October 10, 2024 16:22

anvit added this to the 2.9.0 milestone Oct 10, 2024

anvit force-pushed the dev/upgrade-elasticsearch branch from 1e9114f to e6a7473 Compare October 10, 2024 22:11

anvit force-pushed the dev/upgrade-elasticsearch branch from 7bdf937 to 0842eb9 Compare October 21, 2024 23:49

jraddaoui requested changes Oct 29, 2024

View reviewed changes

anvit and others added 10 commits November 15, 2024 15:53

Update ElasticSearch and Elastica dependencies

81617d6

Update ElasticSearch to 6.8.23 in the Dockerfile and update Elastica to 6.x in composer.

Remove arElasticSearchIndexDecorator.

7c50884

- 'inline' scripts are deprecated, changed to 'source'

Fix autocompleteAction for ES 6.x

ec4e748

Update autocompleteAction to use multiple indices

Add index types for ES Queries without a type

a19f8e1

Add the appropriate index types for ES requests that did not need to specify explicit types in ES 5.x

Fix updateByQuery calls for ElasticSearch 6.x

90d1e08

Fix broken publication status update

47cc1a9

Add missing document type to partialUpdate in arElasticSearchPlugin

Fix linting errors

3adaac0

Switch elasticsearch to the oss docker image

5575f96

Update mapping condition for _all fields

0259ed0

Update condition in arElasticSearchPluginUtil to only get string fields included in _all if copy_to is explicitly set to _all in the mappping.

anvit force-pushed the dev/upgrade-elasticsearch branch from abb1768 to 0259ed0 Compare November 15, 2024 23:53

anvit added 3 commits November 18, 2024 13:35

Update ES mapping to copy relevant fields to _all

a13821d

Rename arElasticSearchIndexDecorator

3f0e4d6

Renamed arElasticSearchIndexDecorator as arElasticSearchMultiIndexWrapper for clarity and added a few comments about changes needed for ElasticSearch/Elastica 7.x.

Rename getType function from ESMultiIndexWrapper

0e0d87d

Rename getType from arElasticSearchMultiIndexWrapper to getIndex for clarity.

anvit force-pushed the dev/upgrade-elasticsearch branch from bbf4bb1 to 0e0d87d Compare November 19, 2024 00:09

anvit requested a review from jraddaoui November 19, 2024 00:18

anvit force-pushed the dev/upgrade-elasticsearch branch from a37039a to 4b52458 Compare November 19, 2024 00:49

Add dummy type for ES index

bc54543

Add a dummy ElasticSearch index type since one is still required for ES 6.x, instead of having a seperate type name for each of the multiple indices.

anvit force-pushed the dev/upgrade-elasticsearch branch from 4b52458 to bc54543 Compare November 19, 2024 01:02

jraddaoui reviewed Nov 20, 2024

View reviewed changes

Change references to index type for ElasticSearch

9d6fa03

Update any references to ElasticSearch index type to index name. Also add getType as an alias for getIndex for backwards compatibility with custom themes that still might be making references to ElasticSearch's getType.

anvit force-pushed the dev/upgrade-elasticsearch branch from e86af5b to 9d6fa03 Compare November 21, 2024 23:59

Fix copy to _all fields for ES mapping

2ff7ea4

Removed copy_to _all from all fields that previously had include_in_all set to false.

anvit force-pushed the dev/upgrade-elasticsearch branch from a583518 to 2ff7ea4 Compare November 22, 2024 00:29

anvit added 2 commits November 21, 2024 16:56

Remove unnecessary ElasticSearch type variables

9adb350

Remove unnecessary $type ElasticSearch variables in QubitLftSyncer, arElasticSearchPlugin, and updatePublicationStatusTask

Remove ES index prefix from MultiIndexWrapper

fc6d069

Remove the index prefix from arElasticSearchMultiIndexWrapper and use AtoM class names internally instead of using prefix to refer to them.

jraddaoui reviewed Nov 22, 2024

View reviewed changes

plugins/arElasticSearchPlugin/lib/arElasticSearchPlugin.class.php Outdated Show resolved Hide resolved

jraddaoui reviewed Nov 22, 2024

View reviewed changes

anvit added 3 commits November 22, 2024 12:04

Fix autocompleteAction bug, address CR feedback

c6f0c2a

Fix bug in search autocompleteAction which was showing duplicate results for actors under repositories. Also address some of the CR feedback and update/add methods for add, update, and deleteById in the arElasticSearchMultiIndexWrapper.

Remove non text fields from _all ES mapping

9859544

Remove unnecessary loop in arElasticSearchPlugin

ef14555

Remove unnecessary foreach loop in arElasticSearchPlugin. Also move code for adding filter for stripping md tags into a seperate method to make the initialization code more readable.

anvit force-pushed the dev/upgrade-elasticsearch branch from c5bbe4a to ef14555 Compare November 22, 2024 20:48

jraddaoui reviewed Nov 25, 2024

View reviewed changes

plugins/arElasticSearchPlugin/lib/arElasticSearchMultiIndexWrapper.class.php Outdated Show resolved Hide resolved

Update ES index refresh to only refresh one index

02b3c4b

Update the index refresh method to accept an optional name param that only refreshes that particular index in arElasticSearchMultiIndexWrapper. Also address some small CR feedback.

anvit force-pushed the dev/upgrade-elasticsearch branch from a94554a to 02b3c4b Compare November 25, 2024 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ElasticSearch and Elastica to 6.x #1880

Update ElasticSearch and Elastica to 6.x #1880

anvit commented Oct 9, 2024 •

edited

Loading

jraddaoui left a comment

jraddaoui left a comment

jraddaoui left a comment

jraddaoui Nov 22, 2024

jraddaoui Nov 22, 2024

jraddaoui Nov 22, 2024

jraddaoui left a comment

Update ElasticSearch and Elastica to 6.x #1880

Are you sure you want to change the base?

Update ElasticSearch and Elastica to 6.x #1880

Conversation

anvit commented Oct 9, 2024 • edited Loading

jraddaoui left a comment

Choose a reason for hiding this comment

jraddaoui left a comment

Choose a reason for hiding this comment

jraddaoui left a comment

Choose a reason for hiding this comment

jraddaoui Nov 22, 2024

Choose a reason for hiding this comment

jraddaoui Nov 22, 2024

Choose a reason for hiding this comment

jraddaoui Nov 22, 2024

Choose a reason for hiding this comment

jraddaoui left a comment

Choose a reason for hiding this comment

anvit commented Oct 9, 2024 •

edited

Loading