Make the dispatch test fixture's temp root collision-proof - #176
Merged
Merged
Conversation
`fixture_toml` named its temporary root from the process id and a `SystemTime` nanosecond stamp. Both are shared across the test binary's threads, and the clock is not fine-grained enough to separate two fixtures built in the same instant, so two tests running in parallel could land on the same directory and the same scratch database. The symptom was an intermittent `UNIQUE constraint failed: projects.name`, seen once in the dispatch suite and passing on the retry — the kind of failure that teaches the operator to re-run rather than to read. The name now carries an atomic counter as well, which the test module increments per fixture. The clock still separates reruns of a recycled pid; the counter separates fixtures within one run, whatever the clock resolution. Two tests cover it. The first builds eight fixtures concurrently behind a barrier and asserts distinct roots. Eight is too few to catch a name that leans on the clock alone, so the second names four thousand roots across eight threads with nothing between them: with the counter replaced by a plain load, that test fails with 29 collisions, which is also the mechanism of the original flake. Test code only. `cargo test --workspace` passes five runs over, with clippy and fmt clean.
The counter added alongside the stamp reads as unnecessary — a nanosecond is short enough that nothing should share one — and the comment asserted the collision without saying what causes it, leaving the next reader to make the same objection. Measured on this workstation: 100000 consecutive `SystemTime::now()` reads on one thread never repeat, so the objection holds sequentially. The clock does not advance a nanosecond at a time, though; it steps every 20-30ns. On one thread each read is ordered after the last and so lands on a later step, but threads have no such ordering, and any two reading inside one step read the same number: 4000 reads across eight threads gave 1493 duplicates. A test binary runs its tests on threads, which is exactly that case. The comments now carry the mechanism and the measurement, and the note on the four-thousand-root test says what the eight-fixture test above it cannot catch: those eight sit in `git init` long enough to drift onto separate steps, so they pass even on a clock-only name. Comments only; no code changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixture_tomlincrates/voro/src/dispatch.rsnamed its temporary root fromthe process id and a
SystemTimenanosecond stamp. Both are shared across thetest binary's threads and the clock is not fine-grained enough to separate two
fixtures built in the same instant, so two tests running in parallel could land
on the same directory and the same scratch database — the intermittent
UNIQUE constraint failed: projects.nameseen in the dispatch suite.The root name now carries an atomic counter incremented per fixture, alongside
the existing pid and stamp: the counter separates fixtures within a run
whatever the clock resolution, the stamp still separates reruns of a recycled
pid. The naming moved into a
fixture_root()helper in the test module;fixtureandfixture_tomlkeep their signatures and behaviour.Two tests cover it.
fixtures_built_at_once_on_many_threads_get_distinct_rootsbuilds eight fixtures concurrently behind a barrier and asserts the roots are
distinct, as the acceptance criteria ask. Eight proved too few to catch a name
that leans on the clock alone, so
roots_named_back_to_back_on_many_threads_are_all_distinctnames four thousand roots across eight threads with nothing between them.
Verified: with the counter swapped for a plain load, the second test fails with
29 collisions out of 4000 — which confirms the original flake's mechanism
rather than assuming it — and passes with the counter restored.
cargo test --workspacewas run five times over and passed every run, withcargo clippy --workspace --all-targets -- -D warningsandcargo fmt --all -- --checkclean. Test code only; no production code changed.Follow-up filed as task #455: twenty other test temp roots are named the same
way, most single-caller and so safe today, but the shared helpers are not
robust by construction.