Upgrade Elasticsearch from 7.17 to 8 #7389

makelicious · 2024-07-17T09:35:04Z

In order to fix the issue #7149 we needed to upgrade to next major version of elasticsearch.

Changes:

Add {meta: true} option to keep the payload compatible with 7.x.x format
Fix type issues revealed by the upgrade
Add search.ts to share type related items between services (migration & search)

Needs config changes from: opencrvs/opencrvs-countryconfig#216

makelicious · 2024-07-17T09:35:30Z

packages/commons/src/search.ts

@@ -0,0 +1,143 @@
+/*


Moved from search

makelicious · 2024-07-17T09:35:49Z

packages/commons/src/search.ts

+ * https://github.com/elastic/elasticsearch-js/issues/1604
+ * @param total total number of documents
+ */
+export const getSearchTotalCount = (


Only new construct for the moved file

makelicious · 2024-07-17T09:36:48Z

packages/gateway/package.json

@@ -21,7 +21,7 @@
    "precommit": "lint-staged"
  },
  "dependencies": {
-    "@elastic/elasticsearch": "7.17.13",
+    "@elastic/elasticsearch": "8.13.1",


elasticsearch.js does not need to follow the same minor as docker image. 8.14.x had some issues with undici.

makelicious · 2024-07-17T09:37:33Z

packages/migration/src/migrations/hearth/20230602120944-populate-valid-locationid.ts

@@ -79,7 +80,7 @@ export const up = async (db: Db, client: MongoClient) => {
      } catch (error: any) {
        // eslint-disable-next-line no-console
        console.error(
-          `Migration - ElasticSearch :: Process for populating missing eventLocationId/eventJurisdictionIds for ${elasticDoc.id} failed : ${error.stack}`
+          `Migration - ElasticSearch :: Process for populating missing eventLocationId/eventJurisdictionIds for ${elasticDoc._id} failed: ${error.stack}`


Type error here. On the function below, _id is used, too.

makelicious · 2024-07-17T09:38:57Z

packages/migration/src/utils/elasticsearch-helper.ts

+    return await client.update(
+      {
+        index: ELASTICSEARCH_INDEX_NAME,
+        // type: 'compositions', @todo: check whether this should work


Could someone confirm that these are not actually needed? type was deprecated and is only used here

Related to breaking change.
https://github.com/elastic/elasticsearch/pull/53670/files#diff-f5af866d898d3af4dd418c4212ad2d3ebb89e941d8ad4052a7512bf416a7e2d5L84

The types property was removed from the elasticsearch/dbhelper.ts file here: fc2594d#diff-f1ee21482bf25e832b890c7a5d40825103ec072bf33ec84275d4022b12adfd75L26
So it should safe to do so here as well

makelicious · 2024-07-17T09:39:14Z

packages/migration/src/utils/elasticsearch-helper.ts

+    return await client.search<SearchDocument>(
+      {
+        index: ELASTICSEARCH_INDEX_NAME,
+        // type: 'compositions', @todo: check whether this should work


Same as above

https://github.com/elastic/elasticsearch/pull/53670/files#diff-f5af866d898d3af4dd418c4212ad2d3ebb89e941d8ad4052a7512bf416a7e2d5L84

makelicious · 2024-07-17T09:41:18Z

packages/migration/tsconfig.json

    "allowJs": true,
-    "moduleResolution": "node",
+    "moduleResolution": "node16",


required for commons import

makelicious · 2024-07-17T09:43:03Z

packages/search/src/elasticsearch/utils.ts

 } from '@opencrvs/commons/types'
 import { findName } from '@search/features/fhir/fhir-utils'

-export const enum EVENT {


moved under commons/search

makelicious · 2024-07-17T11:06:01Z

packages/search/src/features/registration/deduplicate/service.ts

I would appreciate second pair of eyes especially here to ensure this stayed as it should

Left a single comment otherwise looked fine to me!

Zangetsu101

Great job @makelicious! Really appreciate the small refactors here & there

Zangetsu101 · 2024-07-25T04:57:55Z

docker-compose.yml

@@ -252,6 +252,7 @@ services:
      - HEARTH_MONGO_URL=mongodb://mongo1/hearth-dev
      - DASHBOARD_MONGO_URL=mongodb://mongo1/performance
      - OPENHIM_MONGO_URL=mongodb://mongo1/openhim-dev
+      - SEARCH_URL=http://search:9090/


This might just be the reason why the reindexing wasn't working properly on our servers!

Zangetsu101 · 2024-07-25T05:01:50Z

packages/migration/src/migrations/hearth/20240527130750-reindex-elasticsearch.ts

We should be able to just remove this migration as we are running the elastic reindex on every deploy from here: https://github.com/opencrvs/opencrvs-core/blob/develop/packages/migration/run-migrations.sh#L42

Zangetsu101 · 2024-07-25T05:06:58Z

packages/migration/src/utils/elasticsearch-helper.ts

+    return await client.update(
+      {
+        index: ELASTICSEARCH_INDEX_NAME,
+        // type: 'compositions', @todo: check whether this should work


The types property was removed from the elasticsearch/dbhelper.ts file here: fc2594d#diff-f1ee21482bf25e832b890c7a5d40825103ec072bf33ec84275d4022b12adfd75L26
So it should safe to do so here as well

Zangetsu101 · 2024-07-25T05:17:21Z

packages/search/src/features/registration/deduplicate/service.ts

+    bool: {
+      must: {
+        bool: {
+          must: [


Suggested change

bool: {

must: {

bool: {

must: [

bool: {

must: [

We probably don't the extra nesting right? It wasn't there previously too

No we don't! Glad that you spotted it

Zangetsu101 · 2024-07-25T05:20:52Z

packages/search/src/features/registration/deduplicate/service.ts

Left a single comment otherwise looked fine to me!

ignore types in order to test the setup

elasticsearch 8 defaults to security.enabled. Fix tests to match new payload

forced production to use http

Results to Error: connect ECONNREFUSED ::1:9090 otherwise on QA and locally

…ent calls

…ge script

compositions were removed already from db-helper earlier

🍒 Merge changes from PR #7389 to release-v1.6.0

makelicious force-pushed the deploy-elastic-8143-image-2 branch from 7c80e28 to 4aa6ab1 Compare July 17, 2024 10:05

makelicious mentioned this pull request Jul 17, 2024

Upgrade Elastic stack from 7.17 to 8 opencrvs/opencrvs-countryconfig#216

Merged

makelicious commented Jul 17, 2024

View reviewed changes

makelicious added this to the v1.6.0 milestone Jul 17, 2024

makelicious added 🛣️ Infrastructure 🙋Question 💅 Waiting For Review labels Jul 17, 2024

makelicious requested a review from Zangetsu101 July 18, 2024 09:25

Zangetsu101 reviewed Jul 25, 2024

View reviewed changes

makelicious force-pushed the deploy-elastic-8143-image-2 branch from e4edcd1 to 9747f22 Compare July 25, 2024 06:38

Zangetsu101 approved these changes Jul 25, 2024

View reviewed changes

makelicious added 15 commits July 30, 2024 09:10

chore(upgrade-elastic): bump elastic version to 8.14.3

78ce487

chore(upgrade-elastic): upgrade elastic npm packages

e75647f

chore(elastic-upgrade): ignore types

f21d7d5

ignore types in order to test the setup

chore(elastic-upgrade): use 8.14.3, add missing environment variables

a83860f

elasticsearch 8 defaults to security.enabled. Fix tests to match new payload

chore(elastic-upgrade): remove explicit connection property

14b6738

forced production to use http

chore(elastic-upgrade): add NODE_OPTIONS ipv4first for reindexing

6caf8b5

Results to Error: connect ECONNREFUSED ::1:9090 otherwise on QA and locally

chore(elastic-upgrade): ensure meta key is present on all elastic cli…

932ecb6

…ent calls

chore(elastic-upgrade): remove cross-env reference from reindex packa…

406e746

…ge script

chore(elastic-upgrade): add search url to migration docker envs

5600f93

chore(elastic-upgrade): reuse search types between services

5bf1d00

chore(elastic-upgrade): cleanup deduplicate names and types

0925e07

chore(elastic-upgrade): clean up returns and any type

49b0707

chore(elastic-upgrade): import from right file, simplify conditionals

f9cdd6f

chore(elastic-upgrade): remove accidental nesting of must bools

82a672b

chore(elastic-upgrade): remove commented out code

6645e83

compositions were removed already from db-helper earlier

makelicious force-pushed the deploy-elastic-8143-image-2 branch from 9747f22 to 6645e83 Compare July 30, 2024 06:12

makelicious marked this pull request as ready for review July 30, 2024 06:24

makelicious merged commit f107c25 into develop Jul 30, 2024
21 of 22 checks passed

makelicious deleted the deploy-elastic-8143-image-2 branch July 30, 2024 06:59

makelicious restored the deploy-elastic-8143-image-2 branch July 30, 2024 07:00

github-actions bot pushed a commit that referenced this pull request Jul 30, 2024

Placeholder commit for PR #7389

97991ac

makelicious added a commit that referenced this pull request Jul 30, 2024

Merge pull request #7411 from opencrvs/auto-pr-release-v1.6.0-7389-18086

52160df

🍒 Merge changes from PR #7389 to release-v1.6.0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Elasticsearch from 7.17 to 8 #7389

Upgrade Elasticsearch from 7.17 to 8 #7389

makelicious commented Jul 17, 2024 •

edited

Loading

makelicious Jul 17, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

Zangetsu101 Jul 25, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

makelicious Jul 17, 2024

Zangetsu101 Jul 25, 2024

Zangetsu101 left a comment

Zangetsu101 Jul 25, 2024

Zangetsu101 Jul 25, 2024

Zangetsu101 Jul 25, 2024

Zangetsu101 Jul 25, 2024

makelicious Jul 25, 2024

Zangetsu101 Jul 25, 2024

-    bool: {
-      must: {
-        bool: {
-          must: [
+    bool: {
+      must: [

Upgrade Elasticsearch from 7.17 to 8 #7389

Upgrade Elasticsearch from 7.17 to 8 #7389

Conversation

makelicious commented Jul 17, 2024 • 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

Zangetsu101 left a 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

makelicious commented Jul 17, 2024 •

edited

Loading