-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
refactor(blooms): Apply retention in planner #13484
Conversation
b63aeb7
to
3d1889c
Compare
docs/sources/shared/configuration.md
Outdated
|
||
# Max lookback days for retention. | ||
# CLI flag: -bloom-build.planner.retention.max-lookback-days | ||
[max_lookback_days: <int> | default = 365] |
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.
Can we hide this setting from the docs? Usually you don't need to set this, only in very special edge cases.
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.
Because it can cause confusion and could be mixed up with retention period of blooms, which it isn't right?
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 don't have an strong opinion on this. I'll go ahead and hide it.
What this PR does / why we need it:
This PR implements retention in the bloom planner.
Notes to reviewers
The retention manager is copied from the bloom compactor, the only difference is that we have removed the logic to check if the compactor owns the retention by looking at the ring.
Therefore I'd focus on reviewing:
Tested out in dev by setting the
retention_period: 10d
for tenant145265
.Before retention: Both tenants have blocks up to the earliest day built.
After retention: Tenant 29 still has data for older days but
145265
don't.Here are the retention logs:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR