Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

#152

P2: Serialize metadata writes so contention cannot drop an update

closed · opened by keenan 7d ago

keenan opened this 7d ago

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 a flock on 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 manual update-ref, a second host).
  • Do not commit for a push that changes nothing. SyncBranchHead parses every file under requests/ on every push to every branch, then usually writes nothing. Ask data/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, AddIssueComment and their request equivalents load the metadata and then CommitFiles reads 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, as ApproveRequest already does with loadFrom(files, id). Today updateIssue renders 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 CommitFiles the 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 TestCommitFilesStaysProportionalToTheChange counts processes with GIT_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.

Comments (0)

No comments yet.