querier,ingester: support for limit parameter in the finding series endpoint #10620
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
narqo
force-pushed
the
vldmr/api-series-limit
branch
2 times, most recently
from
e0c1de1 to
381c2a6
Compare
dimitarvdimitrov
approved these changes
Feb 10, 2025
pracucci
approved these changes
Feb 11, 2025
pkg/distributor/distributor.go
Outdated
| for _, m := range metrics { | ||
| if len(result) >= resultCap { |
There was a problem hiding this comment.
Why not limit few lines above when we populate metrics to deduplicate series? We could just stop once len(metrics) reached the limit.
Similarly, I'm wondering why we don't apply the limiter in the same for loop above when we populate metrics. Do you see a good reason to not do it there?
There was a problem hiding this comment.
Do you see a good reason to not do it there?
I think, the only one is that, in the worst case, this will check every duplicate against the series limiter, calculating the hash of the series twice. I cannot tell if that better or worse, from applying this limit later.
Note I'll skip looking into this part. We can benchmark different options separately.
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Co-authored-by: Marco Pracucci <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
narqo
force-pushed
the
vldmr/api-series-limit
branch
from
df94c31 to
9a952b1
Compare
dimitarvdimitrov
changed the title
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 this PR does
This PR updates the
querierand theingesterservices, with a basic support for thelimitparameter in the/seriesAPI endpoint.Here the code only passes the limit to the downstream services, as-is. This is not ideal, because the limit of MM series in the distributor can overwhelm the ingesters. I want to focus on that in the follow-up PR to make the review simpler.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.