-
Notifications
You must be signed in to change notification settings - Fork 528
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
feat: limit tags and tag values search #4320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd also like to get your thoughts on a different kind of parameter. perhaps it could be added here as well?
what about a parameter that stopped searching early if N strings were added to the combiner and none were new? so the combiner would keep a running count of the number of added strings since a new one was found and stop searching once we crossed threshold.
consider /api/v2/search/tag/resource.cluster/values?q=<some very expensive query>
. there are likely very few values for resource.cluster
but we still spend a lot of resources to exhaustively evaluate <some very expensive query>
. a parameter that allowed the ingester to just stop searching after N found values and nothing new could be a powerful way to dynamically circuit break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating the docs! I have a few minor suggestions to correct missing periods.
The more I think about this idea the more I want it:
The change here and the current max_bytes_per_tag_values_query protect us when a tag value query finds A LOT of matches. We currently have no protection for an expensive tag values query that find a very low number of values. |
Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
I want to add some logging to control how this behaves |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a changelog, but otherwise good. great work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
What this PR does:
This PR introduces two new optional parameters to enhance the control of Tags and Tag Values lookups:
limit
:Specifies the maximum number of items to retrieve. For the
api/v2/tags
endpoint, the search stops if any scope reaches this limit.maxStaleValues
:Restricts the search for tag values. If the number of stale (already known) values meets or exceeds this limit, the search is halted. This is useful when we have a small set of tag values.
Which issue(s) this PR fixes:
Fixes #4288
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]