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

Make results cache TTL configurable and settable per tenant #4385

Merged
merged 5 commits into from
Mar 9, 2023

Conversation

pstibrany
Copy link
Member

@pstibrany pstibrany commented Mar 6, 2023

What this PR does

This PR replaces hard-coded 7 days (and 10 minutes for queries within OOO window) TTL for results cache, and makes these values configurable per tenant. This allows us to use lower values for users who want to use backfill feature.

Which issue(s) this PR fixes or relates to

Fixes (partially) #4383

Checklist

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

@pstibrany pstibrany requested review from a team as code owners March 6, 2023 09:38
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM, just a few comments.

pkg/util/validation/limits.go Outdated Show resolved Hide resolved
pkg/frontend/querymiddleware/split_and_cache.go Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

This PR does what it says, so I'm approving it. However, I have a question about the end-to-end experience to actually solve #4383 with this PR, so I'm going to follow up with a question there.

@pracucci
Copy link
Collaborator

pracucci commented Mar 7, 2023

This PR partially addresses #3832 too.

Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing my feedback.

@pstibrany pstibrany enabled auto-merge (squash) March 9, 2023 08:12
@pstibrany pstibrany disabled auto-merge March 9, 2023 08:13
@pstibrany
Copy link
Member Author

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
@pstibrany pstibrany force-pushed the make-results-cache-ttl-configurable branch from 2018fb5 to a2940a0 Compare March 9, 2023 10:57
@pstibrany pstibrany enabled auto-merge (squash) March 9, 2023 11:01
@pstibrany pstibrany merged commit 8c6cb52 into main Mar 9, 2023
@pstibrany pstibrany deleted the make-results-cache-ttl-configurable branch March 9, 2023 11:15
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.

3 participants