Fix consistent query metric #5170
Merged
Conversation
This file contains 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
Groxx
reviewed
Mar 27, 2023
|
|
||
| consistentQueryEnabled := e.config.EnableConsistentQuery() && e.config.EnableConsistentQueryByDomain(request.GetRequest().GetDomain()) | ||
| if request.GetRequest().GetQueryConsistencyLevel() == types.QueryConsistencyLevelStrong { | ||
| if !consistentQueryEnabled { | ||
| return nil, workflow.ErrConsistentQueryNotEnabled | ||
| } | ||
| scope.IncCounter(metrics.ConsistentQueryPerShard) |
There was a problem hiding this comment.
should this have been removed? kinda seems like this is to intentionally dual-emit for more efficient total metrics.
There was a problem hiding this comment.
Yeah this should be removed. I was thinking of making it consistent with something else but it's not needed for this particular operation since it's technically not a persistence operation.
allenchen2244
approved these changes
Mar 30, 2023
davidporter-id-au
added a commit
that referenced
this pull request
Apr 19, 2023
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
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.
What changed?
Add shardid tag to log
Add shardid tag to shardMetricScope
Increment counter per shard
Why?
Hot shard detection
How did you test it?
tested in staging
Potential risks
Release notes
Documentation Changes