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.

Metadata writers queue on the branch instead of racing for it

122cbe4f62bf87a941538450584ba1b20444c135 · Keenan Nemetz · 7d ago · parent 4739664b

Metadata writers queue on the branch instead of racing for it

Every write to refs/heads/vole read the whole branch, decided, and moved the
ref with a compare-and-swap, retrying up to 64 times with no backoff and no
coordination. Concurrent writers therefore did not queue: they each paid a full
read, lost, and paid it again. Measured on a 2,000-file branch, one write needed
one attempt alone and 21 with 32 writers, and throughput fell as writers were
added because the retries were the load. At 64 simultaneous writers the loop ran
out and the write failed for being unlucky.

The read-modify-write now runs with one writer at a time, holding a flock in the
repository so the SSH hook and the server queue behind each other. The
compare-and-swap stays for writers that go around the lock — a push, a hand-run
git command, another host — and losing one now waits with jittered backoff
before deciding again.

Writers read the branch once. The load and the decision happen inside the commit
instead of before it, which also stops an edit from replacing a change that
landed while the page was open: the decision is made against the tree it writes.

A push only does that work when it has metadata to move. Post-receive asks the
index which open requests follow the branch it moved, and a branch nobody is
proposing from now writes an audit line and no commit.

Measurements, 32 concurrent writers: 21.3 CAS attempts per write before, 1.0
after. Through the web, posting comments at concurrency 32: p99 4.7 s -> 0.95 s,
and throughput no longer falls as workers are added. The load harness gains a
`comment` scenario so the number is visible.

Closes: 01M6D9G8H6E0RCQ3AD5TMAGX34

metadata/write_read_test.go +193

@@ -0,0 +1,193 @@
1 package metadata
2
3 // A metadata write reads the branch once, and decides against the tree it is about
4 // to replace. These tests hold both halves: the process count that makes the first
5 // true, and the landed change that makes the second matter.
6
7 import (
8 "bytes"
9 "fmt"
10 "os"
11 "path/filepath"
12 "strings"
13 "testing"
14 "time"
15
16 "github.com/rodent-software/vole/core/git"
17 )
18
19 func writeRepo(t *testing.T) *git.Repo {
20 t.Helper()
21 root := t.TempDir()
22 if err := git.InitBare(root, "main"); err != nil {
23 t.Fatal(err)
24 }
25 g := git.Open(root)
26 seed, err := g.CommitTree(mustTree(t, g), "", "seed")
27 if err != nil {
28 t.Fatal(err)
29 }
30 if err := g.UpdateRef(Ref, seed); err != nil {
31 t.Fatal(err)
32 }
33 return g
34 }
35
36 // TestCommentReadsTheBranchOnce is the regression test for the second read: a write
37 // used to load the metadata and then let CommitFiles read it again, so a comment cost
38 // two tree walks. GIT_TRACE counts the child git processes, and one walk is one
39 // ls-tree and one cat-file --batch.
40 func TestCommentReadsTheBranchOnce(t *testing.T) {
41 g := writeRepo(t)
42 now := time.Date(2026, 11, 2, 12, 0, 0, 0, time.UTC)
43 iss, err := NewIssue("Comments cost one read", "", "keenan", nil, nil, now)
44 if err != nil {
45 t.Fatal(err)
46 }
47 if err := CreateIssue(g, iss, "keenan"); err != nil {
48 t.Fatal(err)
49 }
50 for i := 0; i < 20; i++ {
51 other, err := NewIssue(fmt.Sprintf("Unrelated %d", i), "", "keenan", nil, nil, now)
52 if err != nil {
53 t.Fatal(err)
54 }
55 if err := CreateIssue(g, other, "keenan"); err != nil {
56 t.Fatal(err)
57 }
58 }
59
60 trace := filepath.Join(t.TempDir(), "git.trace")
61 traced := *g
62 traced.ExtraEnv = append(traced.ExtraEnv, "GIT_TRACE="+trace)
63
64 c, err := NewIssueComment("one comment", "keenan", now.Add(time.Minute))
65 if err != nil {
66 t.Fatal(err)
67 }
68 if err := AddIssueComment(&traced, iss.ID, c, "keenan"); err != nil {
69 t.Fatal(err)
70 }
71 data, err := os.ReadFile(trace)
72 if err != nil {
73 t.Fatal(err)
74 }
75 // A branch with 21 issues on it, and the comment still reads it once.
76 for _, call := range []string{"ls-tree", "cat-file"} {
77 if n := bytes.Count(data, []byte(call)); n != 1 {
78 t.Fatalf("one comment ran %s %d times, want 1:\n%s", call, n, data)
79 }
80 }
81 }
82
83 // TestEditKeepsAChangeThatLandedFirst is what reading inside the commit buys. The
84 // branch moves between this writer's first decision and its commit — a push, or a
85 // writer that went around the lock — and the retry makes the decision again against
86 // the tree it is about to write, so the other change survives instead of being
87 // replaced by what this writer saw earlier.
88 func TestEditKeepsAChangeThatLandedFirst(t *testing.T) {
89 g := writeRepo(t)
90 now := time.Date(2026, 11, 2, 12, 0, 0, 0, time.UTC)
91 iss, err := NewIssue("Two editors", "", "keenan", []string{"bug"}, nil, now)
92 if err != nil {
93 t.Fatal(err)
94 }
95 if err := CreateIssue(g, iss, "keenan"); err != nil {
96 t.Fatal(err)
97 }
98
99 decisions := 0
100 got, err := UpdateIssue(g, iss.ID, func(cur *Issue) error {
101 decisions++
102 if decisions == 1 {
103 // Somebody else added a label and their commit landed. The ref move is
104 // the event this writer has to notice, and the attempt that made it loses
105 // the compare-and-swap.
106 if err := handEditIssue(g, iss.ID, "Two editors", []string{"bug", "docs"}); err != nil {
107 return fmt.Errorf("hand edit: %w", err)
108 }
109 cur.Labels = append(cur.Labels, "extra")
110 return nil
111 }
112 if len(cur.Labels) != 2 || cur.Labels[1] != "docs" {
113 return fmt.Errorf("the write did not see the tree it is writing: labels %v", cur.Labels)
114 }
115 cur.Labels = append(cur.Labels, "extra")
116 return nil
117 }, "keenan")
118 if err != nil {
119 t.Fatal(err)
120 }
121 if decisions < 2 {
122 t.Fatalf("the write needed %d decision(s), so it never retried", decisions)
123 }
124 if len(got.Labels) != 3 || got.Labels[2] != "extra" {
125 t.Fatalf("labels = %v, want [bug docs extra]", got.Labels)
126 }
127 // And the branch agrees, rather than keeping the first writer's stale copy.
128 stored, found, err := LoadIssueAt(g, Ref, iss.ID)
129 if err != nil || !found {
130 t.Fatalf("reading the issue back: %v, found %v", err, found)
131 }
132 if strings.Join(stored.Labels, ",") != "bug,docs,extra" {
133 t.Fatalf("stored labels = %v, want [bug docs extra]", stored.Labels)
134 }
135 }
136
137 // handEditIssue writes one issue straight onto the branch, the way another writer
138 // would: a new commit and a ref move, with no lock held and no metadata writer
139 // involved.
140 func handEditIssue(g *git.Repo, id, title string, labels []string) error {
141 files, err := g.FlatTree(Ref)
142 if err != nil {
143 return err
144 }
145 raw, err := renderIssue(&Issue{
146 ID: id,
147 Title: title,
148 State: "open",
149 Author: "wilbur",
150 Created: time.Date(2026, 11, 2, 12, 0, 0, 0, time.UTC),
151 Labels: labels,
152 })
153 if err != nil {
154 return err
155 }
156 files[IssuesDir+"/"+id+"/"+issueName] = raw
157 tree, err := git.WriteFlatTree(g, files)
158 if err != nil {
159 return err
160 }
161 old, err := g.RevParseRaw(Ref)
162 if err != nil {
163 return err
164 }
165 commit, err := g.CommitTree(tree, old, "issue: hand edit")
166 if err != nil {
167 return err
168 }
169 return g.UpdateRef(Ref, commit)
170 }
171
172 // A write whose decision fails still says why, and names the thing the caller asked
173 // for rather than the branch it was reading.
174 func TestWritingAMissingEntityNamesIt(t *testing.T) {
175 g := writeRepo(t)
176 missing := "01M5JR2JK29KKMAA1T9MDNTRB9"
177 if _, err := UpdateIssue(g, missing, func(*Issue) error { return nil }, "keenan"); err == nil ||
178 !strings.Contains(err.Error(), "no issue "+missing) {
179 t.Fatalf("edit of a missing issue = %v", err)
180 }
181 c, err := NewIssueComment("hello", "keenan", time.Now())
182 if err != nil {
183 t.Fatal(err)
184 }
185 if err := AddIssueComment(g, missing, c, "keenan"); err == nil ||
186 !strings.Contains(err.Error(), "no issue "+missing) {
187 t.Fatalf("comment on a missing issue = %v", err)
188 }
189 if _, err := EditIssueComment(g, missing, c.ID, "changed", "keenan"); err == nil ||
190 !strings.Contains(err.Error(), "no issue "+missing) {
191 t.Fatalf("edit of a comment on a missing issue = %v", err)
192 }
193 }