Repository navigation
Support OpenSearch 2 and 3, with an open-source Docker image - #2449
Draft
johanpiet2 wants to merge 11 commits into
Draft
johanpiet2 wants to merge 11 commits into
johanpiet2 wants to merge 11 commits into
Conversation
QubitSearchEngine declares everything AtoM asks of its search index: document writes, populate and optimize, and per-entity search, multi-search, get, count, update by query, scroll and bulk partial update, plus the server version. arElasticSearchPlugin implements the new methods with the Elastica calls its callers make today, so nothing changes in behaviour. QubitSearch takes the engine class from the app_search_engine setting, defaulting to arElasticSearchPlugin, and refuses a class that is not a QubitSearchEngine. Refs artefactual#2285.
The 22 places outside arElasticSearchPlugin that searched with QubitSearch::getInstance()->index->getIndex($entity)->search($query) now call QubitSearch::getInstance()->search($entity, $query). The query and the result set are unchanged. Refs artefactual#2285.
findExportRecords() in the actor, accession and description export jobs now returns the query instead of an Elastica Search, and each job names the index it searches in SEARCH_ENTITY. The jobs and the clipboard export count with QubitSearch::count() and read the matching ids with QubitSearch::scrollIds(); the repository CSV export does the same. The count is taken once per export instead of twice. Refs artefactual#2285.
The publication status job and task send their Painless script through QubitSearch::updateByQuery() and read the response array. The actor relations job and search:document read stored documents with QubitSearch::getDocument(), which answers null for a document that is not indexed (the job treats it as no relations, as before; the task now says so instead of failing on Elastica's exception). search:status takes the server version and the per-index counts from QubitSearch. Refs artefactual#2285.
The same change as the previous commit for calls written over several lines: the image flow, the full tree view (including its count), the accession browse and the lft syncer's check now search and count through QubitSearch. Refs artefactual#2285.
Autocomplete builds its six queries and runs them with QubitSearch::multiSearch(), which answers the result sets in the same order, instead of assembling an Elastica multi-search from the engine's client and indices. QubitLftSyncer sends its lft values with QubitSearch::bulkPartialUpdate(). With this, nothing outside arElasticSearchPlugin uses the engine's client or index objects. Refs artefactual#2285.
Each QubitSearchEngine method is checked against the configured engine and the test installation's data, without changing the index, so a second engine can be held to the same contract. Refs artefactual#2285.
false has been the default since Elasticsearch 7.0, AtoM's minimum, so
the parameter changes nothing there; OpenSearch removed it and refuses
the mapping request ("unrecognized parameter: [include_type_name]").
The third argument to Index::create() is dropped as well: Elastica's
create() takes two, so it was already ignored.
Refs artefactual#2285.
OpenSearch serves the same REST API as Elasticsearch 7.10 under its own version numbers and reports "distribution": "opensearch". The version check now reads the distribution and requires OpenSearch 2.0 or later, or Elasticsearch 7.0 or later as before, naming the right product in its message. getVersion() still answers the version number. Checked on the Docker dev environment with OpenSearch 2.19.1 and 3.3.2 and, unchanged, Elasticsearch 7.10.2: search:populate, PHPUnit (569 tests) and Cypress in Electron (13 tests) pass on all three. Refs artefactual#2285.
docker-compose.opensearch.yml, added after the dev or test file, runs the official opensearchproject/opensearch image in place of Elasticsearch, with the security plugin off as for development. The service keeps the name "elasticsearch", so AtoM's settings and port 63002 do not change, and stores its data in its own volume. Refs artefactual#2285.
Author
|
Chrome and Firefox are now run as well (the descriptions said they were still to do). On the Docker dev environment with this branch (which includes #2448), Cypress passes 13/13 in Chrome 156, Firefox 158 and Electron, against both Elasticsearch 7.10.2 and OpenSearch 2.19.1. |
sarah-mason
added
work-in-progress
Request: discussion
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #2285. Builds on #2448 (the search engine contract), whose commits this branch includes; the three new commits are at the top. Draft until #2448 is reviewed.
What this does
AtoM runs on OpenSearch with the existing
arElasticSearchPlugin, and the Docker setup can use the official open-sourceopensearchproject/opensearchimage, which deals with the closed-image blocker for the development environment.It turned out to need very little, because OpenSearch serves the same REST API as Elasticsearch 7.10:
include_type_namewhen creating indices.falsehas been Elasticsearch's default since 7.0, AtoM's minimum, so nothing changes there; OpenSearch removed the parameter and refused the mapping request. (The third argument toIndex::create()goes too: Elastica'screate()takes two, so it was already ignored.)"distribution": "opensearch"; the check now reads the distribution, and its message names the right product.docker/docker-compose.opensearch.yml, added after the dev or test file in the same way as the Varnish and PMM files:export COMPOSE_FILE="$PWD/docker/docker-compose.dev.yml:$PWD/docker/docker-compose.opensearch.yml"The service keeps the name
elasticsearch, so AtoM's settings and port 63002 stay the same.Tests
On the Docker dev environment, each engine with a fresh
search:populate:docker/etc/environment)PHPUnit includes the contract test from #2448. PHP CS Fixer and Prettier are clean.
Not in this PR
ruflin/elastica) are the later steps on ElasticSearch replaced by OpenSearch 3.4 #2285.Questions: should the dev and test files default to OpenSearch now that it works, keeping Elasticsearch as the add-on instead; and which engines should CI cover?