Why
Every write that changes an issue, a comment, a change request, an approval, or
a release pointer commits to one ref, refs/heads/vole, and so does every push
that moves a followed branch. git.CommitFiles (core/git/trees.go) retries a
lost compare-and-swap up to 64 times with no backoff and no serialization, and
each attempt re-reads the whole branch — ls-tree -r plus one cat-file --batch over every blob — before writing the change.
So concurrent writers do not just queue. They each pay a full read, lose, and
pay it again. Attempts per write tracks the number of writers almost exactly,
and throughput falls as writers are added because the retries burn the CPU that
would have served them. At 64 concurrent writers the loop runs out and a user
gets refs/heads/vole: lost 64 ref race(s) for a write that was never wrong,
only unlucky. That is the risk at thousands of users: dropped updates, not slow
ones.
The Phase 0 spike already established the shape (c5626f1,
docs/spikes/ref-cas/README.md, since removed from the tree; quoted in #79):
one ref, 30 writes/s with 22 retries per write at 32 writers and quadratic decay
as the branch fills. Sharding was the structural answer, and that is a separate
change with a schema cost (a shared creation order for [#42](/vole/n/42), single-commit
landings). This issue is the part that needs no schema change.
Measurements
A bare repo seeded with 2,000 files on refs/heads/vole, 64 writes split across
N goroutines, GIT_TRACE counting update-ref invocations to get attempts per
write. Once per configuration, warm other than the first row:
| Writers | Writes/s | CAS attempts/write |
|---|---|---|
| 1 | 16.6 | 1.00 |
| 4 | 29.9 | 3.80 |
| 8 | 25.3 | 7.33 |
| 16 | 18.4 | 13.09 |
| 32 | 11.8 | 21.28 |
One uncontended write on that tree costs 43 ms
(BenchmarkCommitFilesOneFile). The same run with writers serialized behind a
mutex:
| Writers | Writes/s | CAS attempts/write |
|---|---|---|
| 8 | 27.6 | 1.00 |
| 32 | 28.0 | 1.00 |
2.2× throughput at 32 writers, 21× less wasted work, and a write’s latency becomes “queue position × 43 ms” instead of a variable number of discarded full reads.
Scope
- Serialize writers per repository. Hold the lock across the whole
read-modify-CAS loop, so a writer never reads a tree it will lose. Because
post-receive runs as its own process (
server/hook/hook.go), this needs aflockon the repository, not only an in-process mutex. Keep the CAS: it is the backstop for writes that bypass the lock (an admin push, a manualupdate-ref, a second host). - Do not commit for a push that changes nothing.
SyncBranchHeadparses every file underrequests/on every push to every branch, then usually writes nothing. Askdata/index.db(requests.head_ref) whether any open request follows the pushed branch and return before the read when none does. This is the largest machine-driven source of contention with human writes. - Read the branch once per write, not twice.
updateIssue,AddIssueCommentand their request equivalents load the metadata and thenCommitFilesreads it again. Merge the load into the mutate closure, which is also the correct shape: the decision must be made from the tree that is committed, asApproveRequestalready does withloadFrom(files, id). TodayupdateIssuerenders outside the loop and writes unconditionally, so a concurrent edit to the same issue is silently replaced. - Retry with jittered backoff for the cases the lock cannot cover, and
make the exhausted-retry error say what happened and what to do, not
lost 64 ref race(s). - Keep
CommitFilesthe single write path. No caller may move a metadata ref directly.
Acceptance
- A test with N concurrent writers reports a bounded number of CAS attempts per write (close to one), not a number that tracks N.
- A test asserts that a push to a branch no open request follows starts no metadata read and no commit.
- A test asserts a comment or edit reads the metadata tree once, counted the way
TestCommitFilesStaysProportionalToTheChangecounts processes withGIT_TRACE. - A concurrent edit to the same issue from two accounts does not lose one silently: the second write either sees the first or says it could not.
go test ./...passes, and the load harness gains a write scenario so the number is visible when it moves.
Not in this issue
Sharding the metadata refs, or batching commits per shard. Both are the measured
structural fix (#79’s spike: 1,240 writes/s sharded, 6,966 batched at 2k
entities, zero retries) and both change something load-bearing: [#42](/vole/n/42) numbers
derive from one branch’s creation order, and a landing marks a request merged and
closes linked issues in one commit. That needs its own decision.
Related
- #79 — the write path stopped rewriting the whole tree; contention was left open there, listed as “account for the CAS replay cost”.
- #80 — machine-written run records moved out of the branch, which removed the worst grower but not the contention.
- #113 — the index, which makes the push pre-check cheap.
c5626f1— the Phase 0 ref-CAS measurements.