From e9d038cc56258959fa8b3b3ace09a1a2a0b3f48f Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Mon, 9 Dec 2024 17:53:48 -0800 Subject: [PATCH] Fix race on the LabelSet Counter (#6409) * Configure labelset limits on the TestPushRace Signed-off-by: alanprot * Fix race Signed-off-by: alanprot * Changelog Signed-off-by: alanprot --------- Signed-off-by: alanprot Signed-off-by: Alan Protasio --- CHANGELOG.md | 9 +++++---- pkg/ingester/ingester_test.go | 16 ++++++++++++++-- pkg/ingester/user_state.go | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69fc2081ce..37ea06d80f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,12 +45,13 @@ * [ENHANCEMENT] Distributor: Expose `cortex_label_size_bytes` native histogram metric. #6372 * [ENHANCEMENT] Add new option `-server.grpc_server-num-stream-workers` to configure the number of worker goroutines that should be used to process incoming streams. #6386 * [ENHANCEMENT] Distributor: Return HTTP 5XX instead of HTTP 4XX when instance limits are hit. #6358 -* [ENHANCEMENT] Ingester: Add a new `-distributor.num-push-workers` flag to use a goroutine worker pool when sending data from distributor to ingesters. #6406 -* [ENHANCEMENT] Distributor: Create a goroutine worker pool to send data from distributors to ingesters. +* [ENHANCEMENT] Ingester: Make sure unregistered ingester joining the ring after WAL replay. #6277 * [BUGFIX] Runtime-config: Handle absolute file paths when working directory is not / #6224 * [BUGFIX] Ruler: Allow rule evaluation to complete during shutdown. #6326 -* [BUGFIX] Ring: update ring with new ip address when instance is lost, rejoins, but heartbeat is disabled #6271 -* [BUGFIX] ingester: Fix regression on usage of cortex_ingester_queried_chunks #6398 +* [BUGFIX] Ring: update ring with new ip address when instance is lost, rejoins, but heartbeat is disabled. #6271 +* [BUGFIX] Ingester: Fix regression on usage of cortex_ingester_queried_chunks. #6398 +* [ENHANCEMENT] Distributor: Add a new `-distributor.num-push-workers` flag to use a goroutine worker pool when sending data from distributor to ingesters. #6406 +* [BUGFIX] Ingester: Fix possible race condition when `active series per LabelSet` is configured. #6409 ## 1.18.1 2024-10-14 diff --git a/pkg/ingester/ingester_test.go b/pkg/ingester/ingester_test.go index ab2c9ad0b8..f64368877f 100644 --- a/pkg/ingester/ingester_test.go +++ b/pkg/ingester/ingester_test.go @@ -407,14 +407,26 @@ func TestIngesterPerLabelsetLimitExceeded(t *testing.T) { func TestPushRace(t *testing.T) { cfg := defaultIngesterTestConfig(t) + l := defaultLimitsTestConfig() cfg.LabelsStringInterningEnabled = true cfg.LifecyclerConfig.JoinAfter = 0 + + l.LimitsPerLabelSet = []validation.LimitsPerLabelSet{ + { + LabelSet: labels.FromMap(map[string]string{ + labels.MetricName: "foo", + }), + Limits: validation.LimitsPerLabelSetEntry{ + MaxSeries: 10e10, + }, + }, + } + dir := t.TempDir() blocksDir := filepath.Join(dir, "blocks") - require.NoError(t, os.Mkdir(blocksDir, os.ModePerm)) - ing, err := prepareIngesterWithBlocksStorageAndLimits(t, cfg, defaultLimitsTestConfig(), nil, blocksDir, prometheus.NewRegistry(), true) + ing, err := prepareIngesterWithBlocksStorageAndLimits(t, cfg, l, nil, blocksDir, prometheus.NewRegistry(), true) require.NoError(t, err) defer services.StopAndAwaitTerminated(context.Background(), ing) //nolint:errcheck require.NoError(t, services.StartAndAwaitRunning(context.Background(), ing)) diff --git a/pkg/ingester/user_state.go b/pkg/ingester/user_state.go index 60ca5c1eb6..7b0dec6920 100644 --- a/pkg/ingester/user_state.go +++ b/pkg/ingester/user_state.go @@ -118,7 +118,7 @@ func (m *labelSetCounter) canAddSeriesForLabelSet(ctx context.Context, u *userTS s := m.shards[util.HashFP(model.Fingerprint(set.Hash))%numMetricCounterShards] s.RLock() if r, ok := s.valuesCounter[set.Hash]; ok { - s.RUnlock() + defer s.RUnlock() return r.count, nil } s.RUnlock()