-
Notifications
You must be signed in to change notification settings - Fork 529
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
Use BucketIndexBlocksFinder instead of BucketScanBlocksFinder #6779
Conversation
This is less straightforward than I expected; marking as a draft for now while I figure out the failing integration tests. |
3c37d40
to
c68fb19
Compare
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.
But I think we can do better. Can you also remove the alert and its runbook
mimir/operations/mimir-mixin/alerts/blocks.libsonnet
Lines 186 to 201 in 7d9b3d4
{ | |
// Alert if the querier is not successfully scanning the bucket. | |
alert: $.alertName('QuerierHasNotScanTheBucket'), | |
'for': '5m', | |
expr: ||| | |
(time() - cortex_querier_blocks_last_successful_scan_timestamp_seconds > 60 * 30) | |
and | |
cortex_querier_blocks_last_successful_scan_timestamp_seconds > 0 | |
|||, | |
labels: { | |
severity: 'critical', | |
}, | |
annotations: { | |
message: '%(product)s Querier %(alert_instance_variable)s in %(alert_aggregation_variables)s has not successfully scanned the bucket since {{ $value | humanizeDuration }}.' % $._config, | |
}, | |
}, |
After doing that you should probably run make build-mixin build-helm-tests
and commit the diff
The use of BucketScanBlocksFinder was supposed to be removed in #6673 but its counterpart BucketIndexBlocksFinder was accidentally removed instead. This reverses that.
93b0d6f
to
91c93ef
Compare
@dimitarvdimitrov Good call, done! |
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.
LGTM, thanks so much for doing this
Thanks for the reviews! |
* Use BucketIndexBlocksFinder instead of BucketScanBlocksFinder The use of BucketScanBlocksFinder was supposed to be removed in #6673 but its counterpart BucketIndexBlocksFinder was accidentally removed instead. This reverses that. * Remove BlocksFinderBucketScan * Update docs wrt. bucket scanning option * Remove alert and runbook for bucket scanning (cherry picked from commit ff8a70a)
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-6779-to-r267 origin/r267
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x ff8a70aee6d187113d0f2b18b75144a4bee5091d
# Push it to GitHub
git push --set-upstream origin backport-6779-to-r267
git switch main
# Remove the local backport branch
git branch -D backport-6779-to-r267 Then, create a pull request where the |
This didn't need to be backported to r267 (the changelog entry was added in #6808) |
What this PR does
The use of BucketScanBlocksFinder was supposed to be removed in #6673 but its counterpart BucketIndexBlocksFinder was accidentally removed instead.
This PR also removes references to the old bucket scan option from the docs.
This reverses that.
Which issue(s) this PR fixes or relates to
#6670
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.