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

Adjust default value of -querier.streaming-chunks-per-ingester-buffer-size #5203

Merged
merged 2 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Grafana Mimir

* [CHANGE] Store-gateway: skip verifying index header integrity upon loading. To enable verification set `blocks_storage.bucket_store.index_header.verify_on_load: true`.
* [CHANGE] Querier: change the default value of the experimental `-querier.streaming-chunks-per-ingester-buffer-size` flag to 128. #5203
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code says 256 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦 Good catch, fixed in #5210

* [ENHANCEMENT] Cardinality API: When zone aware replication is enabled, the label values cardinality API can now tolerate single zone failure #5178
* [ENHANCEMENT] Distributor: optimize sending requests to ingesters when incoming requests don't need to be modified. For now this feature can be disabled by setting `-timeseries-unmarshal-caching-optimization-enabled=false`. #5137

Expand Down
2 changes: 1 addition & 1 deletion cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -1657,7 +1657,7 @@
"required": false,
"desc": "Number of series to buffer per ingester when streaming chunks from ingesters.",
"fieldValue": null,
"fieldDefaultValue": 512,
"fieldDefaultValue": 256,
"fieldFlag": "querier.streaming-chunks-per-ingester-buffer-size",
"fieldType": "int",
"fieldCategory": "experimental"
Expand Down
2 changes: 1 addition & 1 deletion cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,7 @@ Usage of ./cmd/mimir/mimir:
-querier.store-gateway-client.tls-server-name string
Override the expected name on the server certificate.
-querier.streaming-chunks-per-ingester-buffer-size uint
[experimental] Number of series to buffer per ingester when streaming chunks from ingesters. (default 512)
[experimental] Number of series to buffer per ingester when streaming chunks from ingesters. (default 256)
-querier.timeout duration
The timeout for a query. This config option should be set on query-frontend too when query sharding is enabled. This also applies to queries evaluated by the ruler (internally or remotely). (default 2m0s)
-query-frontend.align-queries-with-step
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ store_gateway_client:
# (experimental) Number of series to buffer per ingester when streaming chunks
# from ingesters.
# CLI flag: -querier.streaming-chunks-per-ingester-buffer-size
[streaming_chunks_per_ingester_series_buffer_size: <int> | default = 512]
[streaming_chunks_per_ingester_series_buffer_size: <int> | default = 256]

# The number of workers running in each querier process. This setting limits the
# maximum number of concurrent queries in each querier.
Expand Down
17 changes: 3 additions & 14 deletions pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,9 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.ShuffleShardingIngestersEnabled, "querier.shuffle-sharding-ingesters-enabled", true, fmt.Sprintf("Fetch in-memory series from the minimum set of required ingesters, selecting only ingesters which may have received series since -%s. If this setting is false or -%s is '0', queriers always query all ingesters (ingesters shuffle sharding on read path is disabled).", validation.QueryIngestersWithinFlag, validation.QueryIngestersWithinFlag))
f.BoolVar(&cfg.PreferStreamingChunks, "querier.prefer-streaming-chunks", false, "Request ingesters stream chunks. Ingesters will only respond with a stream of chunks if the target ingester supports this, and this preference will be ignored by ingesters that do not support this.")

// Why 512 series / ingester?
//
// Assuming the worst case scenario of loading a full 13 hours of chunks for a series from an ingester, a 15s scrape
// interval, 120 samples per chunk with average chunk size of 300 B, 1024 series would consume ~8 MB (26 chunks needed
// per series to cover 13 hours, so 7.8 KB needed per series).
//
// Note that this will not hold true for native histograms, which can be substantially larger.
//
// We have up to two active batches per ingester in SeriesChunksStreamReader (one in the channel and one already
// received from the channel), so we use 1024/2=512 series per buffer per ingester.
//
// 512 series / ingester was also a good balance between the CPU overhead of managing a batch of series with the memory
// cost of keeping it in memory in our testing.
f.Uint64Var(&cfg.StreamingChunksPerIngesterSeriesBufferSize, "querier.streaming-chunks-per-ingester-buffer-size", 512, "Number of series to buffer per ingester when streaming chunks from ingesters.")
// Why 256 series / ingester?
// Based on our testing, 256 series / ingester was a good balance between memory consumption and the CPU overhead of managing a batch of series.
f.Uint64Var(&cfg.StreamingChunksPerIngesterSeriesBufferSize, "querier.streaming-chunks-per-ingester-buffer-size", 256, "Number of series to buffer per ingester when streaming chunks from ingesters.")

// The querier.query-ingesters-within flag has been moved to the limits.go file
// We still need to set a default value for cfg.QueryIngestersWithin since we need to keep supporting the querier yaml field until Mimir 2.11.0
Expand Down