fix(walk): measure the commit-graph racy window from the file mtime (GO-001)

The graph was trusted once it had been loaded for more than racyWindow, so every reload restarted the window and a Refresh every second reloaded and remapped the graph forever. Trust the stamp when the time it was taken is more than racyWindow after the newest mtime of the graph files, the rule of fileSig.same.

commit: 555a3248e525074f46873bfc0a9dd1cf53eb682d
author: brice · 11d ago · 2026-10-10 23:47 UTC
parent: ea9bcf1
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +195 -4

M graph.go +20 -0


@@ -726,6 +726,26 @@ type graphFileStamp struct {
726	726		// the chain file (layers are immutable and named by checksum).
727	727		type graphStamp struct{ single, chain graphFileStamp }
728	728
	729	+	// trusted reports whether unchanged stat data proves that the files did not
	730	+	// change, for a stamp taken at the given time (unix nanoseconds). This is
	731	+	// Git's racy rule, the one of fileSig.same: a write that follows within the
	732	+	// clock granularity of the previous modification can leave the stat data as
	733	+	// it was, so the data only proves anything once the newest modification is
	734	+	// older than racyWindow when the stamp was taken. Git writes graph files by
	735	+	// rename, which already changes the inode; the rule covers the rest. The
	736	+	// time is that of the stamp, not of the last reload: measuring from the
	737	+	// reload would restart the window each time and reload a busy repository
	738	+	// forever.
	739	+	func (s graphStamp) trusted(at int64) bool {
	740	+	newest := int64(math.MinInt64)
	741	+	for _, f := range [...]graphFileStamp{s.single, s.chain} {
	742	+	if f.exists {
	743	+	newest = max(newest, f.mtime)
	744	+	}
	745	+	}
	746	+	return newest == math.MinInt64 \|\| at-newest > int64(racyWindow)
	747	+	}
	748	+
729	749		func statGraphFile(path string) graphFileStamp {
730	750		st, err := os.Stat(path)
731	751		if err != nil {


@@ -726,6 +726,26 @@ type graphFileStamp struct {
726	// the chain file (layers are immutable and named by checksum).	726		// the chain file (layers are immutable and named by checksum).
727	type graphStamp struct{ single, chain graphFileStamp }	727		type graphStamp struct{ single, chain graphFileStamp }
728		728
		729	+	// trusted reports whether unchanged stat data proves that the files did not
		730	+	// change, for a stamp taken at the given time (unix nanoseconds). This is
		731	+	// Git's racy rule, the one of fileSig.same: a write that follows within the
		732	+	// clock granularity of the previous modification can leave the stat data as
		733	+	// it was, so the data only proves anything once the newest modification is
		734	+	// older than racyWindow when the stamp was taken. Git writes graph files by
		735	+	// rename, which already changes the inode; the rule covers the rest. The
		736	+	// time is that of the stamp, not of the last reload: measuring from the
		737	+	// reload would restart the window each time and reload a busy repository
		738	+	// forever.
		739	+	func (s graphStamp) trusted(at int64) bool {
		740	+	newest := int64(math.MinInt64)
		741	+	for _, f := range [...]graphFileStamp{s.single, s.chain} {
		742	+	if f.exists {
		743	+	newest = max(newest, f.mtime)
		744	+	}
		745	+	}
		746	+	return newest == math.MinInt64 \|\| at-newest > int64(racyWindow)
		747	+	}
		748	+
729	func statGraphFile(path string) graphFileStamp {	749		func statGraphFile(path string) graphFileStamp {
730	st, err := os.Stat(path)	750		st, err := os.Stat(path)
731	if err != nil {	751		if err != nil {

A graph_racy_test.go +158 -0

@@ -0,0 +1,158 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package git
	4	+
	5	+	import (
	6	+	"os"
	7	+	"path/filepath"
	8	+	"sync/atomic"
	9	+	"testing"
	10	+	"time"
	11	+	)
	12	+
	13	+	func TestGraphStampTrusted(t *testing.T) {
	14	+	const sec = int64(time.Second)
	15	+	mt := 1_000 * sec
	16	+	win := int64(racyWindow)
	17	+	exists := func(m int64) graphFileStamp { return graphFileStamp{exists: true, mtime: m} }
	18	+	for _, c := range []struct {
	19	+	name string
	20	+	stamp graphStamp
	21	+	at int64
	22	+	want bool
	23	+	}{
	24	+	{"no graph", graphStamp{}, mt, true},
	25	+	{"stamped inside the window", graphStamp{single: exists(mt)}, mt + win, false},
	26	+	{"stamped just after the window", graphStamp{single: exists(mt)}, mt + win + 1, true},
	27	+	{"newest of two decides", graphStamp{single: exists(mt - 100*sec), chain: exists(mt)}, mt + sec, false},
	28	+	{"both old", graphStamp{single: exists(mt - 100sec), chain: exists(mt - 50sec)}, mt, true},
	29	+	{"mtime in the future", graphStamp{chain: exists(mt + 10*sec)}, mt, false},
	30	+	} {
	31	+	if got := c.stamp.trusted(c.at); got != c.want {
	32	+	t.Errorf("%s: trusted = %v, want %v", c.name, got, c.want)
	33	+	}
	34	+	}
	35	+	}
	36	+
	37	+	func graphFiles(r *gtRepo) []string {
	38	+	info := filepath.Join(r.objects(), "info")
	39	+	return []string{filepath.Join(info, "commit-graph"), filepath.Join(info, "commit-graphs", "commit-graph-chain")}
	40	+	}
	41	+
	42	+	func ageGraph(t testing.T, r gtRepo, age time.Duration) {
	43	+	t.Helper()
	44	+	mt := time.Now().Add(-age)
	45	+	n := 0
	46	+	for _, p := range graphFiles(r) {
	47	+	if _, err := os.Stat(p); err == nil {
	48	+	n++
	49	+	if err := os.Chtimes(p, mt, mt); err != nil {
	50	+	t.Fatal(err)
	51	+	}
	52	+	}
	53	+	}
	54	+	if n == 0 {
	55	+	t.Fatal("no graph file")
	56	+	}
	57	+	}
	58	+
	59	+	// A graph that is older than the racy window is trusted when its stat data
	60	+	// is unchanged, however recently it was loaded: Refresh at the pace of the
	61	+	// adapter (every second) must not reload it, and so not remap it.
	62	+	func TestGraphRefreshDoesNotReloadAnUnchangedGraph(t *testing.T) {
	63	+	r := newGTRepo(t)
	64	+	r.commit("a", "a\n")
	65	+	r.commit("b", "b\n")
	66	+	r.git("commit-graph", "write", "--reachable", "--split")
	67	+	ageGraph(t, r, time.Hour)
	68	+	repo, err := Open(r.dir)
	69	+	if err != nil {
	70	+	t.Fatal(err)
	71	+	}
	72	+	defer func() { _ = repo.Close() }()
	73	+	first := repo.graph.Load()
	74	+	if first == nil \|\| first.g == nil {
	75	+	t.Fatal("no graph loaded")
	76	+	}
	77	+	for i := range 20 {
	78	+	if err := repo.Refresh(); err != nil {
	79	+	t.Fatal(err)
	80	+	}
	81	+	if repo.graph.Load() != first {
	82	+	t.Fatalf("Refresh %d reloaded an unchanged graph", i)
	83	+	}
	84	+	}
	85	+	}
	86	+
	87	+	// Refresh once a second on a graph written just now: the graph is reloaded
	88	+	// while its modification time is within the racy window of the stamp, and
	89	+	// never again after that. A fake clock stands in for the waiting.
	90	+	func TestGraphRefreshRacyWindow(t *testing.T) {
	91	+	r := newGTRepo(t)
	92	+	r.commit("a", "a\n")
	93	+	r.git("commit-graph", "write", "--reachable", "--split")
	94	+	start := time.Now()
	95	+	var elapsed atomic.Int64
	96	+	repo, err := Open(r.dir)
	97	+	if err != nil {
	98	+	t.Fatal(err)
	99	+	}
	100	+	defer func() { _ = repo.Close() }()
	101	+	repo.clock = func() time.Time { return start.Add(time.Duration(elapsed.Load())) }
	102	+	if err := repo.Refresh(); err != nil { // stamps the graph at "start"
	103	+	t.Fatal(err)
	104	+	}
	105	+	reloads := 0
	106	+	last := repo.graph.Load()
	107	+	for sec := 1; sec <= 12; sec++ {
	108	+	elapsed.Store(int64(sec) * int64(time.Second))
	109	+	if err := repo.Refresh(); err != nil {
	110	+	t.Fatal(err)
	111	+	}
	112	+	cur := repo.graph.Load()
	113	+	if cur != last {
	114	+	reloads++
	115	+	last = cur
	116	+	if sec > 4 {
	117	+	t.Errorf("Refresh %d s after the write still reloads the graph", sec)
	118	+	}
	119	+	}
	120	+	}
	121	+	if reloads == 0 {
	122	+	t.Error("a graph inside the racy window was trusted")
	123	+	}
	124	+	t.Logf("reloads: %d in 12 refreshes, 1 s apart", reloads)
	125	+	}
	126	+
	127	+	// A graph rewritten the way git does it (new file renamed over the old) is
	128	+	// always seen, trusted or not.
	129	+	func TestGraphRefreshSeesARewrite(t *testing.T) {
	130	+	r := newGTRepo(t)
	131	+	r.commit("a", "a\n")
	132	+	r.git("commit-graph", "write", "--reachable")
	133	+	ageGraph(t, r, time.Hour)
	134	+	repo, err := Open(r.dir)
	135	+	if err != nil {
	136	+	t.Fatal(err)
	137	+	}
	138	+	defer func() { _ = repo.Close() }()
	139	+	first := repo.graph.Load()
	140	+	for range 3 {
	141	+	if err := repo.Refresh(); err != nil {
	142	+	t.Fatal(err)
	143	+	}
	144	+	}
	145	+	if repo.graph.Load() != first {
	146	+	t.Fatal("unchanged graph reloaded")
	147	+	}
	148	+	n := first.g.numCommits()
	149	+	r.commit("b", "b\n")
	150	+	r.git("commit-graph", "write", "--reachable")
	151	+	if err := repo.Refresh(); err != nil {
	152	+	t.Fatal(err)
	153	+	}
	154	+	next := repo.graph.Load()
	155	+	if next == first \|\| next.g == nil \|\| next.g.numCommits() != n+1 {
	156	+	t.Fatalf("rewrite not seen: reloaded %v", next != first)
	157	+	}
	158	+	}

@@ -0,0 +1,158 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package git
	4	+
	5	+	import (
	6	+	"os"
	7	+	"path/filepath"
	8	+	"sync/atomic"
	9	+	"testing"
	10	+	"time"
	11	+	)
	12	+
	13	+	func TestGraphStampTrusted(t *testing.T) {
	14	+	const sec = int64(time.Second)
	15	+	mt := 1_000 * sec
	16	+	win := int64(racyWindow)
	17	+	exists := func(m int64) graphFileStamp { return graphFileStamp{exists: true, mtime: m} }
	18	+	for _, c := range []struct {
	19	+	name string
	20	+	stamp graphStamp
	21	+	at int64
	22	+	want bool
	23	+	}{
	24	+	{"no graph", graphStamp{}, mt, true},
	25	+	{"stamped inside the window", graphStamp{single: exists(mt)}, mt + win, false},
	26	+	{"stamped just after the window", graphStamp{single: exists(mt)}, mt + win + 1, true},
	27	+	{"newest of two decides", graphStamp{single: exists(mt - 100*sec), chain: exists(mt)}, mt + sec, false},
	28	+	{"both old", graphStamp{single: exists(mt - 100sec), chain: exists(mt - 50sec)}, mt, true},
	29	+	{"mtime in the future", graphStamp{chain: exists(mt + 10*sec)}, mt, false},
	30	+	} {
	31	+	if got := c.stamp.trusted(c.at); got != c.want {
	32	+	t.Errorf("%s: trusted = %v, want %v", c.name, got, c.want)
	33	+	}
	34	+	}
	35	+	}
	36	+
	37	+	func graphFiles(r *gtRepo) []string {
	38	+	info := filepath.Join(r.objects(), "info")
	39	+	return []string{filepath.Join(info, "commit-graph"), filepath.Join(info, "commit-graphs", "commit-graph-chain")}
	40	+	}
	41	+
	42	+	func ageGraph(t testing.T, r gtRepo, age time.Duration) {
	43	+	t.Helper()
	44	+	mt := time.Now().Add(-age)
	45	+	n := 0
	46	+	for _, p := range graphFiles(r) {
	47	+	if _, err := os.Stat(p); err == nil {
	48	+	n++
	49	+	if err := os.Chtimes(p, mt, mt); err != nil {
	50	+	t.Fatal(err)
	51	+	}
	52	+	}
	53	+	}
	54	+	if n == 0 {
	55	+	t.Fatal("no graph file")
	56	+	}
	57	+	}
	58	+
	59	+	// A graph that is older than the racy window is trusted when its stat data
	60	+	// is unchanged, however recently it was loaded: Refresh at the pace of the
	61	+	// adapter (every second) must not reload it, and so not remap it.
	62	+	func TestGraphRefreshDoesNotReloadAnUnchangedGraph(t *testing.T) {
	63	+	r := newGTRepo(t)
	64	+	r.commit("a", "a\n")
	65	+	r.commit("b", "b\n")
	66	+	r.git("commit-graph", "write", "--reachable", "--split")
	67	+	ageGraph(t, r, time.Hour)
	68	+	repo, err := Open(r.dir)
	69	+	if err != nil {
	70	+	t.Fatal(err)
	71	+	}
	72	+	defer func() { _ = repo.Close() }()
	73	+	first := repo.graph.Load()
	74	+	if first == nil \|\| first.g == nil {
	75	+	t.Fatal("no graph loaded")
	76	+	}
	77	+	for i := range 20 {
	78	+	if err := repo.Refresh(); err != nil {
	79	+	t.Fatal(err)
	80	+	}
	81	+	if repo.graph.Load() != first {
	82	+	t.Fatalf("Refresh %d reloaded an unchanged graph", i)
	83	+	}
	84	+	}
	85	+	}
	86	+
	87	+	// Refresh once a second on a graph written just now: the graph is reloaded
	88	+	// while its modification time is within the racy window of the stamp, and
	89	+	// never again after that. A fake clock stands in for the waiting.
	90	+	func TestGraphRefreshRacyWindow(t *testing.T) {
	91	+	r := newGTRepo(t)
	92	+	r.commit("a", "a\n")
	93	+	r.git("commit-graph", "write", "--reachable", "--split")
	94	+	start := time.Now()
	95	+	var elapsed atomic.Int64
	96	+	repo, err := Open(r.dir)
	97	+	if err != nil {
	98	+	t.Fatal(err)
	99	+	}
	100	+	defer func() { _ = repo.Close() }()
	101	+	repo.clock = func() time.Time { return start.Add(time.Duration(elapsed.Load())) }
	102	+	if err := repo.Refresh(); err != nil { // stamps the graph at "start"
	103	+	t.Fatal(err)
	104	+	}
	105	+	reloads := 0
	106	+	last := repo.graph.Load()
	107	+	for sec := 1; sec <= 12; sec++ {
	108	+	elapsed.Store(int64(sec) * int64(time.Second))
	109	+	if err := repo.Refresh(); err != nil {
	110	+	t.Fatal(err)
	111	+	}
	112	+	cur := repo.graph.Load()
	113	+	if cur != last {
	114	+	reloads++
	115	+	last = cur
	116	+	if sec > 4 {
	117	+	t.Errorf("Refresh %d s after the write still reloads the graph", sec)
	118	+	}
	119	+	}
	120	+	}
	121	+	if reloads == 0 {
	122	+	t.Error("a graph inside the racy window was trusted")
	123	+	}
	124	+	t.Logf("reloads: %d in 12 refreshes, 1 s apart", reloads)
	125	+	}
	126	+
	127	+	// A graph rewritten the way git does it (new file renamed over the old) is
	128	+	// always seen, trusted or not.
	129	+	func TestGraphRefreshSeesARewrite(t *testing.T) {
	130	+	r := newGTRepo(t)
	131	+	r.commit("a", "a\n")
	132	+	r.git("commit-graph", "write", "--reachable")
	133	+	ageGraph(t, r, time.Hour)
	134	+	repo, err := Open(r.dir)
	135	+	if err != nil {
	136	+	t.Fatal(err)
	137	+	}
	138	+	defer func() { _ = repo.Close() }()
	139	+	first := repo.graph.Load()
	140	+	for range 3 {
	141	+	if err := repo.Refresh(); err != nil {
	142	+	t.Fatal(err)
	143	+	}
	144	+	}
	145	+	if repo.graph.Load() != first {
	146	+	t.Fatal("unchanged graph reloaded")
	147	+	}
	148	+	n := first.g.numCommits()
	149	+	r.commit("b", "b\n")
	150	+	r.git("commit-graph", "write", "--reachable")
	151	+	if err := repo.Refresh(); err != nil {
	152	+	t.Fatal(err)
	153	+	}
	154	+	next := repo.graph.Load()
	155	+	if next == first \|\| next.g == nil \|\| next.g.numCommits() != n+1 {
	156	+	t.Fatalf("rewrite not seen: reloaded %v", next != first)
	157	+	}
	158	+	}

M repo.go +11 -0


@@ -12,6 +12,7 @@ import (
12	12		"sync"
13	13		"sync/atomic"
14	14		"syscall"
	15	+	"time"
15	16		)
16	17
17	18		// Default bounds. See WithMaxObjectSize and WithDeltaCacheSize.

@@ -73,6 +74,16 @@ type Repo struct {
73	74
74	75		graph atomic.Pointer[graphState] // see walk.go
75	76		gmu sync.Mutex // serializes commit-graph reloads
	77	+
	78	+	clock func() time.Time // tests only: time.Now when nil
	79	+	}
	80	+
	81	+	// now is the time the racy-file rules measure from.
	82	+	func (r *Repo) now() time.Time {
	83	+	if r.clock != nil {
	84	+	return r.clock()
	85	+	}
	86	+	return time.Now()
76	87		}
77	88
78	89		// Open opens the repository whose Git directory is dir: a bare repository


@@ -12,6 +12,7 @@ import (
12	"sync"	12		"sync"
13	"sync/atomic"	13		"sync/atomic"
14	"syscall"	14		"syscall"
		15	+	"time"
15	)	16		)
16		17
17	// Default bounds. See WithMaxObjectSize and WithDeltaCacheSize.	18		// Default bounds. See WithMaxObjectSize and WithDeltaCacheSize.

@@ -73,6 +74,16 @@ type Repo struct {
73		74
74	graph atomic.Pointer[graphState] // see walk.go	75		graph atomic.Pointer[graphState] // see walk.go
75	gmu sync.Mutex // serializes commit-graph reloads	76		gmu sync.Mutex // serializes commit-graph reloads
		77	+
		78	+	clock func() time.Time // tests only: time.Now when nil
		79	+	}
		80	+
		81	+	// now is the time the racy-file rules measure from.
		82	+	func (r *Repo) now() time.Time {
		83	+	if r.clock != nil {
		84	+	return r.clock()
		85	+	}
		86	+	return time.Now()
76	}	87		}
77		88
78	// Open opens the repository whose Git directory is dir: a bare repository	89		// Open opens the repository whose Git directory is dir: a bare repository

M walk.go +6 -4


@@ -9,7 +9,6 @@ import (
9	9		"iter"
10	10		"maps"
11	11		"slices"
12		-	"time"
13	12		)
14	13
15	14		// This file implements history walks that match "git log" exactly.

@@ -467,7 +466,7 @@ type graphState struct {
467	466		g *commitGraph // owner's reference; nil when there is none
468	467		stamp graphStamp
469	468		err error // corruption found when opening, reported by walks
470		-	at time.Time
	469	+	at int64 // unix nanoseconds: when the stamp was taken, before the load
471	470		}
472	471
473	472		// acquireGraph returns the current commit-graph with a reference the caller

@@ -508,11 +507,14 @@ func (r *Repo) refreshGraph(force bool) error {
508	507		return ErrClosed
509	508		}
510	509		objects := r.dir + "/objects"
	510	+	// Take the time before the stat data and the content, so that a write
	511	+	// that follows is never mistaken for the version that was loaded.
	512	+	at := r.now().UnixNano()
511	513		stamp := statGraph(objects)
512		-	if cur != nil && !force && cur.stamp == stamp && time.Since(cur.at) > racyWindow {
	514	+	if cur != nil && !force && cur.stamp == stamp && cur.stamp.trusted(cur.at) {
513	515		return nil
514	516		}
515		-	st := &graphState{stamp: stamp, at: time.Now()}
	517	+	st := &graphState{stamp: stamp, at: at}
516	518		g, err := openCommitGraph(objects)
517	519		switch {
518	520		case err == nil:

 	"iter"
 	"maps"
 	"slices"
-	"time"
 )
 // This file implements history walks that match "git log" exactly.
 	g     *commitGraph // owner's reference; nil when there is none
 	stamp graphStamp
 	err   error // corruption found when opening, reported by walks
-	at    time.Time
+	at    int64 // unix nanoseconds: when the stamp was taken, before the load
 }
 // acquireGraph returns the current commit-graph with a reference the caller
 		return ErrClosed
 	}
 	objects := r.dir + "/objects"
+	// Take the time before the stat data and the content, so that a write
+	// that follows is never mistaken for the version that was loaded.
+	at := r.now().UnixNano()
 	stamp := statGraph(objects)
-	if cur != nil && !force && cur.stamp == stamp && time.Since(cur.at) > racyWindow {
+	if cur != nil && !force && cur.stamp == stamp && cur.stamp.trusted(cur.at) {
 		return nil
 	}
-	st := &graphState{stamp: stamp, at: time.Now()}
+	st := &graphState{stamp: stamp, at: at}
 	g, err := openCommitGraph(objects)
 	switch {
 	case err == nil: