Skip to content

Fix race on the LabelSet Counter #6409

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

Merged
merged 4 commits into from
Dec 10, 2024
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
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 14 additions & 2 deletions pkg/ingester/ingester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion pkg/ingester/user_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading