TestConcurrentCallsDoNotRaceOnTheGateHistogram (components/offload/extract_llm_gaterace_test.go:65) failed once during a full go test ./... run, then would not reproduce.
--- FAIL: TestConcurrentCallsDoNotRaceOnTheGateHistogram (0.03s)
extract_llm_gaterace_test.go:65: no single-flight follower ran, so no two gate raises were
concurrent (gates: map[]) — the race was never exercised
What I checked
| Run |
Result |
Full go test ./... on my branch (first time) |
FAIL |
Same test, -count=8, in isolation on my branch |
8/8 pass |
Full go test ./... on my branch, twice more |
pass |
Same test, 5 separate runs on clean main |
5/5 pass |
Full go test ./... on clean main |
pass |
Nothing in my branch touches extract_llm, the offload package, or the single-flight machinery, and
the failure has not recurred in four subsequent full-suite runs. I am reporting it rather than
leaving it in a transcript, not claiming a diagnosis.
Why it is worth fixing rather than ignoring
The test is doing the right thing — it refuses to pass vacuously. gates: map[] says no gate
raise was recorded at all, so the concurrency it exists to check never happened, and rather than
reporting a green result it fails. That is the correct design and exactly what this repo asks of a
guard.
The consequence is that it fails intermittently under load, which is worse than it sounds: a guard
that cries wolf gets re-run until green, and then it stops being trusted for the case it was
written for. It was written for a real race, so that trust matters.
Mechanism, as far as I can tell without digging
The test needs two goroutines to reach the gate-raise concurrently so that one becomes a
single-flight follower. go test ./... runs package binaries in parallel (up to GOMAXPROCS, 16 on
the box I used), so the scheduler has far less freedom to interleave two goroutines inside one test
than it does when the package runs alone. That fits the observation — fails only in the full suite,
passes 8/8 alone — but it is a hypothesis, not a finding.
Options
- Make the interleaving deterministic rather than hoped for: have the first caller block on a
channel the test controls until the second has demonstrably arrived at the gate, then release
both. Removes the load sensitivity entirely, and keeps the vacuity check as a backstop.
- Retry the setup inside the test — attempt the race up to N times and only fail if no attempt
produced a follower. Cheaper, still probabilistic.
- Leave it, and accept an occasional red CI run on a guard nobody will then trust.
(1) is what I would do: the property under test is "when two raises are concurrent, the histogram
does not race", and the concurrency is a precondition the test should manufacture rather than wait
for.
Found while running the suite for #161. Not fixed there — it is unrelated to that change, and
changing a concurrency test's synchronisation deserves its own review rather than riding along.
TestConcurrentCallsDoNotRaceOnTheGateHistogram(components/offload/extract_llm_gaterace_test.go:65) failed once during a fullgo test ./...run, then would not reproduce.What I checked
go test ./...on my branch (first time)-count=8, in isolation on my branchgo test ./...on my branch, twice moremaingo test ./...on cleanmainNothing in my branch touches
extract_llm, the offload package, or the single-flight machinery, andthe failure has not recurred in four subsequent full-suite runs. I am reporting it rather than
leaving it in a transcript, not claiming a diagnosis.
Why it is worth fixing rather than ignoring
The test is doing the right thing — it refuses to pass vacuously.
gates: map[]says no gateraise was recorded at all, so the concurrency it exists to check never happened, and rather than
reporting a green result it fails. That is the correct design and exactly what this repo asks of a
guard.
The consequence is that it fails intermittently under load, which is worse than it sounds: a guard
that cries wolf gets re-run until green, and then it stops being trusted for the case it was
written for. It was written for a real race, so that trust matters.
Mechanism, as far as I can tell without digging
The test needs two goroutines to reach the gate-raise concurrently so that one becomes a
single-flight follower.
go test ./...runs package binaries in parallel (up toGOMAXPROCS, 16 onthe box I used), so the scheduler has far less freedom to interleave two goroutines inside one test
than it does when the package runs alone. That fits the observation — fails only in the full suite,
passes 8/8 alone — but it is a hypothesis, not a finding.
Options
channel the test controls until the second has demonstrably arrived at the gate, then release
both. Removes the load sensitivity entirely, and keeps the vacuity check as a backstop.
produced a follower. Cheaper, still probabilistic.
(1) is what I would do: the property under test is "when two raises are concurrent, the histogram
does not race", and the concurrency is a precondition the test should manufacture rather than wait
for.
Found while running the suite for #161. Not fixed there — it is unrelated to that change, and
changing a concurrency test's synchronisation deserves its own review rather than riding along.