Nothing runs go test -race, so the next data race is found by hand or not at all #1139

New issue

Open

opened 2026-08-23 02:07:32 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-23 02:07:32 +00:00

Member

Filed by Darren (director seat), 2026-08-23. Raised in the body of coilyco-gaming/sirens-echo#1137 and left deliberately undone there, correctly, since changing the gate inside a bug fix is the wrong place for a cost decision. coilyco-gaming/sirens-echo#1136 closed with that pull request, so the question needs a home.

The gap

just test runs without -race. So does CI's test job.

coilyco-gaming/sirens-echo#1136 was a genuine data race on main, three parallel tests rewriting package-level tuning variables while other parallel tests read them, and it was found by a human running go test -race by hand rather than by any check. main was green throughout.

The reporting engineer's own summary:

just test runs without -race, so nothing in CI caught this and nothing would catch the next one.

That last clause is the issue. One race was found and fixed. The detector that found it is not wired to anything.

Why it is a real decision rather than an obvious yes

Racing the suite roughly doubles it. Against coilyco-gaming/sirens-echo#838's measurement of 57 seconds for test today, that is roughly a minute added to every pull request run.

That measurement also has a caveat worth carrying: one pull-request run tonight took about 14.5 minutes against a measured p90 of 76 seconds, at coilyco-gaming/sirens-echo#1121. So the tail is not well characterised, and doubling a poorly-understood distribution is a different proposition from doubling a tight one.

Options, in rough order of cost

Race the suite on every run. Simplest, catches everything, costs about a minute per run.
Race on main pushes only. Free on the pull-request path where cost is felt, and finds a race after it lands rather than before, which is a real weakening given coilyco-gaming/sirens-echo#568 means nothing measures a merge before it becomes main.
Race a scheduled run. Cheapest and slowest to notice, and the repository already has image-coverage.yml as precedent for a scheduled job.
Race only the packages that carry concurrency. internal/community holds the slot pool, the coalescing lane, and the turn log, and it is where both races so far have been. Narrower and cheaper, at the cost of a rule someone has to maintain.

I would take 1 or 4. This lane shipped an eight-slot execution pool tonight, so the amount of concurrent code in internal/community went up materially in one evening, and the two races found so far were both in that package.

Done when

Either the race detector runs somewhere automatic, or a decision is recorded that it does not and why, so the next person finding a race by hand knows it was a choice rather than an oversight.

coilyco-gaming/sirens-echo#1136 - the race, closed
coilyco-gaming/sirens-echo#1137 - the fix, and where this was raised
coilyco-gaming/sirens-echo#838 - the CI duration measurement this trades against
coilyco-gaming/sirens-echo#995 - the slot pool, which is why the package got more concurrent tonight

**Filed by Darren (director seat), 2026-08-23.** Raised in the body of `coilyco-gaming/sirens-echo#1137` and left deliberately undone there, correctly, since changing the gate inside a bug fix is the wrong place for a cost decision. `coilyco-gaming/sirens-echo#1136` closed with that pull request, so the question needs a home. ## The gap `just test` runs without `-race`. So does CI's `test` job. `coilyco-gaming/sirens-echo#1136` was a genuine data race on `main`, three parallel tests rewriting package-level tuning variables while other parallel tests read them, and **it was found by a human running `go test -race` by hand rather than by any check**. `main` was green throughout. The reporting engineer's own summary: > **`just test` runs without `-race`, so nothing in CI caught this and nothing would catch the next one.** That last clause is the issue. One race was found and fixed. The detector that found it is not wired to anything. ## Why it is a real decision rather than an obvious yes **Racing the suite roughly doubles it.** Against `coilyco-gaming/sirens-echo#838`'s measurement of 57 seconds for `test` today, that is roughly a minute added to every pull request run. That measurement also has a caveat worth carrying: **one pull-request run tonight took about 14.5 minutes** against a measured p90 of 76 seconds, at `coilyco-gaming/sirens-echo#1121`. So the tail is not well characterised, and doubling a poorly-understood distribution is a different proposition from doubling a tight one. ## Options, in rough order of cost 1. **Race the suite on every run.** Simplest, catches everything, costs about a minute per run. 2. **Race on `main` pushes only.** Free on the pull-request path where cost is felt, and finds a race after it lands rather than before, which is a real weakening given `coilyco-gaming/sirens-echo#568` means nothing measures a merge before it becomes `main`. 3. **Race a scheduled run.** Cheapest and slowest to notice, and the repository already has `image-coverage.yml` as precedent for a scheduled job. 4. **Race only the packages that carry concurrency.** `internal/community` holds the slot pool, the coalescing lane, and the turn log, and it is where both races so far have been. Narrower and cheaper, at the cost of a rule someone has to maintain. I would take 1 or 4. **This lane shipped an eight-slot execution pool tonight**, so the amount of concurrent code in `internal/community` went up materially in one evening, and the two races found so far were both in that package. ## Done when Either the race detector runs somewhere automatic, or a decision is recorded that it does not and why, so the next person finding a race by hand knows it was a choice rather than an oversight. ## Related * `coilyco-gaming/sirens-echo#1136` - the race, closed * `coilyco-gaming/sirens-echo#1137` - the fix, and where this was raised * `coilyco-gaming/sirens-echo#838` - the CI duration measurement this trades against * `coilyco-gaming/sirens-echo#995` - the slot pool, which is why the package got more concurrent tonight

coilyco-ops added the

labels

2026-08-23 02:07:32 +00:00

coilyco-ops referenced this issue

2026-08-23 02:07:46 +00:00

Three parallel tests write the package knobs, so go test -race fails on main #1136

coilyco-ops referenced this issue

2026-08-23 02:08:16 +00:00