Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add live setting for verbose index metrics #608

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

aprudhomme
Copy link
Contributor

Add and index live setting toggle to control collection of more detailed index metrics, defaulting to false.

When enabled, the following metrics are collected and published:

  • nrt_search_response_size_bytes - summary of response protobuf serialized size
  • nrt_search_response_total_hits - summary of response total hits
  • nrt_search_stage_latency_ms - summary of latency for each search stage (recall/highlight/fetch/rescorers/etc)
  • nrt_search_timeout_count - counter of responses with a search recall timeout
  • nrt_search_terminated_early_count - counter of responses that terminated early for recall count

@aprudhomme aprudhomme requested a review from vim345 November 4, 2023 00:50
Copy link
Contributor

@vim345 vim345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I just had a question if enabling this will use more resources on the cluster and if we should let developers know about that.

@@ -1089,6 +1089,8 @@ message IndexLiveSettings {
google.protobuf.Int32Value defaultTerminateAfter = 13;
// Merge precopy would be stopped after this time, or 0 for no checks, default: 0
google.protobuf.UInt64Value maxMergePreCopyDurationSec = 14;
// Collect and publish additional index metrics, default: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the commend indicate this may introduce performance penalty if set to true?

@aprudhomme aprudhomme merged commit 92ece91 into Yelp:master Nov 13, 2023
1 check passed
sarthakn7 added a commit that referenced this pull request Mar 1, 2024
* Fix node resolver for empty/missing files (#587)

* Autogenerated JaCoCo coverage badge

* Bump to v0.26.1 (#588)

* Autogenerated JaCoCo coverage badge

* parallel explain for nrtsearch (#589)

* parallel explain for nrtsearch

* fix nit

* add explain for multi segment parallel

* refactor isExplain in different contexts

* make NRTSearch empty boolean query result constant score 1 (#592)

* make NRTSearch empty boolean query result constant score 1

* fix tests

* soft exception for fvh failures (#593)

* Support unit in SortType (#590)

create sort context and support unit in lat_lon distance sort

* Autogenerated JaCoCo coverage badge

* Upgrade dependencies for snyk check (#596)

* Fix example-plugin test (#598)

* Add page fault metrics (#599)

* Add additional merge metrics (#607)

* Add live setting for verbose index metrics (#608)

* Add live setting for verbose index metrics

* Address review comment

* Autogenerated JaCoCo coverage badge

* Bump to v0.29.0 (#609)

* Add live index settings override from config file (#610)

* Autogenerated JaCoCo coverage badge

* Add ability to update local index live settings (#611)

* Add deadline cancellation for indexing (#612)

* Add geo polygon query type (#613)

* Add geo polygon query type

* Detect closed polygons, update docs

* Autogenerated JaCoCo coverage badge

* Bump to v0.30.0 (#615)

* Testing readthedocs web hooks (#616)

* Add a bare minimum readthedocs config file (#617)

* log more info when fvh failed (#618)

 log more info when fvh failed

* Avoid calling query.toString() (#619)

* add sts for web identity auth (#620)

add sts for web identity auth

* Add search diagnostics to deadline exceeded exceptions (#621)

* Add search diagnostics to deadline exceeded exceptions
---------

Co-authored-by: swekannan <[email protected]>

* Fixes and spotless apply

* Updated grpc-gateway

---------

Co-authored-by: Andrew Prudhomme <[email protected]>
Co-authored-by: github_actions <runner@fv-az248-700.3twvhzoricxu3figbgb44chhog.cx.internal.cloudapp.net>
Co-authored-by: github_actions <runner@fv-az887-622.kyhrjabtieueri5x0cgfkhfc1a.bx.internal.cloudapp.net>
Co-authored-by: Tao Yu <[email protected]>
Co-authored-by: waziqi89 <[email protected]>
Co-authored-by: github_actions <runner@fv-az248-372.3twvhzoricxu3figbgb44chhog.cx.internal.cloudapp.net>
Co-authored-by: github_actions <runner@fv-az574-753.esnmgxn14wlejbqjnvhsbsbtxa.dx.internal.cloudapp.net>
Co-authored-by: github_actions <runner@fv-az1272-720.grsihaubamwerhiryzjrxtypna.phxx.internal.cloudapp.net>
Co-authored-by: github_actions <runner@fv-az736-601.alpbqrzxv30uzkvtn2qktnuusd.cx.internal.cloudapp.net>
Co-authored-by: Mohammad Mohtasham <[email protected]>
Co-authored-by: swekannan <[email protected]>
Co-authored-by: swekannan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants