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.

Sign up for GitHub

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

Change the default of ignore_deletion_mark_delay to 1h #892

Merged
merged 2 commits into from
Jan 26, 2022
Merged

Change the default of ignore_deletion_mark_delay to 1h #892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fd8f0b
Change the default of ignore_deletion_mark_delay to 1h
andyasp Jan 25, 2022
4ba930a
Remove reference to actual default value in docs
andyasp Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@
* `-ruler.sharding-strategy` option has been removed from ruler. Ruler now uses shuffle-sharding by default, but respects `ruler_tenant_shard_size`, which defaults to 0 (ie. use all rulers for tenant). #889
* `-store-gateway.sharding-strategy` option has been removed store-gateways. Store-gateway now uses shuffle-sharding by default, but respects `store_gateway_tenant_shard_size` for tenant, and this value defaults to 0. #891
* [CHANGE] Server: `-server.http-listen-port` (yaml: `server.http_listen_port`) now defaults to `8080` (previously `80`). #871
* [CHANGE] Changed the default value of `blocks-storage.bucket-store.ignore-deletion-marks-delay` from 6h to 1h. #892
* [FEATURE] Query Frontend: Add `cortex_query_fetched_chunks_total` per-user counter to expose the number of chunks fetched as part of queries. This metric can be enabled with the `-frontend.query-stats-enabled` flag (or its respective YAML config option `query_stats_enabled`). #31
* [FEATURE] Query Frontend: Add experimental querysharding for the blocks storage (instant and range queries). You can now enable querysharding for blocks storage (`-store.engine=blocks`) by setting `-frontend.parallelize-shardable-queries` to `true`. The following additional config and exported metrics have been added. #79 #80 #100 #124 #140 #148 #150 #151 #153 #154 #155 #156 #157 #158 #159 #160 #163 #169 #172 #196 #205 #225 #226 #227 #228 #230 #235 #240 #239 #246 #244 #319 #330 #371 #385 #400 #458 #586 #630 #660 #707
* New config options:
Expand Down
2 changes: 1 addition & 1 deletion docs/sources/blocks-storage/production-tips.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Because of this, we would suggest to avoid querying non compacted blocks. In ord
3. Configure queriers `-querier.query-store-after` large enough to give compactor enough time to compact newly uploaded blocks (_see below_)
4. Configure queriers `-querier.query-ingesters-within` equal to `-querier.query-store-after` plus 5m (5 minutes is just a delta to query the boundary both from ingesters and queriers)
5. Configure ingesters `-blocks-storage.tsdb.retention-period` at least as `-querier.query-ingesters-within`
6. Lower `-blocks-storage.bucket-store.ignore-deletion-marks-delay` to 1h, otherwise non compacted blocks could be queried anyway, even if their compacted replacement is available
6. Ensure `-blocks-storage.bucket-store.ignore-deletion-marks-delay` is not too large (the default is suggested), otherwise non compacted blocks could be queried anyway, even if their compacted replacement is available

#### How to estimate `-querier.query-store-after`

Expand Down
5 changes: 2 additions & 3 deletions docs/sources/blocks-storage/querier.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,10 +578,9 @@ blocks_storage:
# while fetching blocks. The idea of ignore-deletion-marks-delay is to
# ignore blocks that are marked for deletion with some delay. This ensures
# store can still serve blocks that are meant to be deleted but do not have
# a replacement yet. Default is 6h, half of the default value for
# -compactor.deletion-delay.
# a replacement yet.
# CLI flag: -blocks-storage.bucket-store.ignore-deletion-marks-delay
[ignore_deletion_mark_delay: <duration> | default = 6h]
[ignore_deletion_mark_delay: <duration> | default = 1h]

bucket_index:
# True to enable querier and store-gateway to discover blocks in the
Expand Down
5 changes: 2 additions & 3 deletions docs/sources/blocks-storage/store-gateway.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,10 +649,9 @@ blocks_storage:
# while fetching blocks. The idea of ignore-deletion-marks-delay is to
# ignore blocks that are marked for deletion with some delay. This ensures
# store can still serve blocks that are meant to be deleted but do not have
# a replacement yet. Default is 6h, half of the default value for
# -compactor.deletion-delay.
# a replacement yet.
# CLI flag: -blocks-storage.bucket-store.ignore-deletion-marks-delay
[ignore_deletion_mark_delay: <duration> | default = 6h]
Copy link
Contributor

Choose a reason for hiding this comment

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

this is unrelated to your change:

I noticed that there is a discrepancy between the cli flag and the yaml property:

-blocks-storage.bucket-store.ignore-deletion-marks-delay
ignore_deletion_mark_delay

note the mark/marks. I think that's something which could confuse a user, so I'm wondering if it would be worth it to fix that before the launch of Mimir. Now it's easier to make breaking changes than after the launch. WDYT? cc @09jvilla

Copy link
Member

Choose a reason for hiding this comment

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

We have more fields like that, and now would be a great time to fix them all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we'll fix all of them, then we could enforce it in CI (eg. fail CI if any CLI flag doesn't reflect its YAML counterpart).

Copy link
Contributor

Choose a reason for hiding this comment

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

We have more fields like that, and now would be a great time to fix them all.

Do you remember any of them from the top of your head by chance?

I'll create an issue for it and then try to come up with a way to detect these discrepancies

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you could list of all them doing an hack in the doc generator code. There for each CLI flag we know the YAML config, so we you can compare them (with some substitutions, like _ and -) and print when they're different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like a good idea to me to make this change here and for all flags and agreed now is the time to do it.

Copy link
Contributor

@09jvilla 09jvilla Jan 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@replay can you create an issue for this consistent naming convention between CLI and yaml flags for tracking and add it to https://github.com/grafana/mimir-squad/issues/367 under the 'low hanging fruit we really want to get done if possible'? I think that list in this point is mostly becoming "we want this done by launch", but I'd say this isn't a launch blocker if we can't get it complete.

Copy link
Contributor

Choose a reason for hiding this comment

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

Created #901 and added it where you said

[ignore_deletion_mark_delay: <duration> | default = 1h]

bucket_index:
# True to enable querier and store-gateway to discover blocks in the
Expand Down
5 changes: 2 additions & 3 deletions docs/sources/configuration/config-file-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3047,10 +3047,9 @@ bucket_store:
# while fetching blocks. The idea of ignore-deletion-marks-delay is to ignore
# blocks that are marked for deletion with some delay. This ensures store can
# still serve blocks that are meant to be deleted but do not have a
# replacement yet. Default is 6h, half of the default value for
# -compactor.deletion-delay.
# replacement yet.
# CLI flag: -blocks-storage.bucket-store.ignore-deletion-marks-delay
[ignore_deletion_mark_delay: <duration> | default = 6h]
[ignore_deletion_mark_delay: <duration> | default = 1h]

bucket_index:
# True to enable querier and store-gateway to discover blocks in the storage
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-disable-chunk-streaming-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,6 @@ spec:
- args:
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1062,7 +1061,6 @@ spec:
- -api.response-compression-enabled=true
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1657,7 +1655,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-query-sharding-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,6 @@ spec:
- args:
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1066,7 +1065,6 @@ spec:
- -api.response-compression-enabled=true
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1661,7 +1659,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-shuffle-sharding-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,6 @@ spec:
- args:
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1067,7 +1066,6 @@ spec:
- -api.response-compression-enabled=true
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1667,7 +1665,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-storage-azure-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,6 @@ spec:
- -blocks-storage.azure.container-name=blocks-bucket
- -blocks-storage.backend=azure
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1066,7 +1065,6 @@ spec:
- -blocks-storage.azure.container-name=blocks-bucket
- -blocks-storage.backend=azure
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1671,7 +1669,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-storage-gcs-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,6 @@ spec:
- args:
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1061,7 +1060,6 @@ spec:
- -api.response-compression-enabled=true
- -blocks-storage.backend=gcs
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1656,7 +1654,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
3 changes: 0 additions & 3 deletions operations/mimir-tests/test-storage-s3-generated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,6 @@ spec:
- args:
- -blocks-storage.backend=s3
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1062,7 +1061,6 @@ spec:
- -api.response-compression-enabled=true
- -blocks-storage.backend=s3
- -blocks-storage.bucket-store.bucket-index.enabled=true
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.metadata-cache.backend=memcached
- -blocks-storage.bucket-store.metadata-cache.memcached.addresses=dnssrvnoa+memcached-metadata.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.metadata-cache.memcached.max-async-buffer-size=25000
Expand Down Expand Up @@ -1663,7 +1661,6 @@ spec:
- -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size=1048576
- -blocks-storage.bucket-store.chunks-cache.memcached.timeout=200ms
- -blocks-storage.bucket-store.ignore-blocks-within=10h
- -blocks-storage.bucket-store.ignore-deletion-marks-delay=1h
- -blocks-storage.bucket-store.index-cache.backend=memcached
- -blocks-storage.bucket-store.index-cache.memcached.addresses=dnssrvnoa+memcached-index-queries.default.svc.cluster.local:11211
- -blocks-storage.bucket-store.index-cache.memcached.max-async-buffer-size=25000
Expand Down
1 change: 0 additions & 1 deletion operations/mimir/config.libsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@
genericBlocksStorageConfig:: {},
queryBlocksStorageConfig:: {
'blocks-storage.bucket-store.sync-dir': '/data/tsdb',
'blocks-storage.bucket-store.ignore-deletion-marks-delay': '1h',

'store-gateway.sharding-enabled': true,
'store-gateway.sharding-ring.store': 'consul',
Expand Down
5 changes: 2 additions & 3 deletions pkg/storage/tsdb/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,8 @@ func (cfg *BucketStoreConfig) RegisterFlags(f *flag.FlagSet) {
f.IntVar(&cfg.BlockSyncConcurrency, "blocks-storage.bucket-store.block-sync-concurrency", 20, "Maximum number of concurrent blocks synching per tenant.")
f.IntVar(&cfg.MetaSyncConcurrency, "blocks-storage.bucket-store.meta-sync-concurrency", 20, "Number of Go routines to use when syncing block meta files from object storage per tenant.")
f.DurationVar(&cfg.ConsistencyDelay, "blocks-storage.bucket-store.consistency-delay", 0, "Minimum age of a block before it's being read. Set it to safe value (e.g 30m) if your object storage is eventually consistent. GCS and S3 are (roughly) strongly consistent.")
f.DurationVar(&cfg.IgnoreDeletionMarksDelay, "blocks-storage.bucket-store.ignore-deletion-marks-delay", time.Hour*6, "Duration after which the blocks marked for deletion will be filtered out while fetching blocks. "+
"The idea of ignore-deletion-marks-delay is to ignore blocks that are marked for deletion with some delay. This ensures store can still serve blocks that are meant to be deleted but do not have a replacement yet. "+
"Default is 6h, half of the default value for -compactor.deletion-delay.")
f.DurationVar(&cfg.IgnoreDeletionMarksDelay, "blocks-storage.bucket-store.ignore-deletion-marks-delay", time.Hour*1, "Duration after which the blocks marked for deletion will be filtered out while fetching blocks. "+
"The idea of ignore-deletion-marks-delay is to ignore blocks that are marked for deletion with some delay. This ensures store can still serve blocks that are meant to be deleted but do not have a replacement yet.")
f.DurationVar(&cfg.IgnoreBlocksWithin, "blocks-storage.bucket-store.ignore-blocks-within", 0, "Blocks with minimum time within this duration are ignored, and not loaded by store-gateway. Useful when used together with -querier.query-store-after to prevent loading young blocks, because there are usually many of them (depending on number of ingesters) and they are not yet compacted. Negative values or 0 disable the filter.")
f.IntVar(&cfg.PostingOffsetsInMemSampling, "blocks-storage.bucket-store.posting-offsets-in-mem-sampling", DefaultPostingOffsetInMemorySampling, "Controls what is the ratio of postings offsets that the store will hold in memory.")
f.BoolVar(&cfg.IndexHeaderLazyLoadingEnabled, "blocks-storage.bucket-store.index-header-lazy-loading-enabled", true, "If enabled, store-gateway will lazy load an index-header only once required by a query.")
Expand Down