Add thin ES clients #5162

mantas-sidlauskas · 2023-03-15T14:25:33Z

What changed?

Adding v6 and v7 ES clients compatible with new interface
Bulk processor moved to a separate package

Why?
Bulk processor was moved to avoid cyclic dependency between elasticsearch package and ES clients
New ES clients need to wired in a separate PR

How did you test it?

Potential risks

Release notes

Documentation Changes

coveralls · 2023-03-17T08:19:40Z

Pull Request Test Coverage Report for Build 0187608e-66f2-48fc-8259-493cbf8ccf15

23 of 49 (46.94%) changed or added relevant lines in 4 files are covered.
55 unchanged lines in 11 files lost coverage.
Overall coverage increased (+0.05%) to 57.121%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
service/worker/indexer/indexer.go	2	4	50.0%
common/elasticsearch/client_v6_bulk.go	14	19	73.68%
common/elasticsearch/client_v7_bulk.go	0	19	0.0%

Files with Coverage Reduction	New Missed Lines	%
service/history/queue/timer_queue_processor_base.go	1	77.26%
common/membership/hashring.go	2	83.54%
common/task/parallelTaskProcessor.go	2	92.75%
common/task/weightedRoundRobinTaskScheduler.go	2	89.64%
service/matching/matcher.go	2	90.65%
common/persistence/nosql/nosqlplugin/cassandra/workflow.go	3	60.0%
common/persistence/statsComputer.go	3	94.64%
service/history/task/transfer_standby_task_executor.go	4	87.66%
service/history/decision/task_handler.go	5	72.33%
common/persistence/nosql/nosqlplugin/cassandra/workflowUtils.go	12	74.98%

Totals
Change from base Build 01874e2d-13b5-4a33-b7c8-2d7da3311e7b:	0.05%
Covered Lines:	85311
Relevant Lines:	149351

💛 - Coveralls

Groxx · 2023-03-27T21:21:50Z

Since there seems to be a substantial amount of new code in here:
what's the goal / purpose? and/or where are these new requirements coming from, since no libraries have been upgraded?

(part of this is that I'm simply unfamiliar with this code, it might be obvious)

mantas-sidlauskas · 2023-03-28T04:31:11Z

Since there seems to be a substantial amount of new code in here: what's the goal / purpose? and/or where are these new requirements coming from, since no libraries have been upgraded?

(part of this is that I'm simply unfamiliar with this code, it might be obvious)

As this and #5168 are related, let me add some more context here:

olivere v6/v7 libraries now are deprecated and author recommends to use "official" Elasticsearch clients.
There will be no olivere v8
I'm adding support for Opensearch client

To reuse business logic while using different versions of Elasticsearch/OpenSearch, we have to keep "versioned" clients small and only doing client/server communication.

#5168 adds a possibility to build queries in identical way but without depending on external packages/structs.

The code in this PR is not used right now, but needs to be wired in the next PR: common/elasticsearch/client_{v6|v7}.go will be replaced with a single client.go and will use code provided in this PR.

common/elasticsearch/client/v6/client.go

Shaddoll · 2023-04-07T00:08:34Z

common/elasticsearch/client/v6/client.go

+
+	if esResult != nil && esResult.Hits != nil {
+		for _, h := range esResult.Hits.Hits {
+			hits = append(hits, &client.SearchHit{Source: *h.Source})


the nil check is missing in Search func

Shaddoll · 2023-04-07T00:08:39Z

common/elasticsearch/client/v6/client.go

+	if len(esResult.Aggregations) > 0 {
+		result.Aggregations = make(map[string]json.RawMessage, len(esResult.Aggregations))
+		for key, agg := range esResult.Aggregations {
+			result.Aggregations[key] = *agg


the nil check is missing in Search func

nil check is added

commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients

commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients Merge issue

commit b18be27 Author: Mantas Šidlauskas <mantass@netapp.com> Date: Tue Apr 18 14:12:09 2023 +0300 Add generic ES query building utilities (#5168) commit 824f0ac Author: agautam478 <72432016+agautam478@users.noreply.github.com> Date: Fri Apr 14 13:37:29 2023 -0700 Fixed the nil pointer issues in the InactiveDomain Invariant (#5213) commit c5678dd Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com> Date: Wed Apr 12 15:21:10 2023 -0700 Fix consistent query metric (#5170) * add shardid tag to log * remove counter for overall scope * fix lint commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients

mantas-sidlauskas force-pushed the es_multi_clients branch 2 times, most recently from 9cd06bb to d16137d Compare March 17, 2023 07:04

Shaddoll reviewed Mar 30, 2023

View reviewed changes

common/elasticsearch/client/v6/client.go Show resolved Hide resolved

mantas-sidlauskas force-pushed the es_multi_clients branch from c2f5411 to 4670263 Compare April 4, 2023 10:53

Shaddoll reviewed Apr 4, 2023

View reviewed changes

common/elasticsearch/client/v6/client.go Outdated Show resolved Hide resolved

Shaddoll reviewed Apr 4, 2023

View reviewed changes

common/elasticsearch/client/v6/client.go Outdated Show resolved Hide resolved

Shaddoll reviewed Apr 7, 2023

View reviewed changes

mantas-sidlauskas added 6 commits April 8, 2023 14:01

Add thin ES clients

5b1afaa

fix merged code

a380378

Check scroll result for nil

b7417af

lint

0449168

check for nil

45e02e6

add nil checks in Search

81a7b69

mantas-sidlauskas force-pushed the es_multi_clients branch from 20598ff to 81a7b69 Compare April 8, 2023 11:09

Shaddoll approved these changes Apr 10, 2023

View reviewed changes

Shaddoll merged commit b2bc8bf into uber:master Apr 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add thin ES clients #5162

Add thin ES clients #5162

mantas-sidlauskas commented Mar 15, 2023

coveralls commented Mar 17, 2023 •

edited

Loading

Groxx commented Mar 27, 2023

mantas-sidlauskas commented Mar 28, 2023

Shaddoll Apr 7, 2023

mantas-sidlauskas Apr 8, 2023

Shaddoll Apr 7, 2023

mantas-sidlauskas Apr 8, 2023

Add thin ES clients #5162

Add thin ES clients #5162

Conversation

mantas-sidlauskas commented Mar 15, 2023

coveralls commented Mar 17, 2023 • edited Loading

Pull Request Test Coverage Report for Build 0187608e-66f2-48fc-8259-493cbf8ccf15

💛 - Coveralls

Groxx commented Mar 27, 2023

mantas-sidlauskas commented Mar 28, 2023

Shaddoll Apr 7, 2023

Choose a reason for hiding this comment

mantas-sidlauskas Apr 8, 2023

Choose a reason for hiding this comment

Shaddoll Apr 7, 2023

Choose a reason for hiding this comment

mantas-sidlauskas Apr 8, 2023

Choose a reason for hiding this comment

coveralls commented Mar 17, 2023 •

edited

Loading