detached 1401d80

refactor(recovery): fence snapshot GC per incarnation

- Replace the whole-pass GC fence and its cancel-and-retry preemption
  with one fenced step per incarnation. Each step re-reads the
  catalogue under mutationMu, so a durable mutation waits for at most
  one incarnation and a busy daemon still completes every pass.
- Split ports.SnapshotRepository.CollectGarbage into
  SnapshotIncarnations and CollectIncarnationGarbage.
- Reconcile HEAD before pruning generations, and restore a missing
  HEAD to the committed checkpoint.
- Replace WithDurableMaintenance with WithSnapshotGarbageCollection and
  inline the daemon construct-then-listen startup helper.
- Read snapshot files with bytes.Buffer.ReadFrom under an explicit
  limit and share the hardened directory open.
- Cover every mutation entry point, HEAD reconciliation, and read
  bounds/allocations in table-driven tests; drop sleeps via synctest.
commit
1401d80c31c6095a6ac000c5f33697e17ab5a22d
author
brice · · 2026-10-02 07:50 UTC
parent
b94f7d6
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
26 files · +523 -464
Unified
UnifiedSplit
M internal/adapters/snapshot/atomic.go +11 -24
@@ -1,6 +1,7 @@
11 package snapshot
22
33 import (
4+ "bytes"
45 "errors"
56 "fmt"
67 "io"
@@ -342,7 +343,7 @@ func readBoundedFile(f *os.File) ([]byte, error) {
342343 _ = f.Close()
343344 return nil, fmt.Errorf("snapshot file too large")
344345 }
345- data, readErr := readSizedFile(f, int(stat.Size))
346+ data, readErr := readSizedFile(f, int(stat.Size), maxRepositoryRead)
346347 closeErr := f.Close()
347348 if readErr != nil {
348349 return nil, readErr
@@ -356,28 +357,14 @@ func readBoundedFile(f *os.File) ([]byte, error) {
356357 return data, nil
357358 }
358359
359-// readSizedFile reads f into a buffer sized from fstat, avoiding the repeated
360-// grow-and-copy of io.ReadAll for multi-megabyte snapshot objects. One spare
361-// byte detects a file that grew after fstat; the caller still enforces the
362-// read bound on the result.
363-func readSizedFile(f *os.File, size int) ([]byte, error) {
364- data := make([]byte, 0, size+1)
365- for {
366- if len(data) > maxRepositoryRead {
367- return data, nil
368- }
369- if len(data) == cap(data) {
370- data = append(data, 0)[:len(data)]
371- }
372- // Never read past the bound plus the one detection byte, even when
373- // append grew the buffer further.
374- n, err := f.Read(data[len(data):min(cap(data), maxRepositoryRead+1)])
375- data = data[:len(data)+n]
376- if errors.Is(err, io.EOF) {
377- return data, nil
378- }
379- if err != nil {
380- return nil, err
381- }
360+// readSizedFile reads at most limit+1 bytes of f into a buffer sized from
361+// fstat, avoiding the repeated grow-and-copy of io.ReadAll for multi-megabyte
362+// snapshot objects. The extra byte lets the caller detect a file that grew
363+// past limit after fstat.
364+func readSizedFile(f io.Reader, size, limit int) ([]byte, error) {
365+ buf := bytes.NewBuffer(make([]byte, 0, min(size, limit)+bytes.MinRead))
366+ if _, err := buf.ReadFrom(io.LimitReader(f, int64(limit)+1)); err != nil {
367+ return nil, err
382368 }
369+ return buf.Bytes(), nil
383370 }
@@ -1,6 +1,7 @@
1 package snapshot1 package snapshot
2 2
3 import (3 import (
4+ "bytes"
4 "errors"5 "errors"
5 "fmt"6 "fmt"
6 "io"7 "io"
@@ -342,7 +343,7 @@ func readBoundedFile(f *os.File) ([]byte, error) {
342 _ = f.Close()343 _ = f.Close()
343 return nil, fmt.Errorf("snapshot file too large")344 return nil, fmt.Errorf("snapshot file too large")
344 }345 }
345- data, readErr := readSizedFile(f, int(stat.Size))346+ data, readErr := readSizedFile(f, int(stat.Size), maxRepositoryRead)
346 closeErr := f.Close()347 closeErr := f.Close()
347 if readErr != nil {348 if readErr != nil {
348 return nil, readErr349 return nil, readErr
@@ -356,28 +357,14 @@ func readBoundedFile(f *os.File) ([]byte, error) {
356 return data, nil357 return data, nil
357 }358 }
358 359
359-// readSizedFile reads f into a buffer sized from fstat, avoiding the repeated360+// readSizedFile reads at most limit+1 bytes of f into a buffer sized from
360-// grow-and-copy of io.ReadAll for multi-megabyte snapshot objects. One spare361+// fstat, avoiding the repeated grow-and-copy of io.ReadAll for multi-megabyte
361-// byte detects a file that grew after fstat; the caller still enforces the362+// snapshot objects. The extra byte lets the caller detect a file that grew
362-// read bound on the result.363+// past limit after fstat.
363-func readSizedFile(f *os.File, size int) ([]byte, error) {364+func readSizedFile(f io.Reader, size, limit int) ([]byte, error) {
364- data := make([]byte, 0, size+1)365+ buf := bytes.NewBuffer(make([]byte, 0, min(size, limit)+bytes.MinRead))
365- for {366+ if _, err := buf.ReadFrom(io.LimitReader(f, int64(limit)+1)); err != nil {
366- if len(data) > maxRepositoryRead {367+ return nil, err
367- return data, nil
368- }
369- if len(data) == cap(data) {
370- data = append(data, 0)[:len(data)]
371- }
372- // Never read past the bound plus the one detection byte, even when
373- // append grew the buffer further.
374- n, err := f.Read(data[len(data):min(cap(data), maxRepositoryRead+1)])
375- data = data[:len(data)+n]
376- if errors.Is(err, io.EOF) {
377- return data, nil
378- }
379- if err != nil {
380- return nil, err
381- }
382 }368 }
369+ return buf.Bytes(), nil
383 }370 }
M internal/adapters/snapshot/maintenance_gc.go +54 -84
@@ -7,84 +7,65 @@ import (
77 "fmt"
88 "os"
99 "path/filepath"
10- "syscall"
1110
1211 "github.com/bnema/vev/internal/domain"
1312 "github.com/bnema/vev/internal/ports"
1413 codec "github.com/bnema/vev/internal/usecase/snapshot"
1514 )
1615
17-// CollectGarbage applies the complete snapshot retention policy. The keep map
18-// must come from a catalogue that loaded and validated successfully: only then
19-// is an incarnation absent from keep known to be an orphan.
20-func (r *Repository) CollectGarbage(ctx context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error {
16+// SnapshotIncarnations lists the canonical incarnation directories.
17+func (r *Repository) SnapshotIncarnations(ctx context.Context) ([]domain.IncarnationID, error) {
2118 if err := ctx.Err(); err != nil {
22- return err
19+ return nil, err
2320 }
24- sessions := filepath.Join(r.dir, repositorySessionsDir)
25- entries, err := r.readGarbageCollectionDirectory(sessions)
21+ entries, err := r.readGarbageCollectionDirectory(filepath.Join(r.dir, repositorySessionsDir))
2622 if errors.Is(err, os.ErrNotExist) {
27- return nil
23+ return nil, nil
2824 }
2925 if err != nil {
30- return fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err))
26+ return nil, fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err))
3127 }
32-
33- var collected error
28+ ids := make([]domain.IncarnationID, 0, len(entries))
3429 for _, entry := range entries {
35- if err := ctx.Err(); err != nil {
36- return errors.Join(collected, err)
37- }
38- if !entry.IsDir() {
39- continue
40- }
4130 var id domain.IncarnationID
42- if err := id.UnmarshalText([]byte(entry.Name())); err != nil {
43- continue
44- }
45- committed, known := keep[id]
46- if !known {
47- key, keyErr := incarnationKey(id)
48- if keyErr != nil {
49- collected = errors.Join(collected, keyErr)
50- continue
51- }
52- lock := r.lockSession(key)
53- _, known = keep[id]
54- if !known {
55- err = r.removeIncarnationLocked(id)
56- }
57- r.unlockSession(lock)
58- if err != nil {
59- collected = errors.Join(collected, fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err)))
60- continue
61- }
62- if !known {
63- r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation")
64- }
65- continue
66- }
67- if err := r.pruneGenerations(ctx, id, committed); err != nil {
68- collected = errors.Join(collected, err)
31+ if entry.IsDir() && id.UnmarshalText([]byte(entry.Name())) == nil {
32+ ids = append(ids, id)
6933 }
7034 }
71- if err := r.syncDirectory(sessions); err != nil {
72- collected = errors.Join(collected, fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err)))
73- }
74- return collected
35+ return ids, nil
7536 }
7637
77-// pruneGenerations keeps the committed generation and its immediate
78-// predecessor. Everything else, including a forward orphan newer than the
79-// catalogue commit, is removed before unreferenced objects are swept.
80-func (r *Repository) pruneGenerations(ctx context.Context, id domain.IncarnationID, committed domain.CheckpointRef) error {
38+// CollectIncarnationGarbage applies the retention policy to one incarnation.
39+// keep must come from a catalogue that loaded and validated successfully: only
40+// then is a nil keep known to mark an orphan.
41+func (r *Repository) CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
42+ if err := ctx.Err(); err != nil {
43+ return err
44+ }
8145 key, err := incarnationKey(id)
8246 if err != nil {
8347 return err
8448 }
8549 lock := r.lockSession(key)
8650 defer r.unlockSession(lock)
51+ if keep == nil {
52+ if err := r.removeIncarnationLocked(id); err != nil {
53+ return fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err))
54+ }
55+ if err := r.syncDirectory(filepath.Join(r.dir, repositorySessionsDir)); err != nil {
56+ return fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err))
57+ }
58+ r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation")
59+ return nil
60+ }
61+ return r.pruneGenerationsLocked(ctx, id, *keep)
62+}
8763
64+// pruneGenerationsLocked keeps the committed generation and its immediate
65+// predecessor. Everything else, including a forward orphan newer than the
66+// catalogue commit, is removed before unreferenced objects are swept. The
67+// caller holds the incarnation's session lock.
68+func (r *Repository) pruneGenerationsLocked(ctx context.Context, id domain.IncarnationID, committed domain.CheckpointRef) error {
8869 generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations)
8970 entries, err := r.readGarbageCollectionDirectory(generationsDir)
9071 if errors.Is(err, os.ErrNotExist) {
@@ -105,12 +86,16 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation
10586 if err != nil {
10687 return fmt.Errorf("mark snapshot objects for %s: %w", id.String(), err)
10788 }
89+ // Reconcile HEAD before removing generations, so an interrupted pass never
90+ // leaves HEAD naming a removed manifest.
91+ if err := r.reconcileHeadForRetention(id, kept, committed); err != nil {
92+ return err
93+ }
10894
95+ // Generation removal runs to completion: it is bounded by the retained
96+ // window plus any backlog, and a half-pruned directory buys nothing.
10997 removed := 0
11098 for _, entry := range entries {
111- if err := ctx.Err(); err != nil {
112- return err
113- }
11499 generation, canonical := parseGenerationFilename(entry.Name())
115100 if !canonical || entry.IsDir() {
116101 continue
@@ -129,36 +114,24 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation
129114 }
130115 r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-generations", "removed", removed)
131116 }
132- if err := r.removeHeadForDiscardedGenerations(id, kept, committed); err != nil {
133- return err
134- }
135117 return r.sweepUnreferencedObjects(ctx, id, references)
136118 }
137119
138-// removeHeadForDiscardedGenerations removes a repository HEAD only when its
139-// generation was removed by the retention policy. In particular, an
140-// incarnation with no committed checkpoint retains no generations, so its
141-// crash-published HEAD cannot make a retry conflict with an orphan. When a
142-// committed checkpoint exists, HEAD is instead rewound to it: GC also runs
143-// while the daemon serves, and a live session's next publication must still
144-// find its committed parent.
145-func (r *Repository) removeHeadForDiscardedGenerations(id domain.IncarnationID, kept map[uint64]struct{}, committed domain.CheckpointRef) error {
146- removeHead := len(kept) == 0
147- if !removeHead {
120+// reconcileHeadForRetention makes HEAD agree with the retained generations.
121+// With a committed checkpoint, a missing HEAD or one naming a discarded
122+// generation (such as a forward orphan) is rewound to the commit: GC runs while
123+// the daemon serves, and a live session's next publication must find its
124+// committed parent. Without one, no generation is retained and HEAD is removed
125+// so a crash-published HEAD cannot make a retry conflict with an orphan.
126+func (r *Repository) reconcileHeadForRetention(id domain.IncarnationID, kept map[uint64]struct{}, committed domain.CheckpointRef) error {
127+ if committed.Generation > 0 {
148128 generation, _, err := r.readHead(id)
149- if errors.Is(err, os.ErrNotExist) {
150- return nil
151- }
152- if err != nil {
129+ if err != nil && !errors.Is(err, os.ErrNotExist) {
153130 return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))
154131 }
155- _, retained := kept[generation]
156- removeHead = !retained
157- }
158- if !removeHead {
159- return nil
160- }
161- if committed.Generation > 0 {
132+ if _, retained := kept[generation]; err == nil && retained {
133+ return nil
134+ }
162135 // referencesForGenerations already validated the committed manifest digest.
163136 if err := r.writeMutable(r.headPath(id), marshalHead(committed.Generation, committed.ManifestDigest)); err != nil {
164137 return fmt.Errorf("rewind snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))
@@ -247,10 +220,7 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D
247220 // sweep over thousands of objects does not reopen and revalidate the root for
248221 // every shard.
249222 func readRootDirectory(root *os.Root, rel string) ([]os.DirEntry, error) {
250- if err := rejectFinalSymlink(root, rel); err != nil {
251- return nil, err
252- }
253- directory, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
223+ directory, err := openRootDirectory(root, rel)
254224 if err != nil {
255225 return nil, err
256226 }
@@ -7,84 +7,65 @@ import (
7 "fmt"7 "fmt"
8 "os"8 "os"
9 "path/filepath"9 "path/filepath"
10- "syscall"
11 10
12 "github.com/bnema/vev/internal/domain"11 "github.com/bnema/vev/internal/domain"
13 "github.com/bnema/vev/internal/ports"12 "github.com/bnema/vev/internal/ports"
14 codec "github.com/bnema/vev/internal/usecase/snapshot"13 codec "github.com/bnema/vev/internal/usecase/snapshot"
15 )14 )
16 15
17-// CollectGarbage applies the complete snapshot retention policy. The keep map16+// SnapshotIncarnations lists the canonical incarnation directories.
18-// must come from a catalogue that loaded and validated successfully: only then17+func (r *Repository) SnapshotIncarnations(ctx context.Context) ([]domain.IncarnationID, error) {
19-// is an incarnation absent from keep known to be an orphan.
20-func (r *Repository) CollectGarbage(ctx context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error {
21 if err := ctx.Err(); err != nil {18 if err := ctx.Err(); err != nil {
22- return err19+ return nil, err
23 }20 }
24- sessions := filepath.Join(r.dir, repositorySessionsDir)21+ entries, err := r.readGarbageCollectionDirectory(filepath.Join(r.dir, repositorySessionsDir))
25- entries, err := r.readGarbageCollectionDirectory(sessions)
26 if errors.Is(err, os.ErrNotExist) {22 if errors.Is(err, os.ErrNotExist) {
27- return nil23+ return nil, nil
28 }24 }
29 if err != nil {25 if err != nil {
30- return fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err))26+ return nil, fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err))
31 }27 }
32-28+ ids := make([]domain.IncarnationID, 0, len(entries))
33- var collected error
34 for _, entry := range entries {29 for _, entry := range entries {
35- if err := ctx.Err(); err != nil {
36- return errors.Join(collected, err)
37- }
38- if !entry.IsDir() {
39- continue
40- }
41 var id domain.IncarnationID30 var id domain.IncarnationID
42- if err := id.UnmarshalText([]byte(entry.Name())); err != nil {31+ if entry.IsDir() && id.UnmarshalText([]byte(entry.Name())) == nil {
43- continue32+ ids = append(ids, id)
44- }
45- committed, known := keep[id]
46- if !known {
47- key, keyErr := incarnationKey(id)
48- if keyErr != nil {
49- collected = errors.Join(collected, keyErr)
50- continue
51- }
52- lock := r.lockSession(key)
53- _, known = keep[id]
54- if !known {
55- err = r.removeIncarnationLocked(id)
56- }
57- r.unlockSession(lock)
58- if err != nil {
59- collected = errors.Join(collected, fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err)))
60- continue
61- }
62- if !known {
63- r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation")
64- }
65- continue
66- }
67- if err := r.pruneGenerations(ctx, id, committed); err != nil {
68- collected = errors.Join(collected, err)
69 }33 }
70 }34 }
71- if err := r.syncDirectory(sessions); err != nil {35+ return ids, nil
72- collected = errors.Join(collected, fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err)))
73- }
74- return collected
75 }36 }
76 37
77-// pruneGenerations keeps the committed generation and its immediate38+// CollectIncarnationGarbage applies the retention policy to one incarnation.
78-// predecessor. Everything else, including a forward orphan newer than the39+// keep must come from a catalogue that loaded and validated successfully: only
79-// catalogue commit, is removed before unreferenced objects are swept.40+// then is a nil keep known to mark an orphan.
80-func (r *Repository) pruneGenerations(ctx context.Context, id domain.IncarnationID, committed domain.CheckpointRef) error {41+func (r *Repository) CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
42+ if err := ctx.Err(); err != nil {
43+ return err
44+ }
81 key, err := incarnationKey(id)45 key, err := incarnationKey(id)
82 if err != nil {46 if err != nil {
83 return err47 return err
84 }48 }
85 lock := r.lockSession(key)49 lock := r.lockSession(key)
86 defer r.unlockSession(lock)50 defer r.unlockSession(lock)
51+ if keep == nil {
52+ if err := r.removeIncarnationLocked(id); err != nil {
53+ return fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err))
54+ }
55+ if err := r.syncDirectory(filepath.Join(r.dir, repositorySessionsDir)); err != nil {
56+ return fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err))
57+ }
58+ r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation")
59+ return nil
60+ }
61+ return r.pruneGenerationsLocked(ctx, id, *keep)
62+}
87 63
64+// pruneGenerationsLocked keeps the committed generation and its immediate
65+// predecessor. Everything else, including a forward orphan newer than the
66+// catalogue commit, is removed before unreferenced objects are swept. The
67+// caller holds the incarnation's session lock.
68+func (r *Repository) pruneGenerationsLocked(ctx context.Context, id domain.IncarnationID, committed domain.CheckpointRef) error {
88 generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations)69 generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations)
89 entries, err := r.readGarbageCollectionDirectory(generationsDir)70 entries, err := r.readGarbageCollectionDirectory(generationsDir)
90 if errors.Is(err, os.ErrNotExist) {71 if errors.Is(err, os.ErrNotExist) {
@@ -105,12 +86,16 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation
105 if err != nil {86 if err != nil {
106 return fmt.Errorf("mark snapshot objects for %s: %w", id.String(), err)87 return fmt.Errorf("mark snapshot objects for %s: %w", id.String(), err)
107 }88 }
89+ // Reconcile HEAD before removing generations, so an interrupted pass never
90+ // leaves HEAD naming a removed manifest.
91+ if err := r.reconcileHeadForRetention(id, kept, committed); err != nil {
92+ return err
93+ }
108 94
95+ // Generation removal runs to completion: it is bounded by the retained
96+ // window plus any backlog, and a half-pruned directory buys nothing.
109 removed := 097 removed := 0
110 for _, entry := range entries {98 for _, entry := range entries {
111- if err := ctx.Err(); err != nil {
112- return err
113- }
114 generation, canonical := parseGenerationFilename(entry.Name())99 generation, canonical := parseGenerationFilename(entry.Name())
115 if !canonical || entry.IsDir() {100 if !canonical || entry.IsDir() {
116 continue101 continue
@@ -129,36 +114,24 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation
129 }114 }
130 r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-generations", "removed", removed)115 r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-generations", "removed", removed)
131 }116 }
132- if err := r.removeHeadForDiscardedGenerations(id, kept, committed); err != nil {
133- return err
134- }
135 return r.sweepUnreferencedObjects(ctx, id, references)117 return r.sweepUnreferencedObjects(ctx, id, references)
136 }118 }
137 119
138-// removeHeadForDiscardedGenerations removes a repository HEAD only when its120+// reconcileHeadForRetention makes HEAD agree with the retained generations.
139-// generation was removed by the retention policy. In particular, an121+// With a committed checkpoint, a missing HEAD or one naming a discarded
140-// incarnation with no committed checkpoint retains no generations, so its122+// generation (such as a forward orphan) is rewound to the commit: GC runs while
141-// crash-published HEAD cannot make a retry conflict with an orphan. When a123+// the daemon serves, and a live session's next publication must find its
142-// committed checkpoint exists, HEAD is instead rewound to it: GC also runs124+// committed parent. Without one, no generation is retained and HEAD is removed
143-// while the daemon serves, and a live session's next publication must still125+// so a crash-published HEAD cannot make a retry conflict with an orphan.
144-// find its committed parent.126+func (r *Repository) reconcileHeadForRetention(id domain.IncarnationID, kept map[uint64]struct{}, committed domain.CheckpointRef) error {
145-func (r *Repository) removeHeadForDiscardedGenerations(id domain.IncarnationID, kept map[uint64]struct{}, committed domain.CheckpointRef) error {127+ if committed.Generation > 0 {
146- removeHead := len(kept) == 0
147- if !removeHead {
148 generation, _, err := r.readHead(id)128 generation, _, err := r.readHead(id)
149- if errors.Is(err, os.ErrNotExist) {129+ if err != nil && !errors.Is(err, os.ErrNotExist) {
150- return nil
151- }
152- if err != nil {
153 return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))130 return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))
154 }131 }
155- _, retained := kept[generation]132+ if _, retained := kept[generation]; err == nil && retained {
156- removeHead = !retained133+ return nil
157- }134+ }
158- if !removeHead {
159- return nil
160- }
161- if committed.Generation > 0 {
162 // referencesForGenerations already validated the committed manifest digest.135 // referencesForGenerations already validated the committed manifest digest.
163 if err := r.writeMutable(r.headPath(id), marshalHead(committed.Generation, committed.ManifestDigest)); err != nil {136 if err := r.writeMutable(r.headPath(id), marshalHead(committed.Generation, committed.ManifestDigest)); err != nil {
164 return fmt.Errorf("rewind snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))137 return fmt.Errorf("rewind snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err))
@@ -247,10 +220,7 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D
247 // sweep over thousands of objects does not reopen and revalidate the root for220 // sweep over thousands of objects does not reopen and revalidate the root for
248 // every shard.221 // every shard.
249 func readRootDirectory(root *os.Root, rel string) ([]os.DirEntry, error) {222 func readRootDirectory(root *os.Root, rel string) ([]os.DirEntry, error) {
250- if err := rejectFinalSymlink(root, rel); err != nil {223+ directory, err := openRootDirectory(root, rel)
251- return nil, err
252- }
253- directory, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
254 if err != nil {224 if err != nil {
255 return nil, err225 return nil, err
256 }226 }
M internal/adapters/snapshot/maintenance_gc_test.go +38 -18
@@ -20,9 +20,9 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) {
2020 require.NoError(t, repository.Publish(context.Background(), publication))
2121
2222 // Repository publication completed, but the catalogue commit did not.
23- // Startup GC knows the incarnation but has no committed checkpoint for it.
23+ // GC knows the incarnation but has no committed checkpoint for it.
2424 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {}}
25- require.NoError(t, repository.CollectGarbage(context.Background(), keep))
25+ require.NoError(t, collectAll(context.Background(), repository, keep))
2626 _, err := os.Stat(repository.headPath(publication.IncarnationID))
2727 require.ErrorIs(t, err, os.ErrNotExist)
2828 require.Empty(t, garbageCollectionGenerations(t, repository, publication.IncarnationID))
@@ -32,27 +32,47 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) {
3232 require.NoError(t, repository.Publish(context.Background(), publication))
3333 committed := domain.CheckpointRef{Generation: publication.Generation, ManifestDigest: sha256.Sum256(publication.Manifest)}
3434 keep[publication.IncarnationID] = committed
35- require.NoError(t, repository.CollectGarbage(context.Background(), keep))
35+ require.NoError(t, collectAll(context.Background(), repository, keep))
3636 _, err = repository.LoadCheckpoint(context.Background(), publication.IncarnationID, publication.Name, committed)
3737 require.NoError(t, err)
3838 }
3939
40-func TestCollectGarbageRewindsForwardOrphanHeadForLivePublication(t *testing.T) {
40+func TestCollectIncarnationGarbageKeepsHeadPublishable(t *testing.T) {
4141 t.Parallel()
42- repository := NewRepository(privateDir(t))
43- // Generation 4 reached the repository but its catalogue commit failed.
44- publications := publishGarbageCollectionGenerations(t, repository, "work", 4)
45- id := publications[0].IncarnationID
46- committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)}
42+ tests := []struct {
43+ name string
44+ prepare func(t *testing.T, repository *Repository, id domain.IncarnationID)
45+ }{
46+ // Generation 4 reached the repository but its catalogue commit failed.
47+ {name: "forward orphan HEAD is rewound", prepare: func(*testing.T, *Repository, domain.IncarnationID) {}},
48+ {name: "missing HEAD is restored", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) {
49+ require.NoError(t, os.Remove(repository.headPath(id)))
50+ }},
51+ // An interrupted earlier pass must never leave HEAD naming a removed
52+ // manifest: HEAD is reconciled before generations are removed.
53+ {name: "HEAD naming a removed generation is rewound", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) {
54+ require.NoError(t, os.Remove(repository.manifestPath(id, 4)))
55+ }},
56+ }
57+ for _, tt := range tests {
58+ t.Run(tt.name, func(t *testing.T) {
59+ t.Parallel()
60+ repository := NewRepository(privateDir(t))
61+ publications := publishGarbageCollectionGenerations(t, repository, "work", 4)
62+ id := publications[0].IncarnationID
63+ committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)}
64+ tt.prepare(t, repository, id)
4765
48- require.NoError(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed}))
66+ require.NoError(t, repository.CollectIncarnationGarbage(t.Context(), id, &committed))
4967
50- generation, digest, err := repository.readHead(id)
51- require.NoError(t, err)
52- require.Equal(t, committed.Generation, generation)
53- require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest)
54- // The live session retries generation 4 against its committed parent.
55- require.NoError(t, repository.Publish(t.Context(), publications[3]))
68+ generation, digest, err := repository.readHead(id)
69+ require.NoError(t, err)
70+ require.Equal(t, committed.Generation, generation)
71+ require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest)
72+ // The live session retries generation 4 against its committed parent.
73+ require.NoError(t, repository.Publish(t.Context(), publications[3]))
74+ })
75+ }
5676 }
5777
5878 func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) {
@@ -92,7 +112,7 @@ func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) {
92112 committed = tt.mutate(t, repository, id, committed)
93113 before := snapshotRepositoryFiles(t, repository.dir)
94114
95- require.Error(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed}))
115+ require.Error(t, collectAll(t.Context(), repository, map[domain.IncarnationID]domain.CheckpointRef{id: committed}))
96116 require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed")
97117 })
98118 }
@@ -121,7 +141,7 @@ func TestCollectGarbage(t *testing.T) {
121141 keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)}
122142 }
123143
124- require.NoError(t, repository.CollectGarbage(context.Background(), keep))
144+ require.NoError(t, collectAll(context.Background(), repository, keep))
125145 if !tt.known {
126146 _, err := os.Stat(repository.sessionPath(id))
127147 require.ErrorIs(t, err, os.ErrNotExist)
@@ -20,9 +20,9 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) {
20 require.NoError(t, repository.Publish(context.Background(), publication))20 require.NoError(t, repository.Publish(context.Background(), publication))
21 21
22 // Repository publication completed, but the catalogue commit did not.22 // Repository publication completed, but the catalogue commit did not.
23- // Startup GC knows the incarnation but has no committed checkpoint for it.23+ // GC knows the incarnation but has no committed checkpoint for it.
24 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {}}24 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {}}
25- require.NoError(t, repository.CollectGarbage(context.Background(), keep))25+ require.NoError(t, collectAll(context.Background(), repository, keep))
26 _, err := os.Stat(repository.headPath(publication.IncarnationID))26 _, err := os.Stat(repository.headPath(publication.IncarnationID))
27 require.ErrorIs(t, err, os.ErrNotExist)27 require.ErrorIs(t, err, os.ErrNotExist)
28 require.Empty(t, garbageCollectionGenerations(t, repository, publication.IncarnationID))28 require.Empty(t, garbageCollectionGenerations(t, repository, publication.IncarnationID))
@@ -32,27 +32,47 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) {
32 require.NoError(t, repository.Publish(context.Background(), publication))32 require.NoError(t, repository.Publish(context.Background(), publication))
33 committed := domain.CheckpointRef{Generation: publication.Generation, ManifestDigest: sha256.Sum256(publication.Manifest)}33 committed := domain.CheckpointRef{Generation: publication.Generation, ManifestDigest: sha256.Sum256(publication.Manifest)}
34 keep[publication.IncarnationID] = committed34 keep[publication.IncarnationID] = committed
35- require.NoError(t, repository.CollectGarbage(context.Background(), keep))35+ require.NoError(t, collectAll(context.Background(), repository, keep))
36 _, err = repository.LoadCheckpoint(context.Background(), publication.IncarnationID, publication.Name, committed)36 _, err = repository.LoadCheckpoint(context.Background(), publication.IncarnationID, publication.Name, committed)
37 require.NoError(t, err)37 require.NoError(t, err)
38 }38 }
39 39
40-func TestCollectGarbageRewindsForwardOrphanHeadForLivePublication(t *testing.T) {40+func TestCollectIncarnationGarbageKeepsHeadPublishable(t *testing.T) {
41 t.Parallel()41 t.Parallel()
42- repository := NewRepository(privateDir(t))42+ tests := []struct {
43- // Generation 4 reached the repository but its catalogue commit failed.43+ name string
44- publications := publishGarbageCollectionGenerations(t, repository, "work", 4)44+ prepare func(t *testing.T, repository *Repository, id domain.IncarnationID)
45- id := publications[0].IncarnationID45+ }{
46- committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)}46+ // Generation 4 reached the repository but its catalogue commit failed.
47+ {name: "forward orphan HEAD is rewound", prepare: func(*testing.T, *Repository, domain.IncarnationID) {}},
48+ {name: "missing HEAD is restored", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) {
49+ require.NoError(t, os.Remove(repository.headPath(id)))
50+ }},
51+ // An interrupted earlier pass must never leave HEAD naming a removed
52+ // manifest: HEAD is reconciled before generations are removed.
53+ {name: "HEAD naming a removed generation is rewound", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) {
54+ require.NoError(t, os.Remove(repository.manifestPath(id, 4)))
55+ }},
56+ }
57+ for _, tt := range tests {
58+ t.Run(tt.name, func(t *testing.T) {
59+ t.Parallel()
60+ repository := NewRepository(privateDir(t))
61+ publications := publishGarbageCollectionGenerations(t, repository, "work", 4)
62+ id := publications[0].IncarnationID
63+ committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)}
64+ tt.prepare(t, repository, id)
47 65
48- require.NoError(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed}))66+ require.NoError(t, repository.CollectIncarnationGarbage(t.Context(), id, &committed))
49 67
50- generation, digest, err := repository.readHead(id)68+ generation, digest, err := repository.readHead(id)
51- require.NoError(t, err)69+ require.NoError(t, err)
52- require.Equal(t, committed.Generation, generation)70+ require.Equal(t, committed.Generation, generation)
53- require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest)71+ require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest)
54- // The live session retries generation 4 against its committed parent.72+ // The live session retries generation 4 against its committed parent.
55- require.NoError(t, repository.Publish(t.Context(), publications[3]))73+ require.NoError(t, repository.Publish(t.Context(), publications[3]))
74+ })
75+ }
56 }76 }
57 77
58 func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) {78 func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) {
@@ -92,7 +112,7 @@ func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) {
92 committed = tt.mutate(t, repository, id, committed)112 committed = tt.mutate(t, repository, id, committed)
93 before := snapshotRepositoryFiles(t, repository.dir)113 before := snapshotRepositoryFiles(t, repository.dir)
94 114
95- require.Error(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed}))115+ require.Error(t, collectAll(t.Context(), repository, map[domain.IncarnationID]domain.CheckpointRef{id: committed}))
96 require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed")116 require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed")
97 })117 })
98 }118 }
@@ -121,7 +141,7 @@ func TestCollectGarbage(t *testing.T) {
121 keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)}141 keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)}
122 }142 }
123 143
124- require.NoError(t, repository.CollectGarbage(context.Background(), keep))144+ require.NoError(t, collectAll(context.Background(), repository, keep))
125 if !tt.known {145 if !tt.known {
126 _, err := os.Stat(repository.sessionPath(id))146 _, err := os.Stat(repository.sessionPath(id))
127 require.ErrorIs(t, err, os.ErrNotExist)147 require.ErrorIs(t, err, os.ErrNotExist)
M internal/adapters/snapshot/path_security_test.go +4 -4
@@ -45,8 +45,8 @@ func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
4545 t.Fatal("LoadCheckpoint succeeded through replaced configured-root symlink")
4646 }
4747 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}
48- if err := repo.CollectGarbage(context.Background(), keep); err == nil {
49- t.Fatal("CollectGarbage succeeded through replaced configured-root symlink")
48+ if err := collectAll(context.Background(), repo, keep); err == nil {
49+ t.Fatal("GC succeeded through replaced configured-root symlink")
5050 }
5151 loaded, err := loadPublication(context.Background(), external, externalPublication)
5252 if err != nil {
@@ -94,8 +94,8 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
9494 t.Fatal("LoadCheckpoint succeeded through symlinked repository component")
9595 }
9696 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}
97- if err := repo.CollectGarbage(context.Background(), keep); err == nil {
98- t.Fatal("CollectGarbage succeeded through symlinked repository component")
97+ if err := collectAll(context.Background(), repo, keep); err == nil {
98+ t.Fatal("GC succeeded through symlinked repository component")
9999 }
100100 got, err := os.ReadFile(guard)
101101 if err != nil || string(got) != "guard" {
@@ -45,8 +45,8 @@ func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
45 t.Fatal("LoadCheckpoint succeeded through replaced configured-root symlink")45 t.Fatal("LoadCheckpoint succeeded through replaced configured-root symlink")
46 }46 }
47 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}47 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}
48- if err := repo.CollectGarbage(context.Background(), keep); err == nil {48+ if err := collectAll(context.Background(), repo, keep); err == nil {
49- t.Fatal("CollectGarbage succeeded through replaced configured-root symlink")49+ t.Fatal("GC succeeded through replaced configured-root symlink")
50 }50 }
51 loaded, err := loadPublication(context.Background(), external, externalPublication)51 loaded, err := loadPublication(context.Background(), external, externalPublication)
52 if err != nil {52 if err != nil {
@@ -94,8 +94,8 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
94 t.Fatal("LoadCheckpoint succeeded through symlinked repository component")94 t.Fatal("LoadCheckpoint succeeded through symlinked repository component")
95 }95 }
96 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}96 keep := map[domain.IncarnationID]domain.CheckpointRef{publication.IncarnationID: {Generation: publication.Generation, ManifestDigest: codec.ManifestDigest(publication.Manifest)}}
97- if err := repo.CollectGarbage(context.Background(), keep); err == nil {97+ if err := collectAll(context.Background(), repo, keep); err == nil {
98- t.Fatal("CollectGarbage succeeded through symlinked repository component")98+ t.Fatal("GC succeeded through symlinked repository component")
99 }99 }
100 got, err := os.ReadFile(guard)100 got, err := os.ReadFile(guard)
101 if err != nil || string(got) != "guard" {101 if err != nil || string(got) != "guard" {
M internal/adapters/snapshot/repository_hardening_test.go +21 -18
@@ -4,6 +4,7 @@ import (
44 "errors"
55 "os"
66 "path/filepath"
7+ "strings"
78 "syscall"
89 "testing"
910
@@ -63,37 +64,39 @@ func TestReadBoundedRejectsUnsafeFiles(t *testing.T) {
6364 }
6465
6566 func TestReadSizedFile(t *testing.T) {
67+ const limit = 16
6668 tests := []struct {
6769 name string
68- content []byte
70+ content string
6971 hintSize int
72+ want string
7073 }{
71- {name: "empty", content: nil, hintSize: 0},
72- {name: "exact size hint", content: []byte("snapshot object"), hintSize: 15},
73- {name: "file grew after stat", content: []byte("snapshot object grew"), hintSize: 4},
74- {name: "file shrank after stat", content: []byte("tiny"), hintSize: 64},
74+ {name: "empty", content: "", hintSize: 0, want: ""},
75+ {name: "exact size hint", content: "snapshot object", hintSize: 15, want: "snapshot object"},
76+ {name: "file grew after stat", content: "snapshot", hintSize: 4, want: "snapshot"},
77+ {name: "file shrank after stat", content: "tiny", hintSize: 64, want: "tiny"},
78+ {name: "file grew past limit stops one byte over", content: "snapshot object grew far", hintSize: 4, want: "snapshot object g"},
7579 }
7680 for _, tt := range tests {
7781 t.Run(tt.name, func(t *testing.T) {
78- path := filepath.Join(t.TempDir(), "object")
79- if err := os.WriteFile(path, tt.content, 0o600); err != nil {
80- t.Fatal(err)
81- }
82- f, err := os.Open(path)
83- if err != nil {
84- t.Fatal(err)
85- }
86- defer func() { _ = f.Close() }()
87-
88- got, err := readSizedFile(f, tt.hintSize)
82+ got, err := readSizedFile(strings.NewReader(tt.content), tt.hintSize, limit)
8983 if err != nil {
9084 t.Fatal(err)
9185 }
92- if string(got) != string(tt.content) {
93- t.Fatalf("readSizedFile = %q, want %q", got, tt.content)
86+ if string(got) != tt.want {
87+ t.Fatalf("readSizedFile = %q, want %q", got, tt.want)
9488 }
9589 })
9690 }
91+
92+ // An exact size hint must cost one buffer, unlike io.ReadAll's growth.
93+ content := strings.Repeat("x", 1<<20)
94+ allocs := testing.AllocsPerRun(10, func() {
95+ _, _ = readSizedFile(strings.NewReader(content), len(content), maxRepositoryRead)
96+ })
97+ if allocs > 3 {
98+ t.Fatalf("readSizedFile allocations = %v, want at most 3", allocs)
99+ }
97100 }
98101
99102 func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) {
@@ -4,6 +4,7 @@ import (
4 "errors"4 "errors"
5 "os"5 "os"
6 "path/filepath"6 "path/filepath"
7+ "strings"
7 "syscall"8 "syscall"
8 "testing"9 "testing"
9 10
@@ -63,37 +64,39 @@ func TestReadBoundedRejectsUnsafeFiles(t *testing.T) {
63 }64 }
64 65
65 func TestReadSizedFile(t *testing.T) {66 func TestReadSizedFile(t *testing.T) {
67+ const limit = 16
66 tests := []struct {68 tests := []struct {
67 name string69 name string
68- content []byte70+ content string
69 hintSize int71 hintSize int
72+ want string
70 }{73 }{
71- {name: "empty", content: nil, hintSize: 0},74+ {name: "empty", content: "", hintSize: 0, want: ""},
72- {name: "exact size hint", content: []byte("snapshot object"), hintSize: 15},75+ {name: "exact size hint", content: "snapshot object", hintSize: 15, want: "snapshot object"},
73- {name: "file grew after stat", content: []byte("snapshot object grew"), hintSize: 4},76+ {name: "file grew after stat", content: "snapshot", hintSize: 4, want: "snapshot"},
74- {name: "file shrank after stat", content: []byte("tiny"), hintSize: 64},77+ {name: "file shrank after stat", content: "tiny", hintSize: 64, want: "tiny"},
78+ {name: "file grew past limit stops one byte over", content: "snapshot object grew far", hintSize: 4, want: "snapshot object g"},
75 }79 }
76 for _, tt := range tests {80 for _, tt := range tests {
77 t.Run(tt.name, func(t *testing.T) {81 t.Run(tt.name, func(t *testing.T) {
78- path := filepath.Join(t.TempDir(), "object")82+ got, err := readSizedFile(strings.NewReader(tt.content), tt.hintSize, limit)
79- if err := os.WriteFile(path, tt.content, 0o600); err != nil {
80- t.Fatal(err)
81- }
82- f, err := os.Open(path)
83- if err != nil {
84- t.Fatal(err)
85- }
86- defer func() { _ = f.Close() }()
87-
88- got, err := readSizedFile(f, tt.hintSize)
89 if err != nil {83 if err != nil {
90 t.Fatal(err)84 t.Fatal(err)
91 }85 }
92- if string(got) != string(tt.content) {86+ if string(got) != tt.want {
93- t.Fatalf("readSizedFile = %q, want %q", got, tt.content)87+ t.Fatalf("readSizedFile = %q, want %q", got, tt.want)
94 }88 }
95 })89 })
96 }90 }
91+
92+ // An exact size hint must cost one buffer, unlike io.ReadAll's growth.
93+ content := strings.Repeat("x", 1<<20)
94+ allocs := testing.AllocsPerRun(10, func() {
95+ _, _ = readSizedFile(strings.NewReader(content), len(content), maxRepositoryRead)
96+ })
97+ if allocs > 3 {
98+ t.Fatalf("readSizedFile allocations = %v, want at most 3", allocs)
99+ }
97 }100 }
98 101
99 func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) {102 func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) {
M internal/adapters/snapshot/safe_path.go +5 -0
@@ -82,6 +82,11 @@ func (r *Repository) openDirectory(path string) (file *os.File, err error) {
8282 }
8383 joinCloseError(&err, "close snapshot root", closeErr)
8484 }()
85+ return openRootDirectory(root, rel)
86+}
87+
88+// openRootDirectory opens rel through a pinned root, refusing a final symlink.
89+func openRootDirectory(root *os.Root, rel string) (*os.File, error) {
8590 if err := rejectFinalSymlink(root, rel); err != nil {
8691 return nil, err
8792 }
@@ -82,6 +82,11 @@ func (r *Repository) openDirectory(path string) (file *os.File, err error) {
82 }82 }
83 joinCloseError(&err, "close snapshot root", closeErr)83 joinCloseError(&err, "close snapshot root", closeErr)
84 }()84 }()
85+ return openRootDirectory(root, rel)
86+}
87+
88+// openRootDirectory opens rel through a pinned root, refusing a final symlink.
89+func openRootDirectory(root *os.Root, rel string) (*os.File, error) {
85 if err := rejectFinalSymlink(root, rel); err != nil {90 if err := rejectFinalSymlink(root, rel); err != nil {
86 return nil, err91 return nil, err
87 }92 }
M internal/adapters/snapshot/test_helpers_test.go +22 -0
@@ -1,13 +1,35 @@
11 package snapshot
22
33 import (
4+ "context"
5+ "errors"
46 "path/filepath"
57 "testing"
68
9+ "github.com/bnema/vev/internal/domain"
10+
711 vt "github.com/bnema/vev-vt"
812 renderer "github.com/bnema/vev-vt/ansi"
913 )
1014
15+// collectAll runs one GC pass the way the recovery coordinator does: every
16+// listed incarnation absent from keep is an orphan.
17+func collectAll(ctx context.Context, r *Repository, keep map[domain.IncarnationID]domain.CheckpointRef) error {
18+ ids, err := r.SnapshotIncarnations(ctx)
19+ if err != nil {
20+ return err
21+ }
22+ var collected error
23+ for _, id := range ids {
24+ var ref *domain.CheckpointRef
25+ if committed, ok := keep[id]; ok {
26+ ref = &committed
27+ }
28+ collected = errors.Join(collected, r.CollectIncarnationGarbage(ctx, id, ref))
29+ }
30+ return collected
31+}
32+
1133 func privateDir(t *testing.T) string {
1234 t.Helper()
1335 return filepath.Join(t.TempDir(), "vev")
@@ -1,13 +1,35 @@
1 package snapshot1 package snapshot
2 2
3 import (3 import (
4+ "context"
5+ "errors"
4 "path/filepath"6 "path/filepath"
5 "testing"7 "testing"
6 8
9+ "github.com/bnema/vev/internal/domain"
10+
7 vt "github.com/bnema/vev-vt"11 vt "github.com/bnema/vev-vt"
8 renderer "github.com/bnema/vev-vt/ansi"12 renderer "github.com/bnema/vev-vt/ansi"
9 )13 )
10 14
15+// collectAll runs one GC pass the way the recovery coordinator does: every
16+// listed incarnation absent from keep is an orphan.
17+func collectAll(ctx context.Context, r *Repository, keep map[domain.IncarnationID]domain.CheckpointRef) error {
18+ ids, err := r.SnapshotIncarnations(ctx)
19+ if err != nil {
20+ return err
21+ }
22+ var collected error
23+ for _, id := range ids {
24+ var ref *domain.CheckpointRef
25+ if committed, ok := keep[id]; ok {
26+ ref = &committed
27+ }
28+ collected = errors.Join(collected, r.CollectIncarnationGarbage(ctx, id, ref))
29+ }
30+ return collected
31+}
32+
11 func privateDir(t *testing.T) string {33 func privateDir(t *testing.T) string {
12 t.Helper()34 t.Helper()
13 return filepath.Join(t.TempDir(), "vev")35 return filepath.Join(t.TempDir(), "vev")
M internal/app/daemon_integration_test.go +7 -2
@@ -1041,7 +1041,8 @@ func TestLifecycleOwnershipOutlivesMaintenanceWriter(t *testing.T) {
10411041 shutdownClock,
10421042 discardLog(),
10431043 daemon.WithSnapshotRepository(repository),
1044- daemon.WithDurableMaintenance(catalogue, maintenanceRepository),
1044+ daemon.WithRecoveryCoordinator(recovery.NewCoordinator(catalogue, maintenanceRepository, nil)),
1045+ daemon.WithSnapshotGarbageCollection(),
10451046 daemon.WithCatalogue(catalogue, nil),
10461047 )
10471048 listener, err := ipc.Listen(runtimeDir)
@@ -1083,7 +1084,11 @@ type lifecycleBlockingMaintenanceRepository struct {
10831084 returnOnce sync.Once
10841085 }
10851086
1086-func (r *lifecycleBlockingMaintenanceRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {
1087+func (r *lifecycleBlockingMaintenanceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
1088+ return []domain.IncarnationID{{1}}, nil
1089+}
1090+
1091+func (r *lifecycleBlockingMaintenanceRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
10871092 r.enterOnce.Do(func() { close(r.entered) })
10881093 <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call.
10891094 r.returnOnce.Do(func() { close(r.returned) })
@@ -1041,7 +1041,8 @@ func TestLifecycleOwnershipOutlivesMaintenanceWriter(t *testing.T) {
1041 shutdownClock,1041 shutdownClock,
1042 discardLog(),1042 discardLog(),
1043 daemon.WithSnapshotRepository(repository),1043 daemon.WithSnapshotRepository(repository),
1044- daemon.WithDurableMaintenance(catalogue, maintenanceRepository),1044+ daemon.WithRecoveryCoordinator(recovery.NewCoordinator(catalogue, maintenanceRepository, nil)),
1045+ daemon.WithSnapshotGarbageCollection(),
1045 daemon.WithCatalogue(catalogue, nil),1046 daemon.WithCatalogue(catalogue, nil),
1046 )1047 )
1047 listener, err := ipc.Listen(runtimeDir)1048 listener, err := ipc.Listen(runtimeDir)
@@ -1083,7 +1084,11 @@ type lifecycleBlockingMaintenanceRepository struct {
1083 returnOnce sync.Once1084 returnOnce sync.Once
1084 }1085 }
1085 1086
1086-func (r *lifecycleBlockingMaintenanceRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {1087+func (r *lifecycleBlockingMaintenanceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
1088+ return []domain.IncarnationID{{1}}, nil
1089+}
1090+
1091+func (r *lifecycleBlockingMaintenanceRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
1087 r.enterOnce.Do(func() { close(r.entered) })1092 r.enterOnce.Do(func() { close(r.entered) })
1088 <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call.1093 <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call.
1089 r.returnOnce.Do(func() { close(r.returned) })1094 r.returnOnce.Do(func() { close(r.returned) })
M internal/app/run.go +8 -18
@@ -704,15 +704,6 @@ func logStartupRecoveryCounts(log *slog.Logger, records []domain.CatalogueRecord
704704 log.Info("daemon_startup_complete", "healthy", healthy, "fresh", fresh, "restoring", restoring, "broken", broken)
705705 }
706706
707-func constructDaemonBeforeSocketPublication(
708- construct func() *daemon.Daemon,
709- listen func() (wire.Listener, error),
710-) (*daemon.Daemon, wire.Listener, error) {
711- d := construct()
712- ln, err := listen()
713- return d, ln, err
714-}
715-
716707 func runDaemonOwned(ctx context.Context) (retErr error) {
717708 log, logCloser, err := configureLogging(logging.Daemon, true)
718709 if err != nil {
@@ -784,7 +775,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err
784775 }
785776 logCatalogueRecovery(log, opened.Records, recoveryMode)
786777 daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator))
787- daemonOpts = append(daemonOpts, daemon.WithDurableMaintenance(opened.Catalogue, snapshotRepository))
778+ daemonOpts = append(daemonOpts, daemon.WithSnapshotGarbageCollection())
788779 log.Info("session persistence enabled", "path", storePath)
789780 daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records))
790781
@@ -827,16 +818,15 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err
827818 }
828819 }()
829820
830- d, ln, err := constructDaemonBeforeSocketPublication(
831- // Snapshot GC runs in the background once restoration finishes, so it
832- // never delays socket publication.
833- func() *daemon.Daemon { return daemon.New(pty.NewFactory(), clk, log, daemonOpts...) },
834- func() (wire.Listener, error) { return listenDaemon(ipc.SocketDir(), observer) },
835- )
821+ // The catalogue-backed registry exists before the socket is published.
822+ // Snapshot GC runs in the background once restoration finishes, so it never
823+ // delays socket publication.
824+ d := daemon.New(pty.NewFactory(), clk, log, daemonOpts...)
825+ ln, err := listenDaemon(ipc.SocketDir(), observer)
836826 if err != nil {
837827 closeErr := opened.Catalogue.Close()
838- log.Error("daemon startup preparation failed", "socket_dir", ipc.SocketDir(), "err", err)
839- return errors.Join(fmt.Errorf("vev: prepare daemon startup: %w", err), closeErr)
828+ log.Error("daemon listen failed", "socket_dir", ipc.SocketDir(), "err", err)
829+ return errors.Join(fmt.Errorf("vev: daemon listen: %w", err), closeErr)
840830 }
841831 defer func() { _ = ln.Close() }()
842832 log.Info("daemon starting", "socket", ln.Addr())
@@ -704,15 +704,6 @@ func logStartupRecoveryCounts(log *slog.Logger, records []domain.CatalogueRecord
704 log.Info("daemon_startup_complete", "healthy", healthy, "fresh", fresh, "restoring", restoring, "broken", broken)704 log.Info("daemon_startup_complete", "healthy", healthy, "fresh", fresh, "restoring", restoring, "broken", broken)
705 }705 }
706 706
707-func constructDaemonBeforeSocketPublication(
708- construct func() *daemon.Daemon,
709- listen func() (wire.Listener, error),
710-) (*daemon.Daemon, wire.Listener, error) {
711- d := construct()
712- ln, err := listen()
713- return d, ln, err
714-}
715-
716 func runDaemonOwned(ctx context.Context) (retErr error) {707 func runDaemonOwned(ctx context.Context) (retErr error) {
717 log, logCloser, err := configureLogging(logging.Daemon, true)708 log, logCloser, err := configureLogging(logging.Daemon, true)
718 if err != nil {709 if err != nil {
@@ -784,7 +775,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err
784 }775 }
785 logCatalogueRecovery(log, opened.Records, recoveryMode)776 logCatalogueRecovery(log, opened.Records, recoveryMode)
786 daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator))777 daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator))
787- daemonOpts = append(daemonOpts, daemon.WithDurableMaintenance(opened.Catalogue, snapshotRepository))778+ daemonOpts = append(daemonOpts, daemon.WithSnapshotGarbageCollection())
788 log.Info("session persistence enabled", "path", storePath)779 log.Info("session persistence enabled", "path", storePath)
789 daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records))780 daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records))
790 781
@@ -827,16 +818,15 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err
827 }818 }
828 }()819 }()
829 820
830- d, ln, err := constructDaemonBeforeSocketPublication(821+ // The catalogue-backed registry exists before the socket is published.
831- // Snapshot GC runs in the background once restoration finishes, so it822+ // Snapshot GC runs in the background once restoration finishes, so it never
832- // never delays socket publication.823+ // delays socket publication.
833- func() *daemon.Daemon { return daemon.New(pty.NewFactory(), clk, log, daemonOpts...) },824+ d := daemon.New(pty.NewFactory(), clk, log, daemonOpts...)
834- func() (wire.Listener, error) { return listenDaemon(ipc.SocketDir(), observer) },825+ ln, err := listenDaemon(ipc.SocketDir(), observer)
835- )
836 if err != nil {826 if err != nil {
837 closeErr := opened.Catalogue.Close()827 closeErr := opened.Catalogue.Close()
838- log.Error("daemon startup preparation failed", "socket_dir", ipc.SocketDir(), "err", err)828+ log.Error("daemon listen failed", "socket_dir", ipc.SocketDir(), "err", err)
839- return errors.Join(fmt.Errorf("vev: prepare daemon startup: %w", err), closeErr)829+ return errors.Join(fmt.Errorf("vev: daemon listen: %w", err), closeErr)
840 }830 }
841 defer func() { _ = ln.Close() }()831 defer func() { _ = ln.Close() }()
842 log.Info("daemon starting", "socket", ln.Addr())832 log.Info("daemon starting", "socket", ln.Addr())
M internal/app/run_test.go +0 -18
@@ -21,7 +21,6 @@ import (
2121 "github.com/bnema/vev/internal/ports"
2222 "github.com/bnema/vev/internal/protocol"
2323 "github.com/bnema/vev/internal/protocol/wire"
24- "github.com/bnema/vev/internal/usecase/daemon"
2524 "github.com/bnema/vev/pkg/kv"
2625 "github.com/bnema/vev/pkg/safedir"
2726 "github.com/stretchr/testify/require"
@@ -169,23 +168,6 @@ func TestLifecycleOwnershipAcceptsOwnerOnlyVariantLock(t *testing.T) {
169168 require.NoError(t, owner.Release())
170169 }
171170
172-func TestCatalogueRegistryConstructionPrecedesSocketPublication(t *testing.T) {
173- var events []string
174- _, _, err := constructDaemonBeforeSocketPublication(
175- func() *daemon.Daemon {
176- events = append(events, "catalogue-registry")
177- return nil
178- },
179- func() (wire.Listener, error) {
180- require.Equal(t, []string{"catalogue-registry"}, events)
181- events = append(events, "socket-publication")
182- return nil, nil
183- },
184- )
185- require.NoError(t, err)
186- require.Equal(t, []string{"catalogue-registry", "socket-publication"}, events)
187-}
188-
189171 func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) {
190172 t.Run("ownership prefix is exact", func(t *testing.T) {
191173 var events []string
@@ -21,7 +21,6 @@ import (
21 "github.com/bnema/vev/internal/ports"21 "github.com/bnema/vev/internal/ports"
22 "github.com/bnema/vev/internal/protocol"22 "github.com/bnema/vev/internal/protocol"
23 "github.com/bnema/vev/internal/protocol/wire"23 "github.com/bnema/vev/internal/protocol/wire"
24- "github.com/bnema/vev/internal/usecase/daemon"
25 "github.com/bnema/vev/pkg/kv"24 "github.com/bnema/vev/pkg/kv"
26 "github.com/bnema/vev/pkg/safedir"25 "github.com/bnema/vev/pkg/safedir"
27 "github.com/stretchr/testify/require"26 "github.com/stretchr/testify/require"
@@ -169,23 +168,6 @@ func TestLifecycleOwnershipAcceptsOwnerOnlyVariantLock(t *testing.T) {
169 require.NoError(t, owner.Release())168 require.NoError(t, owner.Release())
170 }169 }
171 170
172-func TestCatalogueRegistryConstructionPrecedesSocketPublication(t *testing.T) {
173- var events []string
174- _, _, err := constructDaemonBeforeSocketPublication(
175- func() *daemon.Daemon {
176- events = append(events, "catalogue-registry")
177- return nil
178- },
179- func() (wire.Listener, error) {
180- require.Equal(t, []string{"catalogue-registry"}, events)
181- events = append(events, "socket-publication")
182- return nil, nil
183- },
184- )
185- require.NoError(t, err)
186- require.Equal(t, []string{"catalogue-registry", "socket-publication"}, events)
187-}
188-
189 func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) {171 func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) {
190 t.Run("ownership prefix is exact", func(t *testing.T) {172 t.Run("ownership prefix is exact", func(t *testing.T) {
191 var events []string173 var events []string
M internal/ports/mocks/recovery_mock.go +86 -18
@@ -560,59 +560,65 @@ func (_m *MockSnapshotRepository) EXPECT() *MockSnapshotRepository_Expecter {
560560 return &MockSnapshotRepository_Expecter{mock: &_m.Mock}
561561 }
562562
563-// CollectGarbage provides a mock function for the type MockSnapshotRepository
564-func (_mock *MockSnapshotRepository) CollectGarbage(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef) error {
565- ret := _mock.Called(context1, incarnationIDToCheckpointRef)
563+// CollectIncarnationGarbage provides a mock function for the type MockSnapshotRepository
564+func (_mock *MockSnapshotRepository) CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
565+ ret := _mock.Called(ctx, id, keep)
566566
567567 if len(ret) == 0 {
568- panic("no return value specified for CollectGarbage")
568+ panic("no return value specified for CollectIncarnationGarbage")
569569 }
570570
571571 var r0 error
572- if returnFunc, ok := ret.Get(0).(func(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error); ok {
573- r0 = returnFunc(context1, incarnationIDToCheckpointRef)
572+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.IncarnationID, *domain.CheckpointRef) error); ok {
573+ r0 = returnFunc(ctx, id, keep)
574574 } else {
575575 r0 = ret.Error(0)
576576 }
577577 return r0
578578 }
579579
580-// MockSnapshotRepository_CollectGarbage_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CollectGarbage'
581-type MockSnapshotRepository_CollectGarbage_Call struct {
580+// MockSnapshotRepository_CollectIncarnationGarbage_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CollectIncarnationGarbage'
581+type MockSnapshotRepository_CollectIncarnationGarbage_Call struct {
582582 *mock.Call
583583 }
584584
585-// CollectGarbage is a helper method to define mock.On call
586-// - context1 context.Context
587-// - incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef
588-func (_e *MockSnapshotRepository_Expecter) CollectGarbage(context1 any, incarnationIDToCheckpointRef any) *MockSnapshotRepository_CollectGarbage_Call {
589- return &MockSnapshotRepository_CollectGarbage_Call{Call: _e.mock.On("CollectGarbage", context1, incarnationIDToCheckpointRef)}
585+// CollectIncarnationGarbage is a helper method to define mock.On call
586+// - ctx context.Context
587+// - id domain.IncarnationID
588+// - keep *domain.CheckpointRef
589+func (_e *MockSnapshotRepository_Expecter) CollectIncarnationGarbage(ctx any, id any, keep any) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
590+ return &MockSnapshotRepository_CollectIncarnationGarbage_Call{Call: _e.mock.On("CollectIncarnationGarbage", ctx, id, keep)}
590591 }
591592
592-func (_c *MockSnapshotRepository_CollectGarbage_Call) Run(run func(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef)) *MockSnapshotRepository_CollectGarbage_Call {
593+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) Run(run func(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef)) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
593594 _c.Call.Run(func(args mock.Arguments) {
594595 var arg0 context.Context
595596 if args[0] != nil {
596597 arg0 = args[0].(context.Context)
597598 }
598- var arg1 map[domain.IncarnationID]domain.CheckpointRef
599+ var arg1 domain.IncarnationID
599600 if args[1] != nil {
600- arg1 = args[1].(map[domain.IncarnationID]domain.CheckpointRef)
601+ arg1 = args[1].(domain.IncarnationID)
602+ }
603+ var arg2 *domain.CheckpointRef
604+ if args[2] != nil {
605+ arg2 = args[2].(*domain.CheckpointRef)
601606 }
602607 run(
603608 arg0,
604609 arg1,
610+ arg2,
605611 )
606612 })
607613 return _c
608614 }
609615
610-func (_c *MockSnapshotRepository_CollectGarbage_Call) Return(err error) *MockSnapshotRepository_CollectGarbage_Call {
616+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) Return(err error) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
611617 _c.Call.Return(err)
612618 return _c
613619 }
614620
615-func (_c *MockSnapshotRepository_CollectGarbage_Call) RunAndReturn(run func(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef) error) *MockSnapshotRepository_CollectGarbage_Call {
621+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) RunAndReturn(run func(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
616622 _c.Call.Return(run)
617623 return _c
618624 }
@@ -871,3 +877,65 @@ func (_c *MockSnapshotRepository_ReconcileCheckpoint_Call) RunAndReturn(run func
871877 _c.Call.Return(run)
872878 return _c
873879 }
880+
881+// SnapshotIncarnations provides a mock function for the type MockSnapshotRepository
882+func (_mock *MockSnapshotRepository) SnapshotIncarnations(context1 context.Context) ([]domain.IncarnationID, error) {
883+ ret := _mock.Called(context1)
884+
885+ if len(ret) == 0 {
886+ panic("no return value specified for SnapshotIncarnations")
887+ }
888+
889+ var r0 []domain.IncarnationID
890+ var r1 error
891+ if returnFunc, ok := ret.Get(0).(func(context.Context) ([]domain.IncarnationID, error)); ok {
892+ return returnFunc(context1)
893+ }
894+ if returnFunc, ok := ret.Get(0).(func(context.Context) []domain.IncarnationID); ok {
895+ r0 = returnFunc(context1)
896+ } else {
897+ if ret.Get(0) != nil {
898+ r0 = ret.Get(0).([]domain.IncarnationID)
899+ }
900+ }
901+ if returnFunc, ok := ret.Get(1).(func(context.Context) error); ok {
902+ r1 = returnFunc(context1)
903+ } else {
904+ r1 = ret.Error(1)
905+ }
906+ return r0, r1
907+}
908+
909+// MockSnapshotRepository_SnapshotIncarnations_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SnapshotIncarnations'
910+type MockSnapshotRepository_SnapshotIncarnations_Call struct {
911+ *mock.Call
912+}
913+
914+// SnapshotIncarnations is a helper method to define mock.On call
915+// - context1 context.Context
916+func (_e *MockSnapshotRepository_Expecter) SnapshotIncarnations(context1 any) *MockSnapshotRepository_SnapshotIncarnations_Call {
917+ return &MockSnapshotRepository_SnapshotIncarnations_Call{Call: _e.mock.On("SnapshotIncarnations", context1)}
918+}
919+
920+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) Run(run func(context1 context.Context)) *MockSnapshotRepository_SnapshotIncarnations_Call {
921+ _c.Call.Run(func(args mock.Arguments) {
922+ var arg0 context.Context
923+ if args[0] != nil {
924+ arg0 = args[0].(context.Context)
925+ }
926+ run(
927+ arg0,
928+ )
929+ })
930+ return _c
931+}
932+
933+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) Return(incarnationIDs []domain.IncarnationID, err error) *MockSnapshotRepository_SnapshotIncarnations_Call {
934+ _c.Call.Return(incarnationIDs, err)
935+ return _c
936+}
937+
938+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) RunAndReturn(run func(context1 context.Context) ([]domain.IncarnationID, error)) *MockSnapshotRepository_SnapshotIncarnations_Call {
939+ _c.Call.Return(run)
940+ return _c
941+}
@@ -560,59 +560,65 @@ func (_m *MockSnapshotRepository) EXPECT() *MockSnapshotRepository_Expecter {
560 return &MockSnapshotRepository_Expecter{mock: &_m.Mock}560 return &MockSnapshotRepository_Expecter{mock: &_m.Mock}
561 }561 }
562 562
563-// CollectGarbage provides a mock function for the type MockSnapshotRepository563+// CollectIncarnationGarbage provides a mock function for the type MockSnapshotRepository
564-func (_mock *MockSnapshotRepository) CollectGarbage(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef) error {564+func (_mock *MockSnapshotRepository) CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
565- ret := _mock.Called(context1, incarnationIDToCheckpointRef)565+ ret := _mock.Called(ctx, id, keep)
566 566
567 if len(ret) == 0 {567 if len(ret) == 0 {
568- panic("no return value specified for CollectGarbage")568+ panic("no return value specified for CollectIncarnationGarbage")
569 }569 }
570 570
571 var r0 error571 var r0 error
572- if returnFunc, ok := ret.Get(0).(func(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error); ok {572+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.IncarnationID, *domain.CheckpointRef) error); ok {
573- r0 = returnFunc(context1, incarnationIDToCheckpointRef)573+ r0 = returnFunc(ctx, id, keep)
574 } else {574 } else {
575 r0 = ret.Error(0)575 r0 = ret.Error(0)
576 }576 }
577 return r0577 return r0
578 }578 }
579 579
580-// MockSnapshotRepository_CollectGarbage_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CollectGarbage'580+// MockSnapshotRepository_CollectIncarnationGarbage_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CollectIncarnationGarbage'
581-type MockSnapshotRepository_CollectGarbage_Call struct {581+type MockSnapshotRepository_CollectIncarnationGarbage_Call struct {
582 *mock.Call582 *mock.Call
583 }583 }
584 584
585-// CollectGarbage is a helper method to define mock.On call585+// CollectIncarnationGarbage is a helper method to define mock.On call
586-// - context1 context.Context586+// - ctx context.Context
587-// - incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef587+// - id domain.IncarnationID
588-func (_e *MockSnapshotRepository_Expecter) CollectGarbage(context1 any, incarnationIDToCheckpointRef any) *MockSnapshotRepository_CollectGarbage_Call {588+// - keep *domain.CheckpointRef
589- return &MockSnapshotRepository_CollectGarbage_Call{Call: _e.mock.On("CollectGarbage", context1, incarnationIDToCheckpointRef)}589+func (_e *MockSnapshotRepository_Expecter) CollectIncarnationGarbage(ctx any, id any, keep any) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
590+ return &MockSnapshotRepository_CollectIncarnationGarbage_Call{Call: _e.mock.On("CollectIncarnationGarbage", ctx, id, keep)}
590 }591 }
591 592
592-func (_c *MockSnapshotRepository_CollectGarbage_Call) Run(run func(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef)) *MockSnapshotRepository_CollectGarbage_Call {593+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) Run(run func(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef)) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
593 _c.Call.Run(func(args mock.Arguments) {594 _c.Call.Run(func(args mock.Arguments) {
594 var arg0 context.Context595 var arg0 context.Context
595 if args[0] != nil {596 if args[0] != nil {
596 arg0 = args[0].(context.Context)597 arg0 = args[0].(context.Context)
597 }598 }
598- var arg1 map[domain.IncarnationID]domain.CheckpointRef599+ var arg1 domain.IncarnationID
599 if args[1] != nil {600 if args[1] != nil {
600- arg1 = args[1].(map[domain.IncarnationID]domain.CheckpointRef)601+ arg1 = args[1].(domain.IncarnationID)
602+ }
603+ var arg2 *domain.CheckpointRef
604+ if args[2] != nil {
605+ arg2 = args[2].(*domain.CheckpointRef)
601 }606 }
602 run(607 run(
603 arg0,608 arg0,
604 arg1,609 arg1,
610+ arg2,
605 )611 )
606 })612 })
607 return _c613 return _c
608 }614 }
609 615
610-func (_c *MockSnapshotRepository_CollectGarbage_Call) Return(err error) *MockSnapshotRepository_CollectGarbage_Call {616+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) Return(err error) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
611 _c.Call.Return(err)617 _c.Call.Return(err)
612 return _c618 return _c
613 }619 }
614 620
615-func (_c *MockSnapshotRepository_CollectGarbage_Call) RunAndReturn(run func(context1 context.Context, incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef) error) *MockSnapshotRepository_CollectGarbage_Call {621+func (_c *MockSnapshotRepository_CollectIncarnationGarbage_Call) RunAndReturn(run func(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error) *MockSnapshotRepository_CollectIncarnationGarbage_Call {
616 _c.Call.Return(run)622 _c.Call.Return(run)
617 return _c623 return _c
618 }624 }
@@ -871,3 +877,65 @@ func (_c *MockSnapshotRepository_ReconcileCheckpoint_Call) RunAndReturn(run func
871 _c.Call.Return(run)877 _c.Call.Return(run)
872 return _c878 return _c
873 }879 }
880+
881+// SnapshotIncarnations provides a mock function for the type MockSnapshotRepository
882+func (_mock *MockSnapshotRepository) SnapshotIncarnations(context1 context.Context) ([]domain.IncarnationID, error) {
883+ ret := _mock.Called(context1)
884+
885+ if len(ret) == 0 {
886+ panic("no return value specified for SnapshotIncarnations")
887+ }
888+
889+ var r0 []domain.IncarnationID
890+ var r1 error
891+ if returnFunc, ok := ret.Get(0).(func(context.Context) ([]domain.IncarnationID, error)); ok {
892+ return returnFunc(context1)
893+ }
894+ if returnFunc, ok := ret.Get(0).(func(context.Context) []domain.IncarnationID); ok {
895+ r0 = returnFunc(context1)
896+ } else {
897+ if ret.Get(0) != nil {
898+ r0 = ret.Get(0).([]domain.IncarnationID)
899+ }
900+ }
901+ if returnFunc, ok := ret.Get(1).(func(context.Context) error); ok {
902+ r1 = returnFunc(context1)
903+ } else {
904+ r1 = ret.Error(1)
905+ }
906+ return r0, r1
907+}
908+
909+// MockSnapshotRepository_SnapshotIncarnations_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SnapshotIncarnations'
910+type MockSnapshotRepository_SnapshotIncarnations_Call struct {
911+ *mock.Call
912+}
913+
914+// SnapshotIncarnations is a helper method to define mock.On call
915+// - context1 context.Context
916+func (_e *MockSnapshotRepository_Expecter) SnapshotIncarnations(context1 any) *MockSnapshotRepository_SnapshotIncarnations_Call {
917+ return &MockSnapshotRepository_SnapshotIncarnations_Call{Call: _e.mock.On("SnapshotIncarnations", context1)}
918+}
919+
920+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) Run(run func(context1 context.Context)) *MockSnapshotRepository_SnapshotIncarnations_Call {
921+ _c.Call.Run(func(args mock.Arguments) {
922+ var arg0 context.Context
923+ if args[0] != nil {
924+ arg0 = args[0].(context.Context)
925+ }
926+ run(
927+ arg0,
928+ )
929+ })
930+ return _c
931+}
932+
933+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) Return(incarnationIDs []domain.IncarnationID, err error) *MockSnapshotRepository_SnapshotIncarnations_Call {
934+ _c.Call.Return(incarnationIDs, err)
935+ return _c
936+}
937+
938+func (_c *MockSnapshotRepository_SnapshotIncarnations_Call) RunAndReturn(run func(context1 context.Context) ([]domain.IncarnationID, error)) *MockSnapshotRepository_SnapshotIncarnations_Call {
939+ _c.Call.Return(run)
940+ return _c
941+}
M internal/ports/recovery.go +6 -1
@@ -44,5 +44,10 @@ type SnapshotRepository interface {
4444 LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error)
4545 ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error
4646 DeleteIncarnation(context.Context, domain.IncarnationID) error
47- CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error
47+ // SnapshotIncarnations lists the incarnations present in the repository.
48+ SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error)
49+ // CollectIncarnationGarbage applies retention to one incarnation. A nil
50+ // keep marks an orphan absent from a validated catalogue and removes it
51+ // whole; otherwise *keep is the committed checkpoint (zero when none).
52+ CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error
4853 }
@@ -44,5 +44,10 @@ type SnapshotRepository interface {
44 LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error)44 LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error)
45 ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error45 ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error
46 DeleteIncarnation(context.Context, domain.IncarnationID) error46 DeleteIncarnation(context.Context, domain.IncarnationID) error
47- CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error47+ // SnapshotIncarnations lists the incarnations present in the repository.
48+ SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error)
49+ // CollectIncarnationGarbage applies retention to one incarnation. A nil
50+ // keep marks an orphan absent from a validated catalogue and removes it
51+ // whole; otherwise *keep is the committed checkpoint (zero when none).
52+ CollectIncarnationGarbage(ctx context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error
48 }53 }
M internal/usecase/daemon/daemon.go +9 -15
@@ -297,11 +297,11 @@ type Daemon struct {
297297 catalogueRecords []domain.CatalogueRecord
298298 catalogueRecordsProvided bool
299299 // snapshotRepository is the sole checkpoint storage contract.
300- snapshotRepository ports.SnapshotRepository
301- recovery *recoveryusecase.Coordinator
302- durableMaintenance bool
303- maintenanceWorkerCancel context.CancelFunc
304- maintenanceWorkerDone chan struct{}
300+ snapshotRepository ports.SnapshotRepository
301+ recovery *recoveryusecase.Coordinator
302+ snapshotGarbageCollection bool
303+ maintenanceWorkerCancel context.CancelFunc
304+ maintenanceWorkerDone chan struct{}
305305 // restoreWorkerDone is the restoration goroutine's ownership signal. Startup
306306 // restoration reconciles durable checkpoints, so it is a durable writer and
307307 // is guarded by snapshotWorkerMu with the other two.
@@ -556,16 +556,10 @@ func WithNoticeStore(store ports.NoticeStore) Option {
556556 return func(d *Daemon) { d.noticeStore = store }
557557 }
558558
559-// WithDurableMaintenance enables background snapshot GC. Standalone users
560-// without an explicitly supplied coordinator receive the same canonical
561-// coordinator path.
562-func WithDurableMaintenance(catalogue ports.Catalogue, repository ports.SnapshotRepository) Option {
563- return func(d *Daemon) {
564- if d.recovery == nil {
565- d.recovery = recoveryusecase.NewCoordinator(catalogue, repository, nil)
566- }
567- d.durableMaintenance = true
568- }
559+// WithSnapshotGarbageCollection runs background snapshot GC through the
560+// recovery coordinator installed by WithRecoveryCoordinator.
561+func WithSnapshotGarbageCollection() Option {
562+ return func(d *Daemon) { d.snapshotGarbageCollection = true }
569563 }
570564
571565 // WithCwdReader overrides the process cwd reader used for persistence tests.
@@ -297,11 +297,11 @@ type Daemon struct {
297 catalogueRecords []domain.CatalogueRecord297 catalogueRecords []domain.CatalogueRecord
298 catalogueRecordsProvided bool298 catalogueRecordsProvided bool
299 // snapshotRepository is the sole checkpoint storage contract.299 // snapshotRepository is the sole checkpoint storage contract.
300- snapshotRepository ports.SnapshotRepository300+ snapshotRepository ports.SnapshotRepository
301- recovery *recoveryusecase.Coordinator301+ recovery *recoveryusecase.Coordinator
302- durableMaintenance bool302+ snapshotGarbageCollection bool
303- maintenanceWorkerCancel context.CancelFunc303+ maintenanceWorkerCancel context.CancelFunc
304- maintenanceWorkerDone chan struct{}304+ maintenanceWorkerDone chan struct{}
305 // restoreWorkerDone is the restoration goroutine's ownership signal. Startup305 // restoreWorkerDone is the restoration goroutine's ownership signal. Startup
306 // restoration reconciles durable checkpoints, so it is a durable writer and306 // restoration reconciles durable checkpoints, so it is a durable writer and
307 // is guarded by snapshotWorkerMu with the other two.307 // is guarded by snapshotWorkerMu with the other two.
@@ -556,16 +556,10 @@ func WithNoticeStore(store ports.NoticeStore) Option {
556 return func(d *Daemon) { d.noticeStore = store }556 return func(d *Daemon) { d.noticeStore = store }
557 }557 }
558 558
559-// WithDurableMaintenance enables background snapshot GC. Standalone users559+// WithSnapshotGarbageCollection runs background snapshot GC through the
560-// without an explicitly supplied coordinator receive the same canonical560+// recovery coordinator installed by WithRecoveryCoordinator.
561-// coordinator path.561+func WithSnapshotGarbageCollection() Option {
562-func WithDurableMaintenance(catalogue ports.Catalogue, repository ports.SnapshotRepository) Option {562+ return func(d *Daemon) { d.snapshotGarbageCollection = true }
563- return func(d *Daemon) {
564- if d.recovery == nil {
565- d.recovery = recoveryusecase.NewCoordinator(catalogue, repository, nil)
566- }
567- d.durableMaintenance = true
568- }
569 }563 }
570 564
571 // WithCwdReader overrides the process cwd reader used for persistence tests.565 // WithCwdReader overrides the process cwd reader used for persistence tests.
M internal/usecase/daemon/durable_authority_test.go +4 -1
@@ -18,7 +18,10 @@ type testLifecycleRepository struct{ ports.SnapshotRepository }
1818 func (testLifecycleRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {
1919 return nil
2020 }
21-func (testLifecycleRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {
21+func (testLifecycleRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
22+ return nil, nil
23+}
24+func (testLifecycleRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
2225 return nil
2326 }
2427
@@ -18,7 +18,10 @@ type testLifecycleRepository struct{ ports.SnapshotRepository }
18 func (testLifecycleRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {18 func (testLifecycleRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {
19 return nil19 return nil
20 }20 }
21-func (testLifecycleRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {21+func (testLifecycleRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
22+ return nil, nil
23+}
24+func (testLifecycleRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
22 return nil25 return nil
23 }26 }
24 27
M internal/usecase/daemon/maintenance.go +10 -22
@@ -2,11 +2,8 @@ package daemon
22
33 import (
44 "context"
5- "errors"
65 "fmt"
76 "time"
8-
9- recoveryusecase "github.com/bnema/vev/internal/usecase/recovery"
107 )
118
129 // snapshotGarbageCollectionInterval spaces the periodic snapshot GC passes. A
@@ -14,17 +11,12 @@ import (
1411 // until its next restart, which then pays for the whole backlog.
1512 const snapshotGarbageCollectionInterval = 30 * time.Minute
1613
17-// snapshotGarbageCollectionRetryInterval follows a pass that yielded to a
18-// durable mutation. Work already done is kept, so a retry resumes cheaply.
19-const snapshotGarbageCollectionRetryInterval = time.Minute
20-
2114 // startDurableMaintenance runs snapshot GC off the startup critical path: once
2215 // catalogue restoration has finished, then periodically. The recovery
23-// coordinator fences every pass against catalogue mutations and checkpoint
24-// publications, so the keep set is never stale relative to a concurrent writer,
25-// and any mutation preempts a running pass so it never waits for maintenance.
16+// coordinator fences each incarnation's step against catalogue mutations and
17+// checkpoint publications, so a mutation waits for one incarnation at most.
2618 func (d *Daemon) startDurableMaintenance() {
27- if d == nil || d.recovery == nil || !d.durableMaintenance {
19+ if d == nil || d.recovery == nil || !d.snapshotGarbageCollection {
2820 return
2921 }
3022 d.snapshotWorkerMu.Lock()
@@ -49,6 +41,11 @@ func (d *Daemon) startDurableMaintenance() {
4941 func (d *Daemon) cancelDurableMaintenance() {
5042 d.snapshotWorkerMu.Lock()
5143 defer d.snapshotWorkerMu.Unlock()
44+ d.cancelDurableMaintenanceLocked()
45+}
46+
47+// cancelDurableMaintenanceLocked requires snapshotWorkerMu.
48+func (d *Daemon) cancelDurableMaintenanceLocked() {
5249 if d.maintenanceWorkerCancel != nil {
5350 d.maintenanceWorkerCancel()
5451 }
@@ -63,16 +60,10 @@ func (d *Daemon) runDurableMaintenance(ctx context.Context) {
6360 case <-d.restoreDone:
6461 }
6562 for {
66- interval := snapshotGarbageCollectionInterval
67- switch err := d.collectSnapshotGarbage(ctx); {
68- case err == nil || ctx.Err() != nil:
69- case errors.Is(err, recoveryusecase.ErrGarbageCollectionPreempted):
70- d.log.Debug("snapshot_garbage_collection_preempted")
71- interval = snapshotGarbageCollectionRetryInterval
72- default:
63+ if err := d.collectSnapshotGarbage(ctx); err != nil && ctx.Err() == nil {
7364 d.log.Warn("snapshot_garbage_collection_failed", "err", err)
7465 }
75- timer := d.clock.NewTimer(interval)
66+ timer := d.clock.NewTimer(snapshotGarbageCollectionInterval)
7667 select {
7768 case <-ctx.Done():
7869 timer.Stop()
@@ -84,9 +75,6 @@ func (d *Daemon) runDurableMaintenance(ctx context.Context) {
8475
8576 // collectSnapshotGarbage runs one coordinator-owned GC pass.
8677 func (d *Daemon) collectSnapshotGarbage(ctx context.Context) error {
87- if d == nil || d.recovery == nil {
88- return nil
89- }
9078 incarnations, err := d.recovery.CollectGarbage(ctx)
9179 if err != nil {
9280 return fmt.Errorf("collect snapshots: %w", err)
@@ -2,11 +2,8 @@ package daemon
2 2
3 import (3 import (
4 "context"4 "context"
5- "errors"
6 "fmt"5 "fmt"
7 "time"6 "time"
8-
9- recoveryusecase "github.com/bnema/vev/internal/usecase/recovery"
10 )7 )
11 8
12 // snapshotGarbageCollectionInterval spaces the periodic snapshot GC passes. A9 // snapshotGarbageCollectionInterval spaces the periodic snapshot GC passes. A
@@ -14,17 +11,12 @@ import (
14 // until its next restart, which then pays for the whole backlog.11 // until its next restart, which then pays for the whole backlog.
15 const snapshotGarbageCollectionInterval = 30 * time.Minute12 const snapshotGarbageCollectionInterval = 30 * time.Minute
16 13
17-// snapshotGarbageCollectionRetryInterval follows a pass that yielded to a
18-// durable mutation. Work already done is kept, so a retry resumes cheaply.
19-const snapshotGarbageCollectionRetryInterval = time.Minute
20-
21 // startDurableMaintenance runs snapshot GC off the startup critical path: once14 // startDurableMaintenance runs snapshot GC off the startup critical path: once
22 // catalogue restoration has finished, then periodically. The recovery15 // catalogue restoration has finished, then periodically. The recovery
23-// coordinator fences every pass against catalogue mutations and checkpoint16+// coordinator fences each incarnation's step against catalogue mutations and
24-// publications, so the keep set is never stale relative to a concurrent writer,17+// checkpoint publications, so a mutation waits for one incarnation at most.
25-// and any mutation preempts a running pass so it never waits for maintenance.
26 func (d *Daemon) startDurableMaintenance() {18 func (d *Daemon) startDurableMaintenance() {
27- if d == nil || d.recovery == nil || !d.durableMaintenance {19+ if d == nil || d.recovery == nil || !d.snapshotGarbageCollection {
28 return20 return
29 }21 }
30 d.snapshotWorkerMu.Lock()22 d.snapshotWorkerMu.Lock()
@@ -49,6 +41,11 @@ func (d *Daemon) startDurableMaintenance() {
49 func (d *Daemon) cancelDurableMaintenance() {41 func (d *Daemon) cancelDurableMaintenance() {
50 d.snapshotWorkerMu.Lock()42 d.snapshotWorkerMu.Lock()
51 defer d.snapshotWorkerMu.Unlock()43 defer d.snapshotWorkerMu.Unlock()
44+ d.cancelDurableMaintenanceLocked()
45+}
46+
47+// cancelDurableMaintenanceLocked requires snapshotWorkerMu.
48+func (d *Daemon) cancelDurableMaintenanceLocked() {
52 if d.maintenanceWorkerCancel != nil {49 if d.maintenanceWorkerCancel != nil {
53 d.maintenanceWorkerCancel()50 d.maintenanceWorkerCancel()
54 }51 }
@@ -63,16 +60,10 @@ func (d *Daemon) runDurableMaintenance(ctx context.Context) {
63 case <-d.restoreDone:60 case <-d.restoreDone:
64 }61 }
65 for {62 for {
66- interval := snapshotGarbageCollectionInterval63+ if err := d.collectSnapshotGarbage(ctx); err != nil && ctx.Err() == nil {
67- switch err := d.collectSnapshotGarbage(ctx); {
68- case err == nil || ctx.Err() != nil:
69- case errors.Is(err, recoveryusecase.ErrGarbageCollectionPreempted):
70- d.log.Debug("snapshot_garbage_collection_preempted")
71- interval = snapshotGarbageCollectionRetryInterval
72- default:
73 d.log.Warn("snapshot_garbage_collection_failed", "err", err)64 d.log.Warn("snapshot_garbage_collection_failed", "err", err)
74 }65 }
75- timer := d.clock.NewTimer(interval)66+ timer := d.clock.NewTimer(snapshotGarbageCollectionInterval)
76 select {67 select {
77 case <-ctx.Done():68 case <-ctx.Done():
78 timer.Stop()69 timer.Stop()
@@ -84,9 +75,6 @@ func (d *Daemon) runDurableMaintenance(ctx context.Context) {
84 75
85 // collectSnapshotGarbage runs one coordinator-owned GC pass.76 // collectSnapshotGarbage runs one coordinator-owned GC pass.
86 func (d *Daemon) collectSnapshotGarbage(ctx context.Context) error {77 func (d *Daemon) collectSnapshotGarbage(ctx context.Context) error {
87- if d == nil || d.recovery == nil {
88- return nil
89- }
90 incarnations, err := d.recovery.CollectGarbage(ctx)78 incarnations, err := d.recovery.CollectGarbage(ctx)
91 if err != nil {79 if err != nil {
92 return fmt.Errorf("collect snapshots: %w", err)80 return fmt.Errorf("collect snapshots: %w", err)
M internal/usecase/daemon/maintenance_test.go +44 -66
@@ -4,6 +4,7 @@ import (
44 "context"
55 "errors"
66 "testing"
7+ "testing/synctest"
78 "time"
89
910 "github.com/bnema/vev/internal/domain"
@@ -13,88 +14,65 @@ import (
1314 "github.com/stretchr/testify/require"
1415 )
1516
16-func TestDurableMaintenanceCollectsSnapshotGarbage(t *testing.T) {
17+func TestSnapshotGarbageCollectionRunsAfterRestorationThenPeriodically(t *testing.T) {
1718 committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}}
18- records := []domain.CatalogueRecord{
19- {Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed},
20- {Name: "fresh", IncarnationID: domain.IncarnationID{2}},
21- }
22- validKeep := mock.MatchedBy(func(keep map[domain.IncarnationID]domain.CheckpointRef) bool {
23- return len(keep) == 2 && keep[records[0].IncarnationID] == committed && keep[records[1].IncarnationID] == (domain.CheckpointRef{})
24- })
19+ records := []domain.CatalogueRecord{{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}}
20+ id := records[0].IncarnationID
2521
2622 tests := []struct {
27- name string
28- // passes is the number of GC passes expected: the first after
29- // restoration, each later one after a periodic timer tick.
30- passes int
31- recordsErr error
32- collectErr error
33- wantCollect bool
34- wantInterval time.Duration
23+ name string
24+ recordsErr error
25+ wantCollect bool
3526 }{
36- {name: "first pass after restoration", passes: 1, wantCollect: true, wantInterval: snapshotGarbageCollectionInterval},
37- {name: "periodic pass after timer tick", passes: 2, wantCollect: true, wantInterval: snapshotGarbageCollectionInterval},
38- {name: "unreadable catalogue never collects", passes: 1, recordsErr: errors.New("catalogue read failed"), wantInterval: snapshotGarbageCollectionInterval},
39- {name: "preempted pass retries sooner", passes: 1, wantCollect: true, collectErr: recoveryusecase.ErrGarbageCollectionPreempted, wantInterval: snapshotGarbageCollectionRetryInterval},
27+ {name: "collects after restoration and on every tick", wantCollect: true},
28+ {name: "unreadable catalogue never collects", recordsErr: errors.New("catalogue read failed")},
4029 }
4130 for _, tt := range tests {
4231 t.Run(tt.name, func(t *testing.T) {
43- catalogue := portsmocks.NewMockCatalogue(t)
44- repository := portsmocks.NewMockSnapshotRepository(t)
45- collected := make(chan struct{}, tt.passes)
46- if tt.recordsErr != nil {
47- catalogue.EXPECT().Records().Return(nil, tt.recordsErr).Times(tt.passes)
48- } else {
49- catalogue.EXPECT().Records().Return(records, nil).Times(tt.passes)
50- }
51- if tt.wantCollect {
52- repository.EXPECT().CollectGarbage(mock.Anything, validKeep).
53- Run(func(context.Context, map[domain.IncarnationID]domain.CheckpointRef) { collected <- struct{}{} }).
54- Return(tt.collectErr).Times(tt.passes)
55- }
32+ synctest.Test(t, func(t *testing.T) {
33+ const passes = 2
34+ catalogue := portsmocks.NewMockCatalogue(t)
35+ repository := portsmocks.NewMockSnapshotRepository(t)
36+ catalogue.EXPECT().Records().Return(records, tt.recordsErr).Times(passes)
37+ repository.EXPECT().SnapshotIncarnations(mock.Anything).Return([]domain.IncarnationID{id}, nil).Times(passes)
38+ collected := make(chan struct{}, passes)
39+ if tt.wantCollect {
40+ repository.EXPECT().CollectIncarnationGarbage(mock.Anything, id, &committed).
41+ Run(func(context.Context, domain.IncarnationID, *domain.CheckpointRef) { collected <- struct{}{} }).
42+ Return(nil).Times(passes)
43+ }
5644
57- clk := &signalClock{timers: make(chan *signalTimer, 1)}
58- d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clk)
59- WithDurableMaintenance(catalogue, repository)(d)
60- d.startDurableMaintenance()
45+ clk := &signalClock{timers: make(chan *signalTimer, 1)}
46+ d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clk)
47+ WithRecoveryCoordinator(recoveryusecase.NewCoordinator(catalogue, repository, nil))(d)
48+ WithSnapshotGarbageCollection()(d)
49+ d.startDurableMaintenance()
6150
62- select {
63- case <-collected:
64- t.Fatal("GC must wait for restoration to finish")
65- case <-time.After(20 * time.Millisecond):
66- }
67- d.closeRestoreDone()
51+ synctest.Wait()
52+ require.Empty(t, clk.timers, "GC must wait for restoration to finish")
53+ d.closeRestoreDone()
6854
69- for pass := range tt.passes {
70- timer := receiveWithin(t, clk.timers, "GC interval timer")
71- require.Equal(t, tt.wantInterval, timer.duration)
72- if tt.wantCollect {
73- receiveWithin(t, collected, "GC pass")
55+ for pass := range passes {
56+ synctest.Wait()
57+ timer := <-clk.timers
58+ require.Equal(t, snapshotGarbageCollectionInterval, timer.duration)
59+ if tt.wantCollect {
60+ require.Len(t, collected, 1)
61+ <-collected
62+ }
63+ if pass < passes-1 {
64+ timer.ch <- time.Time{}
65+ }
7466 }
75- if pass < tt.passes-1 {
76- timer.ch <- time.Time{}
77- }
78- }
7967
80- d.serveCancel()
81- d.WaitDurableWriters()
68+ d.serveCancel()
69+ d.WaitDurableWriters()
70+ })
8271 })
8372 }
8473 }
8574
86-func receiveWithin[T any](t *testing.T, ch <-chan T, what string) T {
87- t.Helper()
88- select {
89- case v := <-ch:
90- return v
91- case <-time.After(5 * time.Second):
92- t.Fatalf("timed out waiting for %s", what)
93- panic("unreachable")
94- }
95-}
96-
97-func TestDurableMaintenanceRequiresExplicitOption(t *testing.T) {
75+func TestSnapshotGarbageCollectionRequiresExplicitOption(t *testing.T) {
9876 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
9977 coordinator := recoveryusecase.NewCoordinator(portsmocks.NewMockCatalogue(t), portsmocks.NewMockSnapshotRepository(t), nil)
10078 WithRecoveryCoordinator(coordinator)(d)
@@ -4,6 +4,7 @@ import (
4 "context"4 "context"
5 "errors"5 "errors"
6 "testing"6 "testing"
7+ "testing/synctest"
7 "time"8 "time"
8 9
9 "github.com/bnema/vev/internal/domain"10 "github.com/bnema/vev/internal/domain"
@@ -13,88 +14,65 @@ import (
13 "github.com/stretchr/testify/require"14 "github.com/stretchr/testify/require"
14 )15 )
15 16
16-func TestDurableMaintenanceCollectsSnapshotGarbage(t *testing.T) {17+func TestSnapshotGarbageCollectionRunsAfterRestorationThenPeriodically(t *testing.T) {
17 committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}}18 committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}}
18- records := []domain.CatalogueRecord{19+ records := []domain.CatalogueRecord{{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}}
19- {Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed},20+ id := records[0].IncarnationID
20- {Name: "fresh", IncarnationID: domain.IncarnationID{2}},
21- }
22- validKeep := mock.MatchedBy(func(keep map[domain.IncarnationID]domain.CheckpointRef) bool {
23- return len(keep) == 2 && keep[records[0].IncarnationID] == committed && keep[records[1].IncarnationID] == (domain.CheckpointRef{})
24- })
25 21
26 tests := []struct {22 tests := []struct {
27- name string23+ name string
28- // passes is the number of GC passes expected: the first after24+ recordsErr error
29- // restoration, each later one after a periodic timer tick.25+ wantCollect bool
30- passes int
31- recordsErr error
32- collectErr error
33- wantCollect bool
34- wantInterval time.Duration
35 }{26 }{
36- {name: "first pass after restoration", passes: 1, wantCollect: true, wantInterval: snapshotGarbageCollectionInterval},27+ {name: "collects after restoration and on every tick", wantCollect: true},
37- {name: "periodic pass after timer tick", passes: 2, wantCollect: true, wantInterval: snapshotGarbageCollectionInterval},28+ {name: "unreadable catalogue never collects", recordsErr: errors.New("catalogue read failed")},
38- {name: "unreadable catalogue never collects", passes: 1, recordsErr: errors.New("catalogue read failed"), wantInterval: snapshotGarbageCollectionInterval},
39- {name: "preempted pass retries sooner", passes: 1, wantCollect: true, collectErr: recoveryusecase.ErrGarbageCollectionPreempted, wantInterval: snapshotGarbageCollectionRetryInterval},
40 }29 }
41 for _, tt := range tests {30 for _, tt := range tests {
42 t.Run(tt.name, func(t *testing.T) {31 t.Run(tt.name, func(t *testing.T) {
43- catalogue := portsmocks.NewMockCatalogue(t)32+ synctest.Test(t, func(t *testing.T) {
44- repository := portsmocks.NewMockSnapshotRepository(t)33+ const passes = 2
45- collected := make(chan struct{}, tt.passes)34+ catalogue := portsmocks.NewMockCatalogue(t)
46- if tt.recordsErr != nil {35+ repository := portsmocks.NewMockSnapshotRepository(t)
47- catalogue.EXPECT().Records().Return(nil, tt.recordsErr).Times(tt.passes)36+ catalogue.EXPECT().Records().Return(records, tt.recordsErr).Times(passes)
48- } else {37+ repository.EXPECT().SnapshotIncarnations(mock.Anything).Return([]domain.IncarnationID{id}, nil).Times(passes)
49- catalogue.EXPECT().Records().Return(records, nil).Times(tt.passes)38+ collected := make(chan struct{}, passes)
50- }39+ if tt.wantCollect {
51- if tt.wantCollect {40+ repository.EXPECT().CollectIncarnationGarbage(mock.Anything, id, &committed).
52- repository.EXPECT().CollectGarbage(mock.Anything, validKeep).41+ Run(func(context.Context, domain.IncarnationID, *domain.CheckpointRef) { collected <- struct{}{} }).
53- Run(func(context.Context, map[domain.IncarnationID]domain.CheckpointRef) { collected <- struct{}{} }).42+ Return(nil).Times(passes)
54- Return(tt.collectErr).Times(tt.passes)43+ }
55- }
56 44
57- clk := &signalClock{timers: make(chan *signalTimer, 1)}45+ clk := &signalClock{timers: make(chan *signalTimer, 1)}
58- d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clk)46+ d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clk)
59- WithDurableMaintenance(catalogue, repository)(d)47+ WithRecoveryCoordinator(recoveryusecase.NewCoordinator(catalogue, repository, nil))(d)
60- d.startDurableMaintenance()48+ WithSnapshotGarbageCollection()(d)
49+ d.startDurableMaintenance()
61 50
62- select {51+ synctest.Wait()
63- case <-collected:52+ require.Empty(t, clk.timers, "GC must wait for restoration to finish")
64- t.Fatal("GC must wait for restoration to finish")53+ d.closeRestoreDone()
65- case <-time.After(20 * time.Millisecond):
66- }
67- d.closeRestoreDone()
68 54
69- for pass := range tt.passes {55+ for pass := range passes {
70- timer := receiveWithin(t, clk.timers, "GC interval timer")56+ synctest.Wait()
71- require.Equal(t, tt.wantInterval, timer.duration)57+ timer := <-clk.timers
72- if tt.wantCollect {58+ require.Equal(t, snapshotGarbageCollectionInterval, timer.duration)
73- receiveWithin(t, collected, "GC pass")59+ if tt.wantCollect {
60+ require.Len(t, collected, 1)
61+ <-collected
62+ }
63+ if pass < passes-1 {
64+ timer.ch <- time.Time{}
65+ }
74 }66 }
75- if pass < tt.passes-1 {
76- timer.ch <- time.Time{}
77- }
78- }
79 67
80- d.serveCancel()68+ d.serveCancel()
81- d.WaitDurableWriters()69+ d.WaitDurableWriters()
70+ })
82 })71 })
83 }72 }
84 }73 }
85 74
86-func receiveWithin[T any](t *testing.T, ch <-chan T, what string) T {75+func TestSnapshotGarbageCollectionRequiresExplicitOption(t *testing.T) {
87- t.Helper()
88- select {
89- case v := <-ch:
90- return v
91- case <-time.After(5 * time.Second):
92- t.Fatalf("timed out waiting for %s", what)
93- panic("unreachable")
94- }
95-}
96-
97-func TestDurableMaintenanceRequiresExplicitOption(t *testing.T) {
98 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})76 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
99 coordinator := recoveryusecase.NewCoordinator(portsmocks.NewMockCatalogue(t), portsmocks.NewMockSnapshotRepository(t), nil)77 coordinator := recoveryusecase.NewCoordinator(portsmocks.NewMockCatalogue(t), portsmocks.NewMockSnapshotRepository(t), nil)
100 WithRecoveryCoordinator(coordinator)(d)78 WithRecoveryCoordinator(coordinator)(d)
M internal/usecase/daemon/session_purge_retry_test.go +1 -1
@@ -26,7 +26,7 @@ func (r *retryablePurgeRepository) DeleteIncarnation(ctx context.Context, _ doma
2626 return r.deleteErr
2727 }
2828
29-func TestLivePurgeLeavesFailedDirectoryDeletionForStartupGarbageCollection(t *testing.T) {
29+func TestLivePurgeLeavesFailedDirectoryDeletionForBackgroundGarbageCollection(t *testing.T) {
3030 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
3131 repository := &retryablePurgeRepository{deleteErr: errors.New("delete failed")}
3232 WithSnapshotRepository(repository)(d)
@@ -26,7 +26,7 @@ func (r *retryablePurgeRepository) DeleteIncarnation(ctx context.Context, _ doma
26 return r.deleteErr26 return r.deleteErr
27 }27 }
28 28
29-func TestLivePurgeLeavesFailedDirectoryDeletionForStartupGarbageCollection(t *testing.T) {29+func TestLivePurgeLeavesFailedDirectoryDeletionForBackgroundGarbageCollection(t *testing.T) {
30 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})30 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
31 repository := &retryablePurgeRepository{deleteErr: errors.New("delete failed")}31 repository := &retryablePurgeRepository{deleteErr: errors.New("delete failed")}
32 WithSnapshotRepository(repository)(d)32 WithSnapshotRepository(repository)(d)
M internal/usecase/daemon/snapshot_resume_publication_test.go +5 -1
@@ -117,7 +117,11 @@ func (r *snapshotLifecycleRepository) DeleteIncarnation(ctx context.Context, id
117117 return nil
118118 }
119119
120-func (r *snapshotLifecycleRepository) CollectGarbage(ctx context.Context, _ map[domain.IncarnationID]domain.CheckpointRef) error {
120+func (r *snapshotLifecycleRepository) SnapshotIncarnations(ctx context.Context) ([]domain.IncarnationID, error) {
121+ return nil, ctx.Err()
122+}
123+
124+func (r *snapshotLifecycleRepository) CollectIncarnationGarbage(ctx context.Context, _ domain.IncarnationID, _ *domain.CheckpointRef) error {
121125 return ctx.Err()
122126 }
123127
@@ -117,7 +117,11 @@ func (r *snapshotLifecycleRepository) DeleteIncarnation(ctx context.Context, id
117 return nil117 return nil
118 }118 }
119 119
120-func (r *snapshotLifecycleRepository) CollectGarbage(ctx context.Context, _ map[domain.IncarnationID]domain.CheckpointRef) error {120+func (r *snapshotLifecycleRepository) SnapshotIncarnations(ctx context.Context) ([]domain.IncarnationID, error) {
121+ return nil, ctx.Err()
122+}
123+
124+func (r *snapshotLifecycleRepository) CollectIncarnationGarbage(ctx context.Context, _ domain.IncarnationID, _ *domain.CheckpointRef) error {
121 return ctx.Err()125 return ctx.Err()
122 }126 }
123 127
M internal/usecase/daemon/snapshot_test.go +4 -1
@@ -113,7 +113,10 @@ func (noOpSnapshotRepository) ReconcileCheckpoint(context.Context, domain.Incarn
113113 func (noOpSnapshotRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {
114114 return nil
115115 }
116-func (noOpSnapshotRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {
116+func (noOpSnapshotRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
117+ return nil, nil
118+}
119+func (noOpSnapshotRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
117120 return nil
118121 }
119122
@@ -113,7 +113,10 @@ func (noOpSnapshotRepository) ReconcileCheckpoint(context.Context, domain.Incarn
113 func (noOpSnapshotRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {113 func (noOpSnapshotRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error {
114 return nil114 return nil
115 }115 }
116-func (noOpSnapshotRepository) CollectGarbage(context.Context, map[domain.IncarnationID]domain.CheckpointRef) error {116+func (noOpSnapshotRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
117+ return nil, nil
118+}
119+func (noOpSnapshotRepository) CollectIncarnationGarbage(context.Context, domain.IncarnationID, *domain.CheckpointRef) error {
117 return nil120 return nil
118 }121 }
119 122
M internal/usecase/daemon/snapshot_worker.go +1 -3
@@ -262,9 +262,7 @@ func (d *Daemon) WaitDurableWriters() {
262262 func (d *Daemon) requestDurableWriterStop() (context.CancelFunc, <-chan struct{}) {
263263 d.snapshotWorkerMu.Lock()
264264 defer d.snapshotWorkerMu.Unlock()
265- if d.maintenanceWorkerCancel != nil {
266- d.maintenanceWorkerCancel()
267- }
265+ d.cancelDurableMaintenanceLocked()
268266 cancel := d.snapshotWorkerCancel
269267 if cancel == nil {
270268 return nil, nil
@@ -262,9 +262,7 @@ func (d *Daemon) WaitDurableWriters() {
262 func (d *Daemon) requestDurableWriterStop() (context.CancelFunc, <-chan struct{}) {262 func (d *Daemon) requestDurableWriterStop() (context.CancelFunc, <-chan struct{}) {
263 d.snapshotWorkerMu.Lock()263 d.snapshotWorkerMu.Lock()
264 defer d.snapshotWorkerMu.Unlock()264 defer d.snapshotWorkerMu.Unlock()
265- if d.maintenanceWorkerCancel != nil {265+ d.cancelDurableMaintenanceLocked()
266- d.maintenanceWorkerCancel()
267- }
268 cancel := d.snapshotWorkerCancel266 cancel := d.snapshotWorkerCancel
269 if cancel == nil {267 if cancel == nil {
270 return nil, nil268 return nil, nil
M internal/usecase/recovery/broken.go +1 -1
@@ -21,7 +21,7 @@ func (c *Coordinator) MarkBroken(ctx context.Context, name string, expected doma
2121 if reason == "" {
2222 return domain.CatalogueRecord{}, false, errors.New("recovery: empty broken-session reason")
2323 }
24- c.lockMutation()
24+ c.mutationMu.Lock()
2525 defer c.mutationMu.Unlock()
2626 unlock := c.locks.Lock([]string{name})
2727 defer unlock()
@@ -21,7 +21,7 @@ func (c *Coordinator) MarkBroken(ctx context.Context, name string, expected doma
21 if reason == "" {21 if reason == "" {
22 return domain.CatalogueRecord{}, false, errors.New("recovery: empty broken-session reason")22 return domain.CatalogueRecord{}, false, errors.New("recovery: empty broken-session reason")
23 }23 }
24- c.lockMutation()24+ c.mutationMu.Lock()
25 defer c.mutationMu.Unlock()25 defer c.mutationMu.Unlock()
26 unlock := c.locks.Lock([]string{name})26 unlock := c.locks.Lock([]string{name})
27 defer unlock()27 defer unlock()
M internal/usecase/recovery/checkpoint.go +1 -1
@@ -25,7 +25,7 @@ func (c *Coordinator) PublishCheckpoint(ctx context.Context, name string, public
2525 if err := ctx.Err(); err != nil {
2626 return domain.CatalogueRecord{}, err
2727 }
28- c.lockMutation()
28+ c.mutationMu.Lock()
2929 defer c.mutationMu.Unlock()
3030 unlock := c.locks.Lock([]string{name})
3131 defer unlock()
@@ -25,7 +25,7 @@ func (c *Coordinator) PublishCheckpoint(ctx context.Context, name string, public
25 if err := ctx.Err(); err != nil {25 if err := ctx.Err(); err != nil {
26 return domain.CatalogueRecord{}, err26 return domain.CatalogueRecord{}, err
27 }27 }
28- c.lockMutation()28+ c.mutationMu.Lock()
29 defer c.mutationMu.Unlock()29 defer c.mutationMu.Unlock()
30 unlock := c.locks.Lock([]string{name})30 unlock := c.locks.Lock([]string{name})
31 defer unlock()31 defer unlock()
M internal/usecase/recovery/coordinator.go +47 -71
@@ -85,36 +85,11 @@ type Coordinator struct {
8585 catalogue ports.Catalogue
8686 repository ports.SnapshotRepository
8787 locks *KeyLocks
88- // mutationMu serializes the GC keep-set snapshot and collection with every
89- // catalogue or checkpoint mutation. It is the only fence: GC runs while the
90- // daemon serves. Take it through lockMutation, never directly.
88+ // mutationMu serializes every catalogue or checkpoint mutation with each
89+ // per-incarnation GC step. It is the only fence: GC runs while the daemon
90+ // serves, so a step holds it for one incarnation at a time.
9191 mutationMu sync.Mutex
92- // gcMu guards gcCancel and mutationWaiters. A mutation preempts a running
93- // GC pass so durable writes never wait for maintenance.
94- gcMu sync.Mutex
95- gcCancel context.CancelFunc
96- mutationWaiters int
97- random io.Reader
98-}
99-
100-// ErrGarbageCollectionPreempted reports a GC pass that yielded to a durable
101-// mutation. Work completed before the yield is kept; the caller retries later.
102-var ErrGarbageCollectionPreempted = errors.New("recovery: garbage collection preempted by a durable mutation")
103-
104-// lockMutation acquires mutationMu after cancelling any running GC pass. The
105-// waiter count closes the window where GC has not yet published its cancel
106-// function: a pass that starts while a mutation is waiting does not run.
107-func (c *Coordinator) lockMutation() {
108- c.gcMu.Lock()
109- c.mutationWaiters++
110- if c.gcCancel != nil {
111- c.gcCancel()
112- }
113- c.gcMu.Unlock()
114- c.mutationMu.Lock()
115- c.gcMu.Lock()
116- c.mutationWaiters--
117- c.gcMu.Unlock()
92+ random io.Reader
11893 }
11994
12095 func NewCoordinator(catalogue ports.Catalogue, repository ports.SnapshotRepository, random io.Reader) *Coordinator {
@@ -130,7 +105,7 @@ func (c *Coordinator) Create(ctx context.Context, record domain.CatalogueRecord)
130105 if err := ctx.Err(); err != nil {
131106 return domain.CatalogueRecord{}, err
132107 }
133- c.lockMutation()
108+ c.mutationMu.Lock()
134109 defer c.mutationMu.Unlock()
135110 unlock := c.locks.Lock([]string{record.Name})
136111 defer unlock()
@@ -166,7 +141,7 @@ func (c *Coordinator) Rename(ctx context.Context, oldName, newName string) (doma
166141 if err := ctx.Err(); err != nil {
167142 return domain.CatalogueRecord{}, err
168143 }
169- c.lockMutation()
144+ c.mutationMu.Lock()
170145 defer c.mutationMu.Unlock()
171146 unlock := c.locks.Lock([]string{oldName, newName})
172147 defer unlock()
@@ -203,7 +178,7 @@ func (c *Coordinator) Delete(ctx context.Context, name string) error {
203178 if err := ctx.Err(); err != nil {
204179 return err
205180 }
206- c.lockMutation()
181+ c.mutationMu.Lock()
207182 defer c.mutationMu.Unlock()
208183 unlock := c.locks.Lock([]string{name})
209184 defer unlock()
@@ -230,7 +205,7 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation
230205 if err := ctx.Err(); err != nil {
231206 return err
232207 }
233- c.lockMutation()
208+ c.mutationMu.Lock()
234209 defer c.mutationMu.Unlock()
235210 unlock := c.locks.Lock([]string{name})
236211 defer unlock()
@@ -247,10 +222,16 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation
247222 return c.repository.DeleteIncarnation(ctx, incarnation)
248223 }
249224
250-// CollectGarbage takes a catalogue snapshot and applies retention while all
251-// catalogue mutations and checkpoint publications are fenced. Its keep map is
252-// therefore never stale relative to an operation that creates an incarnation
253-// or commits a new generation.
225+// errGarbageCollectionCatalogue stops a pass: without a readable catalogue no
226+// incarnation can be classified.
227+var errGarbageCollectionCatalogue = errors.New("recovery: read catalogue for garbage collection")
228+
229+// CollectGarbage applies retention to every repository incarnation. Each
230+// incarnation is collected in its own fenced step that re-reads the catalogue
231+// under mutationMu, so its keep decision is never stale relative to an
232+// operation that creates an incarnation or commits a generation, and a
233+// mutation waits for at most one incarnation instead of a whole pass. Failures
234+// are joined so one bad incarnation does not stop the rest.
254235 func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) {
255236 if c == nil || c.catalogue == nil || c.repository == nil {
256237 return 0, errors.New("recovery: incomplete garbage collection dependencies")
@@ -258,48 +239,43 @@ func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) {
258239 if err := ctx.Err(); err != nil {
259240 return 0, err
260241 }
261- c.gcMu.Lock()
262- if c.mutationWaiters > 0 {
263- c.gcMu.Unlock()
264- return 0, ErrGarbageCollectionPreempted
265- }
266- gcCtx, cancel := context.WithCancel(ctx)
267- defer cancel()
268- c.gcCancel = cancel
269- c.gcMu.Unlock()
270- defer func() {
271- c.gcMu.Lock()
272- c.gcCancel = nil
273- c.gcMu.Unlock()
274- }()
275-
276- // GC takes mutationMu directly: it must not preempt itself.
277- c.mutationMu.Lock()
278- defer c.mutationMu.Unlock()
279- preempted := func(err error) error {
280- if gcCtx.Err() != nil && ctx.Err() == nil {
281- return ErrGarbageCollectionPreempted
282- }
283- return err
242+ ids, err := c.repository.SnapshotIncarnations(ctx)
243+ if err != nil {
244+ return 0, err
284245 }
285- if err := gcCtx.Err(); err != nil {
286- return 0, preempted(err)
246+ var collected error
247+ for _, id := range ids {
248+ if err := ctx.Err(); err != nil {
249+ return len(ids), errors.Join(collected, err)
250+ }
251+ if err := c.collectIncarnation(ctx, id); errors.Is(err, errGarbageCollectionCatalogue) {
252+ return len(ids), errors.Join(collected, err)
253+ } else if err != nil {
254+ collected = errors.Join(collected, err)
255+ }
287256 }
257+ return len(ids), collected
258+}
288259
260+func (c *Coordinator) collectIncarnation(ctx context.Context, id domain.IncarnationID) error {
261+ c.mutationMu.Lock()
262+ defer c.mutationMu.Unlock()
263+ // A catalogue read failure never produces a keep decision, so destructive
264+ // collection cannot run without known-good catalogue state.
289265 records, err := c.catalogue.Records()
290266 if err != nil {
291- return 0, err
267+ return fmt.Errorf("%w: %w", errGarbageCollectionCatalogue, err)
292268 }
293- keep := make(map[domain.IncarnationID]domain.CheckpointRef, len(records))
269+ var keep *domain.CheckpointRef
294270 for _, record := range records {
295- if record.Committed != nil {
296- keep[record.IncarnationID] = *record.Committed
271+ if record.IncarnationID != id {
297272 continue
298273 }
299- keep[record.IncarnationID] = domain.CheckpointRef{}
300- }
301- if err := c.repository.CollectGarbage(gcCtx, keep); err != nil {
302- return len(keep), preempted(err)
274+ keep = &domain.CheckpointRef{}
275+ if record.Committed != nil {
276+ *keep = *record.Committed
277+ }
278+ break
303279 }
304- return len(keep), nil
280+ return c.repository.CollectIncarnationGarbage(ctx, id, keep)
305281 }
@@ -85,36 +85,11 @@ type Coordinator struct {
85 catalogue ports.Catalogue85 catalogue ports.Catalogue
86 repository ports.SnapshotRepository86 repository ports.SnapshotRepository
87 locks *KeyLocks87 locks *KeyLocks
88- // mutationMu serializes the GC keep-set snapshot and collection with every88+ // mutationMu serializes every catalogue or checkpoint mutation with each
89- // catalogue or checkpoint mutation. It is the only fence: GC runs while the89+ // per-incarnation GC step. It is the only fence: GC runs while the daemon
90- // daemon serves. Take it through lockMutation, never directly.90+ // serves, so a step holds it for one incarnation at a time.
91 mutationMu sync.Mutex91 mutationMu sync.Mutex
92- // gcMu guards gcCancel and mutationWaiters. A mutation preempts a running92+ random io.Reader
93- // GC pass so durable writes never wait for maintenance.
94- gcMu sync.Mutex
95- gcCancel context.CancelFunc
96- mutationWaiters int
97- random io.Reader
98-}
99-
100-// ErrGarbageCollectionPreempted reports a GC pass that yielded to a durable
101-// mutation. Work completed before the yield is kept; the caller retries later.
102-var ErrGarbageCollectionPreempted = errors.New("recovery: garbage collection preempted by a durable mutation")
103-
104-// lockMutation acquires mutationMu after cancelling any running GC pass. The
105-// waiter count closes the window where GC has not yet published its cancel
106-// function: a pass that starts while a mutation is waiting does not run.
107-func (c *Coordinator) lockMutation() {
108- c.gcMu.Lock()
109- c.mutationWaiters++
110- if c.gcCancel != nil {
111- c.gcCancel()
112- }
113- c.gcMu.Unlock()
114- c.mutationMu.Lock()
115- c.gcMu.Lock()
116- c.mutationWaiters--
117- c.gcMu.Unlock()
118 }93 }
119 94
120 func NewCoordinator(catalogue ports.Catalogue, repository ports.SnapshotRepository, random io.Reader) *Coordinator {95 func NewCoordinator(catalogue ports.Catalogue, repository ports.SnapshotRepository, random io.Reader) *Coordinator {
@@ -130,7 +105,7 @@ func (c *Coordinator) Create(ctx context.Context, record domain.CatalogueRecord)
130 if err := ctx.Err(); err != nil {105 if err := ctx.Err(); err != nil {
131 return domain.CatalogueRecord{}, err106 return domain.CatalogueRecord{}, err
132 }107 }
133- c.lockMutation()108+ c.mutationMu.Lock()
134 defer c.mutationMu.Unlock()109 defer c.mutationMu.Unlock()
135 unlock := c.locks.Lock([]string{record.Name})110 unlock := c.locks.Lock([]string{record.Name})
136 defer unlock()111 defer unlock()
@@ -166,7 +141,7 @@ func (c *Coordinator) Rename(ctx context.Context, oldName, newName string) (doma
166 if err := ctx.Err(); err != nil {141 if err := ctx.Err(); err != nil {
167 return domain.CatalogueRecord{}, err142 return domain.CatalogueRecord{}, err
168 }143 }
169- c.lockMutation()144+ c.mutationMu.Lock()
170 defer c.mutationMu.Unlock()145 defer c.mutationMu.Unlock()
171 unlock := c.locks.Lock([]string{oldName, newName})146 unlock := c.locks.Lock([]string{oldName, newName})
172 defer unlock()147 defer unlock()
@@ -203,7 +178,7 @@ func (c *Coordinator) Delete(ctx context.Context, name string) error {
203 if err := ctx.Err(); err != nil {178 if err := ctx.Err(); err != nil {
204 return err179 return err
205 }180 }
206- c.lockMutation()181+ c.mutationMu.Lock()
207 defer c.mutationMu.Unlock()182 defer c.mutationMu.Unlock()
208 unlock := c.locks.Lock([]string{name})183 unlock := c.locks.Lock([]string{name})
209 defer unlock()184 defer unlock()
@@ -230,7 +205,7 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation
230 if err := ctx.Err(); err != nil {205 if err := ctx.Err(); err != nil {
231 return err206 return err
232 }207 }
233- c.lockMutation()208+ c.mutationMu.Lock()
234 defer c.mutationMu.Unlock()209 defer c.mutationMu.Unlock()
235 unlock := c.locks.Lock([]string{name})210 unlock := c.locks.Lock([]string{name})
236 defer unlock()211 defer unlock()
@@ -247,10 +222,16 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation
247 return c.repository.DeleteIncarnation(ctx, incarnation)222 return c.repository.DeleteIncarnation(ctx, incarnation)
248 }223 }
249 224
250-// CollectGarbage takes a catalogue snapshot and applies retention while all225+// errGarbageCollectionCatalogue stops a pass: without a readable catalogue no
251-// catalogue mutations and checkpoint publications are fenced. Its keep map is226+// incarnation can be classified.
252-// therefore never stale relative to an operation that creates an incarnation227+var errGarbageCollectionCatalogue = errors.New("recovery: read catalogue for garbage collection")
253-// or commits a new generation.228+
229+// CollectGarbage applies retention to every repository incarnation. Each
230+// incarnation is collected in its own fenced step that re-reads the catalogue
231+// under mutationMu, so its keep decision is never stale relative to an
232+// operation that creates an incarnation or commits a generation, and a
233+// mutation waits for at most one incarnation instead of a whole pass. Failures
234+// are joined so one bad incarnation does not stop the rest.
254 func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) {235 func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) {
255 if c == nil || c.catalogue == nil || c.repository == nil {236 if c == nil || c.catalogue == nil || c.repository == nil {
256 return 0, errors.New("recovery: incomplete garbage collection dependencies")237 return 0, errors.New("recovery: incomplete garbage collection dependencies")
@@ -258,48 +239,43 @@ func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) {
258 if err := ctx.Err(); err != nil {239 if err := ctx.Err(); err != nil {
259 return 0, err240 return 0, err
260 }241 }
261- c.gcMu.Lock()242+ ids, err := c.repository.SnapshotIncarnations(ctx)
262- if c.mutationWaiters > 0 {243+ if err != nil {
263- c.gcMu.Unlock()244+ return 0, err
264- return 0, ErrGarbageCollectionPreempted
265- }
266- gcCtx, cancel := context.WithCancel(ctx)
267- defer cancel()
268- c.gcCancel = cancel
269- c.gcMu.Unlock()
270- defer func() {
271- c.gcMu.Lock()
272- c.gcCancel = nil
273- c.gcMu.Unlock()
274- }()
275-
276- // GC takes mutationMu directly: it must not preempt itself.
277- c.mutationMu.Lock()
278- defer c.mutationMu.Unlock()
279- preempted := func(err error) error {
280- if gcCtx.Err() != nil && ctx.Err() == nil {
281- return ErrGarbageCollectionPreempted
282- }
283- return err
284 }245 }
285- if err := gcCtx.Err(); err != nil {246+ var collected error
286- return 0, preempted(err)247+ for _, id := range ids {
248+ if err := ctx.Err(); err != nil {
249+ return len(ids), errors.Join(collected, err)
250+ }
251+ if err := c.collectIncarnation(ctx, id); errors.Is(err, errGarbageCollectionCatalogue) {
252+ return len(ids), errors.Join(collected, err)
253+ } else if err != nil {
254+ collected = errors.Join(collected, err)
255+ }
287 }256 }
257+ return len(ids), collected
258+}
288 259
260+func (c *Coordinator) collectIncarnation(ctx context.Context, id domain.IncarnationID) error {
261+ c.mutationMu.Lock()
262+ defer c.mutationMu.Unlock()
263+ // A catalogue read failure never produces a keep decision, so destructive
264+ // collection cannot run without known-good catalogue state.
289 records, err := c.catalogue.Records()265 records, err := c.catalogue.Records()
290 if err != nil {266 if err != nil {
291- return 0, err267+ return fmt.Errorf("%w: %w", errGarbageCollectionCatalogue, err)
292 }268 }
293- keep := make(map[domain.IncarnationID]domain.CheckpointRef, len(records))269+ var keep *domain.CheckpointRef
294 for _, record := range records {270 for _, record := range records {
295- if record.Committed != nil {271+ if record.IncarnationID != id {
296- keep[record.IncarnationID] = *record.Committed
297 continue272 continue
298 }273 }
299- keep[record.IncarnationID] = domain.CheckpointRef{}274+ keep = &domain.CheckpointRef{}
300- }275+ if record.Committed != nil {
301- if err := c.repository.CollectGarbage(gcCtx, keep); err != nil {276+ *keep = *record.Committed
302- return len(keep), preempted(err)277+ }
278+ break
303 }279 }
304- return len(keep), nil280+ return c.repository.CollectIncarnationGarbage(ctx, id, keep)
305 }281 }
M internal/usecase/recovery/discard.go +1 -1
@@ -22,7 +22,7 @@ func (c *Coordinator) Discard(ctx context.Context, name string) error {
2222 if err := ctx.Err(); err != nil {
2323 return err
2424 }
25- c.lockMutation()
25+ c.mutationMu.Lock()
2626 defer c.mutationMu.Unlock()
2727 unlock := c.locks.Lock([]string{name})
2828 defer unlock()
@@ -22,7 +22,7 @@ func (c *Coordinator) Discard(ctx context.Context, name string) error {
22 if err := ctx.Err(); err != nil {22 if err := ctx.Err(); err != nil {
23 return err23 return err
24 }24 }
25- c.lockMutation()25+ c.mutationMu.Lock()
26 defer c.mutationMu.Unlock()26 defer c.mutationMu.Unlock()
27 unlock := c.locks.Lock([]string{name})27 unlock := c.locks.Lock([]string{name})
28 defer unlock()28 defer unlock()
M internal/usecase/recovery/reset.go +1 -1
@@ -17,7 +17,7 @@ func (c *Coordinator) ResetIncompatible(ctx context.Context, name string, expect
1717 if err := ctx.Err(); err != nil {
1818 return domain.CatalogueRecord{}, false, err
1919 }
20- c.lockMutation()
20+ c.mutationMu.Lock()
2121 defer c.mutationMu.Unlock()
2222 unlock := c.locks.Lock([]string{name})
2323 defer unlock()
@@ -17,7 +17,7 @@ func (c *Coordinator) ResetIncompatible(ctx context.Context, name string, expect
17 if err := ctx.Err(); err != nil {17 if err := ctx.Err(); err != nil {
18 return domain.CatalogueRecord{}, false, err18 return domain.CatalogueRecord{}, false, err
19 }19 }
20- c.lockMutation()20+ c.mutationMu.Lock()
21 defer c.mutationMu.Unlock()21 defer c.mutationMu.Unlock()
22 unlock := c.locks.Lock([]string{name})22 unlock := c.locks.Lock([]string{name})
23 defer unlock()23 defer unlock()
M internal/usecase/recovery/transactions_test.go +132 -74
@@ -6,7 +6,6 @@ import (
66 "errors"
77 "sync"
88 "testing"
9- "time"
109
1110 "github.com/bnema/vev/internal/domain"
1211 "github.com/bnema/vev/internal/ports"
@@ -114,51 +113,73 @@ type transactionRepository struct {
114113 releaseDelete chan struct{}
115114 }
116115
116+// garbageCollectionFenceRepository models per-incarnation retention. The first
117+// CollectIncarnationGarbage call blocks until released, so a test can race a
118+// mutation against a step that already read its keep decision.
117119 type garbageCollectionFenceRepository struct {
118120 ports.SnapshotRepository
119- mu sync.Mutex
120- events []string
121- collectionEntered chan struct{}
122- releaseCollection chan struct{}
123- collectionFinished bool
124- publishBeforeGC bool
125- incarnations map[domain.IncarnationID]struct{}
126- generations map[domain.IncarnationID]map[uint64]struct{}
127-}
128-
129-func (r *garbageCollectionFenceRepository) CollectGarbage(_ context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error {
130- close(r.collectionEntered)
131- <-r.releaseCollection
121+ mu sync.Mutex
122+ events []string
123+ collectionEntered chan struct{}
124+ releaseCollection chan struct{}
125+ blocked bool
126+ incarnations map[domain.IncarnationID]struct{}
127+ generations map[domain.IncarnationID]map[uint64]struct{}
128+}
129+
130+func (r *garbageCollectionFenceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
132131 r.mu.Lock()
133132 defer r.mu.Unlock()
133+ seen := make(map[domain.IncarnationID]struct{})
134134 for id := range r.incarnations {
135- if _, known := keep[id]; !known {
136- delete(r.incarnations, id)
137- }
135+ seen[id] = struct{}{}
138136 }
139- for id, generations := range r.generations {
140- committed, known := keep[id]
141- if !known {
142- delete(r.generations, id)
143- continue
144- }
145- for generation := range generations {
146- if generation != committed.Generation && generation+1 != committed.Generation {
147- delete(generations, generation)
137+ for id := range r.generations {
138+ seen[id] = struct{}{}
139+ }
140+ ids := make([]domain.IncarnationID, 0, len(seen))
141+ for id := range seen {
142+ ids = append(ids, id)
143+ }
144+ return ids, nil
145+}
146+
147+func (r *garbageCollectionFenceRepository) CollectIncarnationGarbage(_ context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
148+ r.mu.Lock()
149+ block := !r.blocked
150+ r.blocked = true
151+ r.mu.Unlock()
152+ if block {
153+ close(r.collectionEntered)
154+ <-r.releaseCollection
155+ }
156+ r.mu.Lock()
157+ defer r.mu.Unlock()
158+ if keep == nil {
159+ delete(r.incarnations, id)
160+ delete(r.generations, id)
161+ } else {
162+ for generation := range r.generations[id] {
163+ if generation != keep.Generation && generation+1 != keep.Generation {
164+ delete(r.generations[id], generation)
148165 }
149166 }
150167 }
151- r.collectionFinished = true
152168 r.events = append(r.events, "collect")
153169 return nil
154170 }
155171
172+func (r *garbageCollectionFenceRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error {
173+ r.mu.Lock()
174+ defer r.mu.Unlock()
175+ delete(r.incarnations, id)
176+ delete(r.generations, id)
177+ return nil
178+}
179+
156180 func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error {
157181 r.mu.Lock()
158182 defer r.mu.Unlock()
159- if !r.collectionFinished {
160- r.publishBeforeGC = true
161- }
162183 if r.generations == nil {
163184 r.generations = make(map[domain.IncarnationID]map[uint64]struct{})
164185 }
@@ -193,10 +214,10 @@ func (r *garbageCollectionFenceRepository) hasGeneration(id domain.IncarnationID
193214 return ok
194215 }
195216
196-func (r *garbageCollectionFenceRepository) eventLog() ([]string, bool) {
217+func (r *garbageCollectionFenceRepository) eventLog() []string {
197218 r.mu.Lock()
198219 defer r.mu.Unlock()
199- return append([]string(nil), r.events...), r.publishBeforeGC
220+ return append([]string(nil), r.events...)
200221 }
201222
202223 func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error {
@@ -382,18 +403,22 @@ func TestResetIncompatibleStaleAuthorityIsNoOp(t *testing.T) {
382403 }
383404 }
384405
385-func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) {
406+func TestGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) {
386407 t.Parallel()
387408
388- t.Run("a stale keep snapshot cannot remove a newly created incarnation", func(t *testing.T) {
409+ t.Run("a keep decision is re-read under the fence for every incarnation", func(t *testing.T) {
389410 t.Parallel()
411+ // The pass lists incarnation 9 (an orphan) before "new" is created with
412+ // that same ID; its step must observe the creation and keep it.
413+ reused := domain.IncarnationID{9}
390414 catalogue := newTransactionCatalogue()
391415 repository := &garbageCollectionFenceRepository{
392416 collectionEntered: make(chan struct{}),
393417 releaseCollection: make(chan struct{}),
418+ incarnations: map[domain.IncarnationID]struct{}{{1}: {}, reused: {}},
394419 }
395420 catalogue.onCreate = repository.addIncarnation
396- coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16)))
421+ coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(nil))
397422
398423 collected := make(chan error, 1)
399424 go func() {
@@ -404,23 +429,18 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(
404429
405430 created := make(chan error, 1)
406431 go func() {
407- _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new"})
432+ _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new", IncarnationID: reused})
408433 created <- err
409434 }()
410435 close(repository.releaseCollection)
411436
412437 require.NoError(t, <-collected)
413438 require.NoError(t, <-created)
414- got, ok, err := catalogue.Record("new")
415- require.NoError(t, err)
416- require.True(t, ok)
417- require.NotZero(t, got.IncarnationID)
418- require.True(t, repository.hasIncarnation(got.IncarnationID), "GC's stale keep snapshot must finish before the new incarnation is published")
419- events, _ := repository.eventLog()
420- require.Equal(t, []string{"collect"}, events)
439+ require.True(t, repository.hasIncarnation(reused), "a step after the creation must keep the new incarnation")
440+ require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "a true orphan is still removed")
421441 })
422442
423- t.Run("a stale keep snapshot cannot prune a newly committed generation", func(t *testing.T) {
443+ t.Run("a stale keep decision cannot prune a newly committed generation", func(t *testing.T) {
424444 t.Parallel()
425445 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}
426446 record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}
@@ -456,29 +476,39 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(
456476
457477 require.NoError(t, <-collected)
458478 require.NoError(t, <-published)
459- events, publishedBeforeGC := repository.eventLog()
460- require.Equal(t, []string{"collect", "publish"}, events)
461- require.False(t, publishedBeforeGC)
462- require.True(t, repository.hasGeneration(record.IncarnationID, 2), "GC's stale keep snapshot must finish before generation two is published")
479+ require.Equal(t, []string{"collect", "publish"}, repository.eventLog())
480+ require.True(t, repository.hasGeneration(record.IncarnationID, 2), "the fenced step must finish before generation two is published")
463481 })
464482 }
465483
466-// cooperativeGarbageCollectionRepository blocks a GC pass until its context is
467-// cancelled, like the filesystem repository between entries.
468-type cooperativeGarbageCollectionRepository struct {
469- garbageCollectionFenceRepository
484+// betweenStepsContext pauses a GC pass at its first cancellation check after
485+// one incarnation step has completed, outside any fenced step.
486+type betweenStepsContext struct {
487+ context.Context
488+ repository *garbageCollectionFenceRepository
489+ once sync.Once
490+ reached chan struct{}
491+ release chan struct{}
470492 }
471493
472-func (r *cooperativeGarbageCollectionRepository) CollectGarbage(ctx context.Context, _ map[domain.IncarnationID]domain.CheckpointRef) error {
473- close(r.collectionEntered)
474- <-ctx.Done()
475- return ctx.Err()
494+func (c *betweenStepsContext) Err() error {
495+ if len(c.repository.eventLog()) == 1 {
496+ c.once.Do(func() {
497+ close(c.reached)
498+ <-c.release
499+ })
500+ }
501+ return c.Context.Err()
476502 }
477503
478-func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {
504+// TestDurableMutationWaitsForAtMostOneIncarnation proves the fence is released
505+// between incarnation steps, so every mutation entry point can run while a
506+// pass is still in progress instead of waiting for the whole pass.
507+func TestDurableMutationWaitsForAtMostOneIncarnation(t *testing.T) {
479508 t.Parallel()
480509 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}
481- record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}
510+ work := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}
511+ broken := degradedTransactionRecord()
482512 tests := []struct {
483513 name string
484514 mutate func(*Coordinator) error
@@ -487,9 +517,26 @@ func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {
487517 _, err := c.Create(context.Background(), domain.CatalogueRecord{Name: "new"})
488518 return err
489519 }},
520+ {name: "rename", mutate: func(c *Coordinator) error {
521+ _, err := c.Rename(context.Background(), work.Name, "renamed")
522+ return err
523+ }},
524+ {name: "delete", mutate: func(c *Coordinator) error { return c.Delete(context.Background(), work.Name) }},
525+ {name: "exact delete", mutate: func(c *Coordinator) error {
526+ return c.DeleteExact(context.Background(), work.Name, work.IncarnationID, work.CreatedAt)
527+ }},
528+ {name: "mark broken", mutate: func(c *Coordinator) error {
529+ _, _, err := c.MarkBroken(context.Background(), work.Name, work.IncarnationID, "restore failed")
530+ return err
531+ }},
532+ {name: "discard", mutate: func(c *Coordinator) error { return c.Discard(context.Background(), broken.Name) }},
533+ {name: "reset incompatible", mutate: func(c *Coordinator) error {
534+ _, _, err := c.ResetIncompatible(context.Background(), work.Name, work.IncarnationID, committed)
535+ return err
536+ }},
490537 {name: "checkpoint publication", mutate: func(c *Coordinator) error {
491- _, err := c.PublishCheckpoint(context.Background(), "work", ports.SnapshotPublication{
492- Name: "work", IncarnationID: record.IncarnationID, Generation: 2,
538+ _, err := c.PublishCheckpoint(context.Background(), work.Name, ports.SnapshotPublication{
539+ Name: work.Name, IncarnationID: work.IncarnationID, Generation: 2,
493540 ParentCheckpoint: &committed, Manifest: []byte("generation two"),
494541 })
495542 return err
@@ -498,25 +545,36 @@ func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {
498545 for _, tt := range tests {
499546 t.Run(tt.name, func(t *testing.T) {
500547 t.Parallel()
501- repository := &cooperativeGarbageCollectionRepository{garbageCollectionFenceRepository{collectionEntered: make(chan struct{})}}
502- coordinator := NewCoordinator(newTransactionCatalogue(record), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16)))
548+ // Two orphan incarnations give the pass two fenced steps.
549+ repository := &garbageCollectionFenceRepository{
550+ blocked: true,
551+ incarnations: map[domain.IncarnationID]struct{}{{7}: {}, {8}: {}},
552+ }
553+ coordinator := NewCoordinator(newTransactionCatalogue(work, broken), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 32)))
554+ ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})}
503555
504556 collected := make(chan error, 1)
505557 go func() {
506- _, err := coordinator.CollectGarbage(context.Background())
558+ _, err := coordinator.CollectGarbage(ctx)
507559 collected <- err
508560 }()
509- <-repository.collectionEntered
510-
511- mutated := make(chan error, 1)
512- go func() { mutated <- tt.mutate(coordinator) }()
513- select {
514- case err := <-mutated:
515- require.NoError(t, err)
516- case <-time.After(5 * time.Second):
517- t.Fatal("mutation waited for the GC pass instead of preempting it")
561+ <-ctx.reached
562+
563+ // TryLock first so a regression that holds the fence across the pass
564+ // fails here instead of deadlocking the mutation.
565+ require.True(t, coordinator.mutationMu.TryLock(), "the fence must be free between incarnation steps")
566+ coordinator.mutationMu.Unlock()
567+ require.NoError(t, tt.mutate(coordinator))
568+
569+ close(ctx.release)
570+ require.NoError(t, <-collected)
571+ steps := 0
572+ for _, event := range repository.eventLog() {
573+ if event == "collect" {
574+ steps++
575+ }
518576 }
519- require.ErrorIs(t, <-collected, ErrGarbageCollectionPreempted)
577+ require.Equal(t, 2, steps, "both incarnation steps ran")
520578 })
521579 }
522580 }
@@ -656,7 +714,7 @@ func TestDiscardIsRetryIdempotent(t *testing.T) {
656714 if tt.failBefore {
657715 require.Contains(t, repository.deletedIDs, old.IncarnationID)
658716 } else {
659- require.Empty(t, repository.deletedIDs, "startup garbage collection owns the orphaned old directory")
717+ require.Empty(t, repository.deletedIDs, "background garbage collection owns the orphaned old directory")
660718 }
661719 })
662720 }
@@ -6,7 +6,6 @@ import (
6 "errors"6 "errors"
7 "sync"7 "sync"
8 "testing"8 "testing"
9- "time"
10 9
11 "github.com/bnema/vev/internal/domain"10 "github.com/bnema/vev/internal/domain"
12 "github.com/bnema/vev/internal/ports"11 "github.com/bnema/vev/internal/ports"
@@ -114,51 +113,73 @@ type transactionRepository struct {
114 releaseDelete chan struct{}113 releaseDelete chan struct{}
115 }114 }
116 115
116+// garbageCollectionFenceRepository models per-incarnation retention. The first
117+// CollectIncarnationGarbage call blocks until released, so a test can race a
118+// mutation against a step that already read its keep decision.
117 type garbageCollectionFenceRepository struct {119 type garbageCollectionFenceRepository struct {
118 ports.SnapshotRepository120 ports.SnapshotRepository
119- mu sync.Mutex121+ mu sync.Mutex
120- events []string122+ events []string
121- collectionEntered chan struct{}123+ collectionEntered chan struct{}
122- releaseCollection chan struct{}124+ releaseCollection chan struct{}
123- collectionFinished bool125+ blocked bool
124- publishBeforeGC bool126+ incarnations map[domain.IncarnationID]struct{}
125- incarnations map[domain.IncarnationID]struct{}127+ generations map[domain.IncarnationID]map[uint64]struct{}
126- generations map[domain.IncarnationID]map[uint64]struct{}128+}
127-}129+
128-130+func (r *garbageCollectionFenceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) {
129-func (r *garbageCollectionFenceRepository) CollectGarbage(_ context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error {
130- close(r.collectionEntered)
131- <-r.releaseCollection
132 r.mu.Lock()131 r.mu.Lock()
133 defer r.mu.Unlock()132 defer r.mu.Unlock()
133+ seen := make(map[domain.IncarnationID]struct{})
134 for id := range r.incarnations {134 for id := range r.incarnations {
135- if _, known := keep[id]; !known {135+ seen[id] = struct{}{}
136- delete(r.incarnations, id)
137- }
138 }136 }
139- for id, generations := range r.generations {137+ for id := range r.generations {
140- committed, known := keep[id]138+ seen[id] = struct{}{}
141- if !known {139+ }
142- delete(r.generations, id)140+ ids := make([]domain.IncarnationID, 0, len(seen))
143- continue141+ for id := range seen {
144- }142+ ids = append(ids, id)
145- for generation := range generations {143+ }
146- if generation != committed.Generation && generation+1 != committed.Generation {144+ return ids, nil
147- delete(generations, generation)145+}
146+
147+func (r *garbageCollectionFenceRepository) CollectIncarnationGarbage(_ context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error {
148+ r.mu.Lock()
149+ block := !r.blocked
150+ r.blocked = true
151+ r.mu.Unlock()
152+ if block {
153+ close(r.collectionEntered)
154+ <-r.releaseCollection
155+ }
156+ r.mu.Lock()
157+ defer r.mu.Unlock()
158+ if keep == nil {
159+ delete(r.incarnations, id)
160+ delete(r.generations, id)
161+ } else {
162+ for generation := range r.generations[id] {
163+ if generation != keep.Generation && generation+1 != keep.Generation {
164+ delete(r.generations[id], generation)
148 }165 }
149 }166 }
150 }167 }
151- r.collectionFinished = true
152 r.events = append(r.events, "collect")168 r.events = append(r.events, "collect")
153 return nil169 return nil
154 }170 }
155 171
172+func (r *garbageCollectionFenceRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error {
173+ r.mu.Lock()
174+ defer r.mu.Unlock()
175+ delete(r.incarnations, id)
176+ delete(r.generations, id)
177+ return nil
178+}
179+
156 func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error {180 func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error {
157 r.mu.Lock()181 r.mu.Lock()
158 defer r.mu.Unlock()182 defer r.mu.Unlock()
159- if !r.collectionFinished {
160- r.publishBeforeGC = true
161- }
162 if r.generations == nil {183 if r.generations == nil {
163 r.generations = make(map[domain.IncarnationID]map[uint64]struct{})184 r.generations = make(map[domain.IncarnationID]map[uint64]struct{})
164 }185 }
@@ -193,10 +214,10 @@ func (r *garbageCollectionFenceRepository) hasGeneration(id domain.IncarnationID
193 return ok214 return ok
194 }215 }
195 216
196-func (r *garbageCollectionFenceRepository) eventLog() ([]string, bool) {217+func (r *garbageCollectionFenceRepository) eventLog() []string {
197 r.mu.Lock()218 r.mu.Lock()
198 defer r.mu.Unlock()219 defer r.mu.Unlock()
199- return append([]string(nil), r.events...), r.publishBeforeGC220+ return append([]string(nil), r.events...)
200 }221 }
201 222
202 func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error {223 func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error {
@@ -382,18 +403,22 @@ func TestResetIncompatibleStaleAuthorityIsNoOp(t *testing.T) {
382 }403 }
383 }404 }
384 405
385-func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) {406+func TestGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) {
386 t.Parallel()407 t.Parallel()
387 408
388- t.Run("a stale keep snapshot cannot remove a newly created incarnation", func(t *testing.T) {409+ t.Run("a keep decision is re-read under the fence for every incarnation", func(t *testing.T) {
389 t.Parallel()410 t.Parallel()
411+ // The pass lists incarnation 9 (an orphan) before "new" is created with
412+ // that same ID; its step must observe the creation and keep it.
413+ reused := domain.IncarnationID{9}
390 catalogue := newTransactionCatalogue()414 catalogue := newTransactionCatalogue()
391 repository := &garbageCollectionFenceRepository{415 repository := &garbageCollectionFenceRepository{
392 collectionEntered: make(chan struct{}),416 collectionEntered: make(chan struct{}),
393 releaseCollection: make(chan struct{}),417 releaseCollection: make(chan struct{}),
418+ incarnations: map[domain.IncarnationID]struct{}{{1}: {}, reused: {}},
394 }419 }
395 catalogue.onCreate = repository.addIncarnation420 catalogue.onCreate = repository.addIncarnation
396- coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16)))421+ coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(nil))
397 422
398 collected := make(chan error, 1)423 collected := make(chan error, 1)
399 go func() {424 go func() {
@@ -404,23 +429,18 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(
404 429
405 created := make(chan error, 1)430 created := make(chan error, 1)
406 go func() {431 go func() {
407- _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new"})432+ _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new", IncarnationID: reused})
408 created <- err433 created <- err
409 }()434 }()
410 close(repository.releaseCollection)435 close(repository.releaseCollection)
411 436
412 require.NoError(t, <-collected)437 require.NoError(t, <-collected)
413 require.NoError(t, <-created)438 require.NoError(t, <-created)
414- got, ok, err := catalogue.Record("new")439+ require.True(t, repository.hasIncarnation(reused), "a step after the creation must keep the new incarnation")
415- require.NoError(t, err)440+ require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "a true orphan is still removed")
416- require.True(t, ok)
417- require.NotZero(t, got.IncarnationID)
418- require.True(t, repository.hasIncarnation(got.IncarnationID), "GC's stale keep snapshot must finish before the new incarnation is published")
419- events, _ := repository.eventLog()
420- require.Equal(t, []string{"collect"}, events)
421 })441 })
422 442
423- t.Run("a stale keep snapshot cannot prune a newly committed generation", func(t *testing.T) {443+ t.Run("a stale keep decision cannot prune a newly committed generation", func(t *testing.T) {
424 t.Parallel()444 t.Parallel()
425 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}445 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}
426 record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}446 record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}
@@ -456,29 +476,39 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(
456 476
457 require.NoError(t, <-collected)477 require.NoError(t, <-collected)
458 require.NoError(t, <-published)478 require.NoError(t, <-published)
459- events, publishedBeforeGC := repository.eventLog()479+ require.Equal(t, []string{"collect", "publish"}, repository.eventLog())
460- require.Equal(t, []string{"collect", "publish"}, events)480+ require.True(t, repository.hasGeneration(record.IncarnationID, 2), "the fenced step must finish before generation two is published")
461- require.False(t, publishedBeforeGC)
462- require.True(t, repository.hasGeneration(record.IncarnationID, 2), "GC's stale keep snapshot must finish before generation two is published")
463 })481 })
464 }482 }
465 483
466-// cooperativeGarbageCollectionRepository blocks a GC pass until its context is484+// betweenStepsContext pauses a GC pass at its first cancellation check after
467-// cancelled, like the filesystem repository between entries.485+// one incarnation step has completed, outside any fenced step.
468-type cooperativeGarbageCollectionRepository struct {486+type betweenStepsContext struct {
469- garbageCollectionFenceRepository487+ context.Context
488+ repository *garbageCollectionFenceRepository
489+ once sync.Once
490+ reached chan struct{}
491+ release chan struct{}
470 }492 }
471 493
472-func (r *cooperativeGarbageCollectionRepository) CollectGarbage(ctx context.Context, _ map[domain.IncarnationID]domain.CheckpointRef) error {494+func (c *betweenStepsContext) Err() error {
473- close(r.collectionEntered)495+ if len(c.repository.eventLog()) == 1 {
474- <-ctx.Done()496+ c.once.Do(func() {
475- return ctx.Err()497+ close(c.reached)
498+ <-c.release
499+ })
500+ }
501+ return c.Context.Err()
476 }502 }
477 503
478-func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {504+// TestDurableMutationWaitsForAtMostOneIncarnation proves the fence is released
505+// between incarnation steps, so every mutation entry point can run while a
506+// pass is still in progress instead of waiting for the whole pass.
507+func TestDurableMutationWaitsForAtMostOneIncarnation(t *testing.T) {
479 t.Parallel()508 t.Parallel()
480 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}509 committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}}
481- record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}510+ work := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}
511+ broken := degradedTransactionRecord()
482 tests := []struct {512 tests := []struct {
483 name string513 name string
484 mutate func(*Coordinator) error514 mutate func(*Coordinator) error
@@ -487,9 +517,26 @@ func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {
487 _, err := c.Create(context.Background(), domain.CatalogueRecord{Name: "new"})517 _, err := c.Create(context.Background(), domain.CatalogueRecord{Name: "new"})
488 return err518 return err
489 }},519 }},
520+ {name: "rename", mutate: func(c *Coordinator) error {
521+ _, err := c.Rename(context.Background(), work.Name, "renamed")
522+ return err
523+ }},
524+ {name: "delete", mutate: func(c *Coordinator) error { return c.Delete(context.Background(), work.Name) }},
525+ {name: "exact delete", mutate: func(c *Coordinator) error {
526+ return c.DeleteExact(context.Background(), work.Name, work.IncarnationID, work.CreatedAt)
527+ }},
528+ {name: "mark broken", mutate: func(c *Coordinator) error {
529+ _, _, err := c.MarkBroken(context.Background(), work.Name, work.IncarnationID, "restore failed")
530+ return err
531+ }},
532+ {name: "discard", mutate: func(c *Coordinator) error { return c.Discard(context.Background(), broken.Name) }},
533+ {name: "reset incompatible", mutate: func(c *Coordinator) error {
534+ _, _, err := c.ResetIncompatible(context.Background(), work.Name, work.IncarnationID, committed)
535+ return err
536+ }},
490 {name: "checkpoint publication", mutate: func(c *Coordinator) error {537 {name: "checkpoint publication", mutate: func(c *Coordinator) error {
491- _, err := c.PublishCheckpoint(context.Background(), "work", ports.SnapshotPublication{538+ _, err := c.PublishCheckpoint(context.Background(), work.Name, ports.SnapshotPublication{
492- Name: "work", IncarnationID: record.IncarnationID, Generation: 2,539+ Name: work.Name, IncarnationID: work.IncarnationID, Generation: 2,
493 ParentCheckpoint: &committed, Manifest: []byte("generation two"),540 ParentCheckpoint: &committed, Manifest: []byte("generation two"),
494 })541 })
495 return err542 return err
@@ -498,25 +545,36 @@ func TestDurableMutationPreemptsRunningGarbageCollection(t *testing.T) {
498 for _, tt := range tests {545 for _, tt := range tests {
499 t.Run(tt.name, func(t *testing.T) {546 t.Run(tt.name, func(t *testing.T) {
500 t.Parallel()547 t.Parallel()
501- repository := &cooperativeGarbageCollectionRepository{garbageCollectionFenceRepository{collectionEntered: make(chan struct{})}}548+ // Two orphan incarnations give the pass two fenced steps.
502- coordinator := NewCoordinator(newTransactionCatalogue(record), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16)))549+ repository := &garbageCollectionFenceRepository{
550+ blocked: true,
551+ incarnations: map[domain.IncarnationID]struct{}{{7}: {}, {8}: {}},
552+ }
553+ coordinator := NewCoordinator(newTransactionCatalogue(work, broken), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 32)))
554+ ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})}
503 555
504 collected := make(chan error, 1)556 collected := make(chan error, 1)
505 go func() {557 go func() {
506- _, err := coordinator.CollectGarbage(context.Background())558+ _, err := coordinator.CollectGarbage(ctx)
507 collected <- err559 collected <- err
508 }()560 }()
509- <-repository.collectionEntered561+ <-ctx.reached
510-562+
511- mutated := make(chan error, 1)563+ // TryLock first so a regression that holds the fence across the pass
512- go func() { mutated <- tt.mutate(coordinator) }()564+ // fails here instead of deadlocking the mutation.
513- select {565+ require.True(t, coordinator.mutationMu.TryLock(), "the fence must be free between incarnation steps")
514- case err := <-mutated:566+ coordinator.mutationMu.Unlock()
515- require.NoError(t, err)567+ require.NoError(t, tt.mutate(coordinator))
516- case <-time.After(5 * time.Second):568+
517- t.Fatal("mutation waited for the GC pass instead of preempting it")569+ close(ctx.release)
570+ require.NoError(t, <-collected)
571+ steps := 0
572+ for _, event := range repository.eventLog() {
573+ if event == "collect" {
574+ steps++
575+ }
518 }576 }
519- require.ErrorIs(t, <-collected, ErrGarbageCollectionPreempted)577+ require.Equal(t, 2, steps, "both incarnation steps ran")
520 })578 })
521 }579 }
522 }580 }
@@ -656,7 +714,7 @@ func TestDiscardIsRetryIdempotent(t *testing.T) {
656 if tt.failBefore {714 if tt.failBefore {
657 require.Contains(t, repository.deletedIDs, old.IncarnationID)715 require.Contains(t, repository.deletedIDs, old.IncarnationID)
658 } else {716 } else {
659- require.Empty(t, repository.deletedIDs, "startup garbage collection owns the orphaned old directory")717+ require.Empty(t, repository.deletedIDs, "background garbage collection owns the orphaned old directory")
660 }718 }
661 })719 })
662 }720 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close