fix(ingest-limits): Use stripe locking for metadata #17150

periklis · 2025-04-11T21:10:33Z

What this PR does / why we need it:
This pull request implements a stripe-lock approach to read/write the internal ingest-limits stream metadata structure. The stripes are distributed over the total amount of partitions (e.g. default is 64). This allows updating and reading the structure with less lock contention.

                                                                                    │    old.txt    │                new.txt                │
                                                                                   │    sec/op     │    sec/op     vs base                 │
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create-14               180.8n ± ∞ ¹   133.7n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update-14               190.6n ± ∞ ¹   135.5n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create_parallel-14      320.4n ± ∞ ¹   232.3n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update_parallel-14      347.7n ± ∞ ¹   256.7n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create-14               189.1n ± ∞ ¹   124.0n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update-14               195.1n ± ∞ ¹   125.9n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create_parallel-14      341.6n ± ∞ ¹   125.1n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update_parallel-14      356.0n ± ∞ ¹   150.3n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create-14               206.9n ± ∞ ¹   134.7n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update-14               208.6n ± ∞ ¹   136.6n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create_parallel-14     336.20n ± ∞ ¹   82.15n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update_parallel-14     343.20n ± ∞ ¹   91.31n ± ∞ ¹        ~ (p=1.000 n=1) ²
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create-14              211.4n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update-14              212.2n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create_parallel-14     327.2n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update_parallel-14     332.0n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create-14                            134.9n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update-14                            139.1n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create_parallel-14                   60.90n ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update_parallel-14                   65.67n ± ∞ ¹
geomean                                                                               259.3n         124.4n        -46.83%               ³
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05
³ benchmark set differs from baseline; geomeans may not be comparable

                                                                                   │   old.txt   │               new.txt                │
                                                                                   │    B/op     │     B/op      vs base                │
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create-14              56.00 ± ∞ ¹    56.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update-14              72.00 ± ∞ ¹    71.00 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create_parallel-14     90.00 ± ∞ ¹    90.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update_parallel-14     157.0 ± ∞ ¹    141.0 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create-14              56.00 ± ∞ ¹    56.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update-14              72.00 ± ∞ ¹    72.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create_parallel-14     89.00 ± ∞ ¹    90.00 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update_parallel-14     137.0 ± ∞ ¹    138.0 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create-14              56.00 ± ∞ ¹    56.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update-14              72.00 ± ∞ ¹    72.00 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create_parallel-14     73.00 ± ∞ ¹   116.00 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update_parallel-14     113.0 ± ∞ ¹    159.0 ± ∞ ¹       ~ (p=1.000 n=1) ³
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create-14             56.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update-14             72.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create_parallel-14    62.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update_parallel-14    92.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create-14                           56.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update-14                           71.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create_parallel-14                  95.00 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update_parallel-14                  138.0 ± ∞ ¹
geomean                                                                              78.65          86.48        +6.02%               ⁴
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ need >= 4 samples to detect a difference at alpha level 0.05
⁴ benchmark set differs from baseline; geomeans may not be comparable

                                                                                   │   old.txt   │               new.txt               │
                                                                                   │  allocs/op  │  allocs/op   vs base                │
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_create_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/4_partitions_small_streams_single_tenant_update_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_create_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/8_partitions_medium_streams_multi_tenant_update_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update-14              2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_create_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/16_partitions_large_streams_multi_tenant_update_parallel-14     2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=1) ²
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create-14             2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update-14             2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_create_parallel-14    2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant_update_parallel-14    2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create-14                          2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update-14                          2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__create_parallel-14                 2.000 ± ∞ ¹
StreamMetadata_Store/32_partitions_xlarge_streams_multi_tenant__update_parallel-14                 2.000 ± ∞ ¹
geomean                                                                              2.000         2.000        +0.00%               ³
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ benchmark set differs from baseline; geomeans may not be comparable

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
Title matches the required conventional commits format, see here
- Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

pkg/limits/ingest_limits.go

grobinson-grafana

In general looks good! I would love to see a better abstraction around the stripes and the lock acquisition though. I think if you do this you'll find the code easier to reason about and also easier to write tests. You could consider using https://pkg.go.dev/iter for some of the operations.

pkg/limits/ingest_limits.go

pkg/limits/ingest_limits_test.go

pkg/limits/frontend/ring_test.go

pkg/limits/http.go

pkg/limits/ingest_limits.go

pkg/limits/stream_metadata.go

pkg/limits/ingest_limits.go

pkg/limits/ingest_limits_test.go

grobinson-grafana · 2025-04-24T16:36:56Z

pkg/limits/stream_metadata.go

+
+// AllFunc is a function that is called for each stream in the metadata.
+// It is used to count per tenant active streams.
+// Note: The collect function should not modify the stream metadata.


Is modification possible as we pass a copy of Stream? I suppose you could change the values in RateBuckets?

You are right, this is more of a note on how to use the interface not an enforced rule as of now.

pkg/limits/stream_metadata.go

grobinson-grafana · 2025-04-24T16:47:03Z

pkg/limits/stream_metadata_test.go

+	require.ElementsMatch(t, expected, actual)
+}
+
+func TestStreamMetadata_Usage_Concurrent(t *testing.T) {


Do we need to run these with -race?

Just FYI, we don't run our tests with -race:

Run gotestsum -- -covermode=atomic -coverprofile=coverage.txt -p=4 ./pkg/limits/...

periklis self-assigned this Apr 11, 2025

pull-request-size bot added the size/XXL label Apr 11, 2025

periklis force-pushed the ingest-limits-stripe-locks branch from 7222994 to c6b8c9c Compare April 15, 2025 13:47

periklis marked this pull request as ready for review April 15, 2025 18:16

periklis requested a review from a team as a code owner April 15, 2025 18:16

periklis force-pushed the ingest-limits-stripe-locks branch from c6b8c9c to 805f573 Compare April 15, 2025 18:17

periklis commented Apr 15, 2025

View reviewed changes

pkg/limits/ingest_limits.go Outdated Show resolved Hide resolved

grobinson-grafana reviewed Apr 16, 2025

View reviewed changes

pkg/limits/ingest_limits_test.go Outdated Show resolved Hide resolved

pkg/limits/ingest_limits_test.go Outdated Show resolved Hide resolved

periklis force-pushed the ingest-limits-stripe-locks branch from 6cb1f32 to 8a03e24 Compare April 22, 2025 09:02

grobinson-grafana reviewed Apr 22, 2025

View reviewed changes

periklis added 11 commits April 23, 2025 08:41

fix(ingest-limits): Use stripe locking for metadata

c639e85

Handle context cancelation

2218c8b

Eliminate size field

fb58c7a

Optimize for continguous memory usage of stream medata

a098a10

Refactor stream metadata stripes

6967b6a

Refactor interface and benchmark

9102236

Refine benchmark setup

3b1440e

Fix lint issues

ad52e19

Fix format issues

4a67514

Remove evivtion on store path

9ae3d81

Switch to per tenant stripes

4bf6f5d

periklis force-pushed the ingest-limits-stripe-locks branch from 266b8c1 to 4bf6f5d Compare April 23, 2025 06:43

periklis added 2 commits April 23, 2025 09:51

Remove obsolete onLost handler for cooperative consumer group

afa04eb

Fix linter

2552a3b

grobinson-grafana reviewed Apr 24, 2025

View reviewed changes

Address code review suggestions

e143a59

grobinson-grafana enabled auto-merge (squash) April 25, 2025 08:37

grobinson-grafana approved these changes Apr 25, 2025

View reviewed changes

grobinson-grafana disabled auto-merge April 25, 2025 08:41

Fix linter

ee1fb8e

periklis enabled auto-merge (squash) April 25, 2025 09:29

periklis merged commit c32e479 into main Apr 25, 2025
61 checks passed

periklis deleted the ingest-limits-stripe-locks branch April 25, 2025 09:36

This was referenced Jun 24, 2025

chore(k260): release 3.5.0 #18210

Open

chore(k261): release 3.5.0 #18278

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ingest-limits): Use stripe locking for metadata #17150

fix(ingest-limits): Use stripe locking for metadata #17150

Uh oh!

periklis commented Apr 11, 2025 •

edited

Loading

Uh oh!

grobinson-grafana left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

grobinson-grafana Apr 24, 2025

periklis Apr 25, 2025

Uh oh!

Uh oh!

grobinson-grafana Apr 24, 2025

grobinson-grafana Apr 25, 2025

Uh oh!

fix(ingest-limits): Use stripe locking for metadata #17150

fix(ingest-limits): Use stripe locking for metadata #17150

Uh oh!

Conversation

periklis commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grobinson-grafana left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

grobinson-grafana Apr 24, 2025

Choose a reason for hiding this comment

periklis Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

grobinson-grafana Apr 24, 2025

Choose a reason for hiding this comment

grobinson-grafana Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

periklis commented Apr 11, 2025 •

edited

Loading