Deprecate -blocks-storage.bucket-store.bucket-index.enabled configuration parameter #5051
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
pstibrany
approved these changes
May 23, 2023
56quarters
reviewed
May 23, 2023
| UpdateOnErrorInterval time.Duration `yaml:"update_on_error_interval" category:"advanced"` | ||
| IdleTimeout time.Duration `yaml:"idle_timeout" category:"advanced"` | ||
| MaxStalePeriod time.Duration `yaml:"max_stale_period" category:"advanced"` | ||
| } | ||
|
|
||
| func (cfg *BucketIndexConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) { | ||
| f.BoolVar(&cfg.Enabled, prefix+"enabled", true, "If enabled, queriers and store-gateways discover blocks by reading a bucket index (created and updated by the compactor) instead of periodically scanning the bucket.") | ||
| f.BoolVar(&cfg.DeprecatedEnabled, prefix+"enabled", true, "If enabled, queriers and store-gateways discover blocks by reading a bucket index (created and updated by the compactor) instead of periodically scanning the bucket.") |
There was a problem hiding this comment.
Since we no longer care about this setting and will always treat it as true in the future, should we use flagext.DeprecatedFlag and remove all the logic that depends on it?
There was a problem hiding this comment.
Based on our deprecation policy, whenever possible we should keep the configuration option working while deprecated. For this reason, I haven't used flagext.DeprecatedFlag and not removed the logic yet (which will be done after 2.10 is released).
…tion parameter Signed-off-by: Marco Pracucci <marco@pracucci.com>
pracucci
force-pushed
the
make-bucket-index-mandatory
branch
from
ddb6067
to
de85d40
Compare
leizor
added a commit
that referenced
this pull request
Nov 16, 2023
This configuration parameter has been removed. Mimir is running with bucket index enabled by default since 2.0 and it is now not possible to disable it. See #5051 for more details.
leizor
added a commit
that referenced
this pull request
Nov 17, 2023
* Remove -querier.query-ingesters-within config The `-querier.query-ingesters-within` config has been moved from a global config to a per-tenant limit config. See #4287 for more details. * Remove -querier.iterators and -querier.batch-iterators The `-querier.iterators` and `-querier.batch-iterators` configuration parameters have been removed. See #5114 for more details. * Remove deprecated bucket store flags The following deprecated flags are removed: - `-blocks-storage.bucket-store.max-chunk-pool-bytes` - `-blocks-storage.bucket-store.chunk-pool-min-bucket-size-bytes` - `-blocks-storage.bucket-store.chunk-pool-max-bucket-size-bytes` See #4996 for more details. * Remove -blocks-storage.bucket-store.bucket-index.enabled config This configuration parameter has been removed. Mimir is running with bucket index enabled by default since 2.0 and it is now not possible to disable it. See #5051 for more details. * Update CHANGELOG.md
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
We, as Mimir maintainers, give the bucket index for granted but there's actually a configuration parameter to disable it. The bucket index is enabled by default since Mimir 2.0 and I'm not aware of any reason why anyone may want to disable it. Since keeping the support to run Mimir without bucket index involves some extra code in the querier and store-gateway, I propose to deprecate the configuration parameter and then remove it in Mimir 2.11.
Note: we'll have to change some integration tests to start using the bucket index everywhere, but we have to adapt them before 2.11.
Which issue(s) this PR fixes or relates to
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]