Skip to content
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

Fix data race on expanded postings Cache #6369

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

alanprot
Copy link
Member

@alanprot alanprot commented Nov 22, 2024

What this PR does:
We are using the wrong index on the stripped lock, causing data race.

This fix the issue by using the correct index on the stripped lock.

First commit has the test showing the race (failing): https://github.com/cortexproject/cortex/actions/runs/11981212549/job/33407007009

==================
WARNING: DATA RACE
Read at 0x00c004dc8010 by goroutine 18675:
  github.com/cortexproject/cortex/pkg/storage/tsdb.(*seedByHash).incrementSeed()
      /__w/cortex/cortex/pkg/storage/tsdb/expanded_postings_cache.go:317 +0x208
  github.com/cortexproject/cortex/pkg/storage/tsdb.(*blocksPostingsForMatchersCache).ExpireSeries()
      /__w/cortex/cortex/pkg/storage/tsdb/expanded_postings_cache.go:1[61](https://github.com/cortexproject/cortex/actions/runs/11981212549/job/33407007009#step:6:62) +0x157
  github.com/cortexproject/cortex/pkg/ingester.(*userTSDB).PostCreation()
      /__w/cortex/cortex/pkg/ingester/ingester.go:4[62](https://github.com/cortexproject/cortex/actions/runs/11981212549/job/33407007009#step:6:63) +0x315
  github.com/prometheus/prometheus/tsdb.(*stripeSeries).getOrSet()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head.go:2031 +0x2cd
  github.com/prometheus/prometheus/tsdb.(*Head).getOrCreateWithID()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head.go:1703 +0x13d
  github.com/prometheus/prometheus/tsdb.(*Head).getOrCreate()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head.go:1699 +0xe4
  github.com/prometheus/prometheus/tsdb.(*headAppender).getOrCreate()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head_append.go:438 +0x4ab
  github.com/prometheus/prometheus/tsdb.(*headAppender).Append()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head_append.go:334 +0x13b
  github.com/prometheus/prometheus/tsdb.(*initAppender).Append()
      /__w/cortex/cortex/vendor/github.com/prometheus/prometheus/tsdb/head_append.go:52 +0x137
  github.com/prometheus/prometheus/tsdb.(*dbAppender).Append()
      <autogenerated>:1 +0xa1
  github.com/cortexproject/cortex/pkg/ingester.(*Ingester).Push()
      /__w/cortex/cortex/pkg/ingester/ingester.go:1234 +0x3078
  github.com/cortexproject/cortex/pkg/ingester.TestExpendedPostingsCacheIsolation.func1()
      /__w/cortex/cortex/pkg/ingester/ingester_test.go:5117 +0x404

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@alanprot alanprot force-pushed the data-race-expanded-postings branch 2 times, most recently from e6b7c0e to d075c5c Compare November 22, 2024 22:34
@pull-request-size pull-request-size bot added size/M and removed size/S labels Nov 22, 2024
Signed-off-by: alanprot <alanprot@gmail.com>
Signed-off-by: alanprot <alanprot@gmail.com>
@alanprot alanprot merged commit 2153bef into cortexproject:master Nov 22, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants