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 query results cache support for label names and label values APIs #5426

Merged
merged 5 commits into from
Jul 7, 2023

Conversation

pracucci
Copy link
Collaborator

@pracucci pracucci commented Jul 5, 2023

What this PR does

Similarly to #5212, in this PR I'm adding the support to cache label names and label values responses. See details at: #5395

Note to reviewers:

  • I manually tested it and looks working as intended.
  • I opted for a new configuration option to specify the labels query response cache TTL, instead of one in common with the cardinality API, because I think we can potentially cache a bit longer for cardinality APIs. Since these config options are still marked experimental (so we're not committing to them yet) I prefer to keep them separated while rolling out the feature.

Which issue(s) this PR fixes or relates to

Fixes #5395

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pracucci pracucci force-pushed the add-query-results-cache-support-for-label-apis branch from 18bcd0d to 70ba7f4 Compare July 6, 2023 10:42
@pracucci pracucci changed the title Add query results cache support for label apis Add query results cache support for label names and label values APIs Jul 6, 2023
@pracucci pracucci force-pushed the add-query-results-cache-support-for-label-apis branch from 70ba7f4 to 9c23199 Compare July 6, 2023 10:49
Signed-off-by: Marco Pracucci <[email protected]>
instantQueryPathSuffix = "/query"
cardinalityLabelNamesPathSuffix = "/cardinality/label_names"
cardinalityLabelValuesPathSuffix = "/cardinality/label_values"
queryRangePathSuffix = "/api/v1/query_range"
Copy link
Collaborator Author

@pracucci pracucci Jul 6, 2023

Choose a reason for hiding this comment

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

Note to reviwers: adding the /api/v1/ prefix is not expected to be a breaking change because these are the prefixes mapped in pkg/api/api.go.

Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci marked this pull request as ready for review July 6, 2023 11:01
@pracucci pracucci requested review from a team as code owners July 6, 2023 11:01
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM with a few questions

pkg/frontend/querymiddleware/labels_query_cache.go Outdated Show resolved Hide resolved
pkg/frontend/querymiddleware/labels_query_cache.go Outdated Show resolved Hide resolved
pkg/frontend/querymiddleware/labels_query_cache.go Outdated Show resolved Hide resolved
@pracucci pracucci merged commit 0f6bd56 into main Jul 7, 2023
28 checks passed
@pracucci pracucci deleted the add-query-results-cache-support-for-label-apis branch July 7, 2023 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: cache label names/values API response in the query results cache
3 participants