Fix race on the LabelSet Counter (#6409)

* Configure labelset limits on the TestPushRace Signed-off-by: alanprot <[email protected]> * Fix race Signed-off-by: alanprot <[email protected]> * Changelog Signed-off-by: alanprot <[email protected]> --------- Signed-off-by: alanprot <[email protected]> Signed-off-by: Alan Protasio <[email protected]>
cortexproject · Dec 10, 2024 · e9d038c · e9d038c
1 parent 5d2dac0
commit e9d038c
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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()