Skip to content

Commit c277158

Browse files
authored
Cleanup data race associated with workerID var (#11922)
**What this PR does / why we need it**: A data race existed with the workerID variable, as it could be modified by multiple goroutines. Relates to: #8586 -- Before fix: ``` go test -count=1 -race ./pkg/querier/worker ================== WARNING: DATA RACE Read at 0x00c000494108 by goroutine 229: github.com/grafana/loki/pkg/querier/worker.(*processorManager).concurrency.func1() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/processor_manager.go:81 +0x118 Previous write at 0x00c000494108 by goroutine 222: github.com/grafana/loki/pkg/querier/worker.(*processorManager).concurrency() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/processor_manager.go:70 +0x108 github.com/grafana/loki/pkg/querier/worker.(*querierWorker).resetConcurrency() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker.go:267 +0x10c github.com/grafana/loki/pkg/querier/worker.(*querierWorker).AddressAdded() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker.go:219 +0x868 github.com/grafana/loki/pkg/querier/worker.TestResetConcurrency.func1() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker_test.go:64 +0x1c8 testing.tRunner() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0 testing.(*T).Run.func1() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40 Goroutine 229 (running) created at: github.com/grafana/loki/pkg/querier/worker.(*processorManager).concurrency() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/processor_manager.go:75 +0xcc github.com/grafana/loki/pkg/querier/worker.(*querierWorker).resetConcurrency() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker.go:267 +0x10c github.com/grafana/loki/pkg/querier/worker.(*querierWorker).AddressAdded() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker.go:219 +0x868 github.com/grafana/loki/pkg/querier/worker.TestResetConcurrency.func1() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker_test.go:64 +0x1c8 testing.tRunner() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0 testing.(*T).Run.func1() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40 Goroutine 222 (running) created at: testing.(*T).Run() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x5e8 github.com/grafana/loki/pkg/querier/worker.TestResetConcurrency() /Users/progers/dev/src/github.com/grafana/loki/pkg/querier/worker/worker_test.go:52 +0x1b0 testing.tRunner() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1595 +0x1b0 testing.(*T).Run.func1() /opt/homebrew/Cellar/go/1.21.6/libexec/src/testing/testing.go:1648 +0x40 ================== --- FAIL: TestResetConcurrency (0.02s) --- FAIL: TestResetConcurrency/concurrency_is_correct_when_numTargets_does_not_divide_evenly_into_maxConcurrent (0.01s) testing.go:1465: race detected during execution of test testing.go:1465: race detected during execution of test FAIL FAIL github.com/grafana/loki/pkg/querier/worker 4.626s FAIL ``` -- After fix: ``` go clean -testcache go test -count=1 -race ./pkg/querier/worker ok github.com/grafana/loki/pkg/querier/worker 6.034s ``` **Which issue(s) this PR fixes**: Fixes #<issue number> **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [ ] Tests updated - [ ] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](d10549e) - [ ] 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](0d4416a)
1 parent d9d9ebd commit c277158

File tree

1 file changed

+3
-4
lines changed

1 file changed

+3
-4
lines changed

pkg/querier/worker/processor_manager.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,20 @@ func (pm *processorManager) concurrency(n int) {
6565
n = 0
6666
}
6767

68-
workerID := 0
6968
for len(pm.cancels) < n {
70-
workerID++
69+
workerID := len(pm.cancels) + 1
7170
ctx, cancel := context.WithCancel(pm.ctx)
7271
pm.cancels = append(pm.cancels, cancel)
7372

7473
pm.wg.Add(1)
75-
go func() {
74+
go func(workerID int) {
7675
defer pm.wg.Done()
7776

7877
pm.currentProcessors.Inc()
7978
defer pm.currentProcessors.Dec()
8079

8180
pm.p.processQueriesOnSingleStream(ctx, pm.conn, pm.address, strconv.Itoa(workerID))
82-
}()
81+
}(workerID)
8382
}
8483

8584
for len(pm.cancels) > n {

0 commit comments

Comments
 (0)