chadtmiller.com/knotless

Switch repository

Loading…

What changes

Fetches into one mirror now take turns, under the operating system's lock on a file named fetching inside it. A sync that finds a commit already there now waits for the fetch that brought it, rather than going on with its parents still to come.

Why

A fetch lands the newest packs first. A second sync starting partway found the wanted commit present, so it had nothing to fetch, moved the refs, and git commit-graph write failed on the parent that had not landed: Could not read <sha>, then unable to parse commit <sha>.

That is what a_workflow_uses_a_shared_step has been failing on: several of its workflows sync the same shared-steps repository at once. It is logged FLAKY on main's runs, and it ejected a pull from the queue on 2026-11-04. The check it fails is an error to anyone using shared steps, so this is a fault in the runner and not only in the test.

How it was checked

syncs_at_once_wait_for_a_fetch_under_way in crates/knotless/tests/library.rs starts twelve syncs a few milliseconds apart on an empty mirror, thirty times over. Run here on one test, with the lock taken out it failed 3 of 3 times with the error above. With the lock it passed 5 of 5.

Changeset: patch, in .changeset/fetches-take-turns.md.

Shortcomings

A fetch that stops partway, as a killed process would, still leaves a mirror that fails this way. That is filed apart, as "A fetch that stops partway leaves a mirror that cannot sync".

Readers with nothing to fetch now wait for a fetch under way in the same mirror. That was not measured on prod. The wait is as long as that fetch's download, and only while one is running.

The lock's file also lands in a checkout's .git when the git helper fetches. Git ignores it.

🤖 Generated with Claude Code

This makes fetches into one mirror take turns, under an OS lock on a fetching file inside it. A sync that finds a commit already there now waits for the fetch that brought it, instead of going on with its parents still to come. I read the diff and the code around it, and the fix looks right.

• Placement: fetch::fetch takes the lock before its first git.missing check, so the "commit present, parents not" race is closed at the point it happens. All callers (sync, the runner, compact, the git helper and the pull-head fetch) go through it.
• Deadlock: refresh fetches before it takes the refs lock, so the two locks are never held in opposite orders.
• Crashes: the OS releases the lock when its holder dies, so a crashed process cannot block the mirror.
• Tests, docs and changeset: the test would fail without the lock, per the body. docs/design/storage.md and the changeset are updated as AGENTS.md asks.

The costs the body lists (readers waiting on a fetch under way, a partial fetch still leaving a bad mirror) are fair trade-offs and are filed or noted already. I have no change to ask for.

Carried forward: the change this pull makes at 9c8c9b91c32ed2c79564f8201f7660adc0c28242 is the one reviewed at 619cad944f18ffcfd94cb54ea77c2c706846c608, so that review stands. What is new comes from its base.