You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The -race CI job has been off for ~2 years. I investigated why, found
the blockers are 3 small test-side data races (not production bugs), fixed them,
and verified the full unit suite now passes under -race. This issue tracks
landing those fixes and turning the job back on.
What happened
The unit-test race detector job was disabled in #3147 (Jan 2024), described there
as a temporary removal because it "intermittently fails across a handful of
tests". It's still commented out in .github/workflows/go-ci.yml today.
Why it matters
With -race off, data races merge unnoticed. Re-enabling it closes a ~2-year gap
and protects the codebase going forward — every PR gets checked automatically.
What I found
Ran the unit suite under -race locally, per-package: 36/41 packages already
clean. Every failure is test-side (shared fixtures/mocks or racy test code),
not a production bug:
share/eds + store/file — TestSuiteAccessor runs parallel subtests on a
shared *rsmt2d.ExtendedDataSquare; rsmt2d caches row/col roots lazily without
locking, so the parallel first-access races.
shrex_getter — the test shares one headertest.Subscriber mock between two
managers that call NextHeader concurrently.
libs/utils — TestSessions_ConcurrentDifferentKeys updates a plain int from
many goroutines (the Sessions implementation itself is correct).
api/clientTestSubmission — not a race; an intermittent embedded-testnode
failure ("nil block meta for height 2"), same framework as flaky: TestArchivalBlobSync in nodebuilder/tests #4958. Pre-existing
flake, out of scope here.
What I propose
Land the three test fixes as small PRs (each reproduces the race under -race),
then flip the job back on:
libs/utils: maxActive → atomic
shrex_getter: give each manager its own subscriber
share/eds: warm the EDS roots once before the parallel subtests
With all three fixes applied, the full unit suite passes under -race — 41/41
packages, locally. Re-enabling may still surface rarer intermittent races on CI
hardware; those can be handled iteratively.
I'd like to drive this. Separately, I've found several latent production races by
inspection that the current tests don't exercise — I'll send those as follow-ups
with regression tests once -race is back to guard against regressions.
The
-raceCI job has been off for ~2 years. I investigated why, foundthe blockers are 3 small test-side data races (not production bugs), fixed them,
and verified the full unit suite now passes under
-race. This issue trackslanding those fixes and turning the job back on.
What happened
The unit-test race detector job was disabled in #3147 (Jan 2024), described there
as a temporary removal because it "intermittently fails across a handful of
tests". It's still commented out in
.github/workflows/go-ci.ymltoday.Why it matters
With
-raceoff, data races merge unnoticed. Re-enabling it closes a ~2-year gapand protects the codebase going forward — every PR gets checked automatically.
What I found
Ran the unit suite under
-racelocally, per-package: 36/41 packages alreadyclean. Every failure is test-side (shared fixtures/mocks or racy test code),
not a production bug:
share/eds+store/file—TestSuiteAccessorruns parallel subtests on ashared
*rsmt2d.ExtendedDataSquare; rsmt2d caches row/col roots lazily withoutlocking, so the parallel first-access races.
shrex_getter— the test shares oneheadertest.Subscribermock between twomanagers that call
NextHeaderconcurrently.libs/utils—TestSessions_ConcurrentDifferentKeysupdates a plain int frommany goroutines (the
Sessionsimplementation itself is correct).api/clientTestSubmission— not a race; an intermittent embedded-testnodefailure ("nil block meta for height 2"), same framework as flaky: TestArchivalBlobSync in nodebuilder/tests #4958. Pre-existing
flake, out of scope here.
What I propose
Land the three test fixes as small PRs (each reproduces the race under
-race),then flip the job back on:
libs/utils:maxActive→ atomicshrex_getter: give each manager its own subscribershare/eds: warm the EDS roots once before the parallel subtests-racejob ingo-ci.yml(closes ci(unit tests): YOINK unit tests with race detector temporarily #3147)Verified
With all three fixes applied, the full unit suite passes under
-race— 41/41packages, locally. Re-enabling may still surface rarer intermittent races on CI
hardware; those can be handled iteratively.
I'd like to drive this. Separately, I've found several latent production races by
inspection that the current tests don't exercise — I'll send those as follow-ups
with regression tests once
-raceis back to guard against regressions.