Skip to content

Conversation

@pracucci
Copy link
Collaborator

@pracucci pracucci commented Jan 8, 2026

What this PR does

This PR is a follow up of #13924. In this PR I'm doing a micro optimization to parallelStorageShards for the case there's only 1 shard. The impact of this optimization is minimal, but I think doesn't make the code harder to follow, so it may be worth to keep it.

My local benchmarks are not super stable, but here you can get an idea of the impact (minimal). Keep in mind PusherConsumer_ParallelPusher_MultiTenant uses a mocked backend. In the real world, where metrics are actually ingested in TSDB, the impact is much smaller:

goos: darwin
goarch: arm64
pkg: github.com/grafana/mimir/pkg/storage/ingest
cpu: Apple M3 Pro
                                                                       │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-before.txt │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-after.txt │
                                                                       │                            sec/op                             │                sec/op                  vs base               │
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1-11                                                          8.421µ ±  5%                            8.862µ ±  2%   +5.24% (p=0.009 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=10-11                                                         8.578µ ± 81%                            8.971µ ± 46%        ~ (p=0.240 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=100-11                                                        8.394µ ±  3%                            9.005µ ±  1%   +7.28% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1000-11                                                       8.308µ ±  2%                            8.998µ ±  0%   +8.31% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1-11                                                         21.22µ ±  4%                            21.35µ ±  2%        ~ (p=0.937 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=10-11                                                        43.02µ ±  6%                            40.24µ ±  0%   -6.46% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=100-11                                                       43.24µ ±  1%                            40.50µ ±  0%   -6.33% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1000-11                                                      43.37µ ±  2%                            40.73µ ±  1%   -6.10% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1-11                                                        126.3µ ±  0%                            120.2µ ±  1%   -4.85% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=10-11                                                       137.7µ ±  0%                            122.0µ ±  3%  -11.40% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=100-11                                                      304.7µ ±  3%                            275.2µ ±  0%   -9.69% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1000-11                                                     303.5µ ±  2%                            275.1µ ±  0%   -9.35% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1-11                                                       1.142m ±  1%                            1.124m ±  1%   -1.58% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=10-11                                                      1.051m ±  1%                            1.115m ±  0%   +6.10% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=100-11                                                     1.264m ±  1%                            1.301m ±  0%   +2.91% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1000-11                                                    3.060m ±  3%                            2.996m ±  4%        ~ (p=0.240 n=6)
geomean                                                                                                                   97.29µ                                  95.70µ         -1.63%

                                                                       │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-before.txt │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-after.txt │
                                                                       │                             B/op                              │                  B/op                   vs base              │
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1-11                                                          17.40Ki ± 0%                             17.44Ki ± 0%  +0.19% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=10-11                                                         17.40Ki ± 0%                             17.43Ki ± 0%  +0.19% (p=0.035 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=100-11                                                        17.39Ki ± 0%                             17.43Ki ± 0%  +0.21% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1000-11                                                       17.39Ki ± 0%                             17.43Ki ± 0%  +0.20% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1-11                                                         25.15Ki ± 0%                             25.21Ki ± 0%  +0.25% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=10-11                                                        78.70Ki ± 0%                             78.70Ki ± 0%       ~ (p=0.223 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=100-11                                                       78.70Ki ± 0%                             78.70Ki ± 0%  +0.01% (p=0.037 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1000-11                                                      78.69Ki ± 0%                             78.70Ki ± 0%       ~ (p=0.058 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1-11                                                        120.9Ki ± 0%                             121.7Ki ± 0%  +0.68% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=10-11                                                       158.2Ki ± 0%                             159.4Ki ± 0%  +0.80% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=100-11                                                      707.4Ki ± 0%                             708.3Ki ± 0%  +0.13% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1000-11                                                     707.3Ki ± 0%                             708.3Ki ± 0%  +0.14% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1-11                                                       985.3Ki ± 0%                             978.7Ki ± 1%  -0.66% (p=0.041 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=10-11                                                      1.064Mi ± 0%                             1.091Mi ± 0%  +2.54% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=100-11                                                     1.479Mi ± 0%                             1.547Mi ± 0%  +4.62% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1000-11                                                    6.935Mi ± 0%                             6.947Mi ± 0%  +0.17% (p=0.002 n=6)
geomean                                                                                                                   156.1Ki                                  157.0Ki       +0.59%

                                                                       │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-before.txt │ BenchmarkPusherConsumer_ParallelPusher_MultiTenant-after.txt │
                                                                       │                           allocs/op                           │              allocs/op                vs base                │
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1-11                                                            48.00 ± 0%                             48.00 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=10-11                                                           48.00 ± 0%                             48.00 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=100-11                                                          48.00 ± 0%                             48.00 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=1,tenants=1000-11                                                         48.00 ± 0%                             48.00 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1-11                                                           166.0 ± 0%                             166.0 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=10-11                                                          291.0 ± 0%                             291.0 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=100-11                                                         291.0 ± 0%                             291.0 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=10,tenants=1000-11                                                        291.0 ± 0%                             291.0 ± 0%       ~ (p=1.000 n=6) ¹
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1-11                                                         1.364k ± 0%                            1.366k ± 0%  +0.15% (p=0.004 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=10-11                                                        1.466k ± 0%                            1.467k ± 0%  +0.07% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=100-11                                                       2.664k ± 0%                            2.664k ± 0%       ~ (p=0.545 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=100,tenants=1000-11                                                      2.663k ± 0%                            2.664k ± 0%       ~ (p=0.182 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1-11                                                        13.28k ± 0%                            13.27k ± 0%       ~ (p=0.093 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=10-11                                                       13.38k ± 0%                            13.41k ± 0%  +0.19% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=100-11                                                      14.36k ± 0%                            14.37k ± 0%  +0.08% (p=0.002 n=6)
PusherConsumer_ParallelPusher_MultiTenant/records=1000,tenants=1000-11                                                     26.23k ± 0%                            26.22k ± 0%  -0.07% (p=0.002 n=6)
geomean                                                                                                                     784.6                                  784.8       +0.02%
¹ all samples are equal

No changelog because I will add one at the end of this work, once -ingest-storage.kafka.ingestion-concurrency-sequential-pusher-enabled will be removed and the new behaviour will be the default. With the default config parallelStorageShards is never used with only 1 shard (you need to set -ingest-storage.kafka.ingestion-concurrency-sequential-pusher-enabled=false to use parallelStorageShards even when there's only 1 shard).

Which issue(s) this PR fixes or relates to

N/A

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

Note

Optimizes shard routing when numShards == 1 and refines performance benchmarks.

  • In parallelStorageShards.PushToStorageAndReleaseRequest, bypasses label hashing/modulo and always targets shard 0 when only one shard; metadata routing now skips round-robin and random start when single-shard.
  • BenchmarkPusherConsumer_ParallelPusher_MultiTenant: replaces single-parameter sweep with nested records×tenants cases, prebuilds records per case, and sets IngestionConcurrencySequentialPusherEnabled=false to exercise parallel pusher consistently.

Written by Cursor Bugbot for commit 49af667. This will update automatically on new commits. Configure here.

@pracucci pracucci added the changelog-not-needed PRs that don't need a CHANGELOG.md entry label Jan 8, 2026
@pracucci pracucci requested a review from a team as a code owner January 8, 2026 09:28
@pracucci pracucci changed the title Optimise parallelStorageShards for the case there's only 1 shard Micro optimization to parallelStorageShards for the case there's only 1 shard Jan 8, 2026
Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🔥 Code-wise looks good to me. The CI isn't happy, though

@pracucci
Copy link
Collaborator Author

pracucci commented Jan 8, 2026

The CI isn't happy, though

It was an unrelated flaky test.

@pracucci pracucci merged commit 6f0aa07 into main Jan 8, 2026
64 of 65 checks passed
@pracucci pracucci deleted the optimize-parallel-shards-with-1-shard branch January 8, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-not-needed PRs that don't need a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants