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

jraddaoui · 2024-10-29T01:11:16Z

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

+                $itemType = QubitSearch::getInstance()->index->getIndexTypeName($item['type']);
+                $search->addIndex($index)->addType($index->getType($itemType));


Instead of using a different type on each index, I'd suggest to use the dummy type name _doc, as it will be used in 7.x:

https://www.elastic.co/blog/removal-of-mapping-types-elasticsearch#schedule

Using the same type for all indices should simplify many places modified in this PR.

jraddaoui · 2024-10-29T01:14:12Z

lib/QubitLftSyncer.class.php

-        $bulk->setType('QubitInformationObject');
+        $type = 'QubitInformationObject';
+        $bulk->setIndex(QubitSearch::getInstance()->index->getType($type)->getName());
+        $bulk->setType(QubitSearch::getInstance()->index->getIndexTypeName($type));


This would be just $bulk->setType("_doc");

jraddaoui · 2024-10-29T01:19:12Z

plugins/arElasticSearchPlugin/lib/arElasticSearchIndexDecorator.class.php

    {
-        $this->_instance = $instance;
+        $this->_instance = [];


It feels strange to use an index decorator to manage multiple indexes. I'd remove this class and move the logic altogether to arElasticSearchPlugin.class.php.

I looked at removing this at first but realized that one reason we'd kept this file was that since we introduced multiple indices, we needed to use the ES index name set in the AtoM config as the prefix for all index names, and we would be repeating a lot of code for adding in the prefix if we didn't include a function for doing this.

I've renamed this class and refactored some code to clarify what this class is for.

jraddaoui · 2024-10-29T01:23:53Z

plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php

+                        !isset($propertyProperties['copy_to'])
+                        || ('_all' == $propertyProperties['copy_to'])


This condition will now copy to _all all the mapping fields where you removed include_in_all: false. It may be better to add copy_to: _all to the fields we really want to include and change this condition to isset($propertyProperties['copy_to']) && ('_all' == $propertyProperties['copy_to']).

Updated the mappings.yml file to copy all relevant mappings to _all

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.

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

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 Oct 29, 2024

jraddaoui Oct 29, 2024

jraddaoui Oct 29, 2024

anvit Nov 18, 2024

jraddaoui Oct 29, 2024

anvit Nov 18, 2024

		$itemType = QubitSearch::getInstance()->index->getIndexTypeName($item['type']);
		$search->addIndex($index)->addType($index->getType($itemType));

		!isset($propertyProperties['copy_to'])
		\|\| ('_all' == $propertyProperties['copy_to'])

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 Oct 29, 2024

Choose a reason for hiding this comment

jraddaoui Oct 29, 2024

Choose a reason for hiding this comment

jraddaoui Oct 29, 2024

Choose a reason for hiding this comment

anvit Nov 18, 2024

Choose a reason for hiding this comment

jraddaoui Oct 29, 2024

Choose a reason for hiding this comment

anvit Nov 18, 2024

Choose a reason for hiding this comment

anvit commented Oct 9, 2024 •

edited

Loading