perf(daemon): publish the socket before snapshot GC and fence GC per incarnation (#1)
## Problem After a reboot, sessions took several seconds to appear. Snapshot GC ran synchronously before the daemon published its socket. On a backlog of about 21k files, this meant one `os.Root` per file and one fsync per shard, which added about 8 s. ## Changes - **Background GC**: the daemon publishes its socket first. GC starts after restoration finishes, then repeats every 30 minutes, and is cancelled at shutdown before the final checkpoints. - **Per-incarnation fencing**: `Coordinator.CollectGarbage` lists the incarnations (`SnapshotIncarnations`). For each one it takes `mutationMu`, re-reads the catalogue, then calls `CollectIncarnationGarbage(id, keep)`. A mutation waits for at most one step, never for the whole pass. - **HEAD reconciled before deleting generations**: any HEAD that differs from the commit is rewritten from the validated commit. This covers a missing, malformed or forward HEAD, one pointing to an older generation, and a digest mismatch. Without a commit, HEAD is deleted. - **Object sweep**: a single root, and no fsync per shard. - **Restore allocations**: - `readSizedFile` is presized. - `ReconcileCheckpoint` returns early when HEAD is already correct. - `PreflightObject` aliases `generationObject`. ## Measurements (isolated sandbox, pprof) | Scenario | Before | After | |---|---|---| | Socket ready, dirty repo (21k garbage files) | 5.7 s | 0.017 s | | Socket ready, clean repo | 0.33 s | 0.01 s | | Startup allocations | 1.84 GB | 0.46 GB | | Background GC pass (22.8k → 1.1k files) | n/a | about 1 s | ## Review There were 3 go-reviewer reviews plus a targeted re-review. Every finding was fixed, or deferred for the reasons below. **Deferred:** - The orphan `RemoveAll` is not cancellable and runs inside the fence. Its cost depends on the size of the orphan, and a large orphan backlog is rare. - The duplicate sha256 in `generationObject` (about 5% CPU). Removing it would move a trust boundary into the port contract. - `UnmarshalObject` is now used only by the tests, but it stays part of the codec's shape. ## Tests - `make lint`, `make test` (race). - Mutation tests: - a fence held across the whole pass makes all 8 mutation rows fail; - a catalogue read once per pass fails the keep test; - the old HEAD logic fails the new HEAD rows. - `maintenance_test.go` uses `testing/synctest`. ## Out of scope The restored scrollback still uses about 540 MB of live heap, uncompressed in vev-vt. That needs a separate branch. Reviewed-on: https://git.bnema.dev/bnema/vev/pulls/1
- commit
- 232a7db246449239e8a744252732c5b45107fd78
- author
- brice · · 2026-10-02 09:53 UTC
- committer
- bnema · · 2026-10-02 09:53 UTC
- parent
- 74436e9
- signature
- unsigned
27 files · +769 -298
M internal/adapters/snapshot/atomic.go +14 -1
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package snapshot | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "io" | |
| @@ -342,7 +343,7 @@ func readBoundedFile(f *os.File) ([]byte, error) { | |||
| 342 | 343 | _ = f.Close() | |
| 343 | 344 | return nil, fmt.Errorf("snapshot file too large") | |
| 344 | 345 | } | |
| 345 | - | data, readErr := io.ReadAll(io.LimitReader(f, int64(maxRepositoryRead)+1)) | |
| 346 | + | data, readErr := readSizedFile(f, int(stat.Size), maxRepositoryRead) | |
| 346 | 347 | closeErr := f.Close() | |
| 347 | 348 | if readErr != nil { | |
| 348 | 349 | return nil, readErr | |
| @@ -355,3 +356,15 @@ func readBoundedFile(f *os.File) ([]byte, error) { | |||
| 355 | 356 | } | |
| 356 | 357 | return data, nil | |
| 357 | 358 | } | |
| 359 | + | ||
| 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 | |
| 368 | + | } | |
| 369 | + | return buf.Bytes(), nil | |
| 370 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package snapshot | 1 | 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 := io.ReadAll(io.LimitReader(f, int64(maxRepositoryRead)+1)) | 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, readErr | 349 | return nil, readErr | ||
| @@ -355,3 +356,15 @@ func readBoundedFile(f *os.File) ([]byte, error) { | |||||
| 355 | } | 356 | } | ||
| 356 | return data, nil | 357 | return data, nil | ||
| 357 | } | 358 | } | ||
| 359 | + | ||||
| 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 | |||
| 368 | + | } | |||
| 369 | + | return buf.Bytes(), nil | |||
| 370 | + | } | |||
M internal/adapters/snapshot/direct_load_test.go +40 -0
| @@ -186,6 +186,46 @@ func TestReconcileCheckpoint(t *testing.T) { | |||
| 186 | 186 | } | |
| 187 | 187 | } | |
| 188 | 188 | ||
| 189 | + | func TestReconcileCheckpointRepairsOnlyDivergentHead(t *testing.T) { | |
| 190 | + | tests := []struct { | |
| 191 | + | name string | |
| 192 | + | prepare func(t *testing.T, repo *Repository, id domain.IncarnationID) | |
| 193 | + | wantWrite bool | |
| 194 | + | }{ | |
| 195 | + | {name: "matching HEAD is left untouched", prepare: func(*testing.T, *Repository, domain.IncarnationID) {}}, | |
| 196 | + | {name: "missing HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |
| 197 | + | require.NoError(t, os.Remove(repo.headPath(id))) | |
| 198 | + | }}, | |
| 199 | + | {name: "corrupt HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |
| 200 | + | require.NoError(t, os.WriteFile(repo.headPath(id), []byte("garbage"), 0o600)) | |
| 201 | + | }}, | |
| 202 | + | {name: "stale HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |
| 203 | + | require.NoError(t, os.WriteFile(repo.headPath(id), marshalHead(1, [32]byte{9}), 0o600)) | |
| 204 | + | }}, | |
| 205 | + | } | |
| 206 | + | for _, tt := range tests { | |
| 207 | + | t.Run(tt.name, func(t *testing.T) { | |
| 208 | + | repo := NewRepository(privateDir(t)) | |
| 209 | + | id := domain.IncarnationID{1} | |
| 210 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |
| 211 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |
| 212 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(pub.Manifest)} | |
| 213 | + | tt.prepare(t, repo, id) | |
| 214 | + | before, _ := os.Stat(repo.headPath(id)) | |
| 215 | + | ||
| 216 | + | require.NoError(t, repo.ReconcileCheckpoint(t.Context(), id, ref)) | |
| 217 | + | ||
| 218 | + | generation, digest, err := repo.readHead(id) | |
| 219 | + | require.NoError(t, err) | |
| 220 | + | require.Equal(t, ref.Generation, generation) | |
| 221 | + | require.Equal(t, ports.SnapshotDigest(ref.ManifestDigest), digest) | |
| 222 | + | after, err := os.Stat(repo.headPath(id)) | |
| 223 | + | require.NoError(t, err) | |
| 224 | + | require.Equal(t, tt.wantWrite, before == nil || !os.SameFile(before, after), "HEAD rewrite") | |
| 225 | + | }) | |
| 226 | + | } | |
| 227 | + | } | |
| 228 | + | ||
| 189 | 229 | func TestForwardOrphanRetryValidatesCheckpointBeforeCatalogueAdvance(t *testing.T) { | |
| 190 | 230 | for _, tt := range []struct { | |
| 191 | 231 | name string | |
| @@ -186,6 +186,46 @@ func TestReconcileCheckpoint(t *testing.T) { | |||||
| 186 | } | 186 | } | ||
| 187 | } | 187 | } | ||
| 188 | 188 | ||||
| 189 | + | func TestReconcileCheckpointRepairsOnlyDivergentHead(t *testing.T) { | |||
| 190 | + | tests := []struct { | |||
| 191 | + | name string | |||
| 192 | + | prepare func(t *testing.T, repo *Repository, id domain.IncarnationID) | |||
| 193 | + | wantWrite bool | |||
| 194 | + | }{ | |||
| 195 | + | {name: "matching HEAD is left untouched", prepare: func(*testing.T, *Repository, domain.IncarnationID) {}}, | |||
| 196 | + | {name: "missing HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |||
| 197 | + | require.NoError(t, os.Remove(repo.headPath(id))) | |||
| 198 | + | }}, | |||
| 199 | + | {name: "corrupt HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |||
| 200 | + | require.NoError(t, os.WriteFile(repo.headPath(id), []byte("garbage"), 0o600)) | |||
| 201 | + | }}, | |||
| 202 | + | {name: "stale HEAD is repaired", wantWrite: true, prepare: func(t *testing.T, repo *Repository, id domain.IncarnationID) { | |||
| 203 | + | require.NoError(t, os.WriteFile(repo.headPath(id), marshalHead(1, [32]byte{9}), 0o600)) | |||
| 204 | + | }}, | |||
| 205 | + | } | |||
| 206 | + | for _, tt := range tests { | |||
| 207 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 208 | + | repo := NewRepository(privateDir(t)) | |||
| 209 | + | id := domain.IncarnationID{1} | |||
| 210 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |||
| 211 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |||
| 212 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(pub.Manifest)} | |||
| 213 | + | tt.prepare(t, repo, id) | |||
| 214 | + | before, _ := os.Stat(repo.headPath(id)) | |||
| 215 | + | ||||
| 216 | + | require.NoError(t, repo.ReconcileCheckpoint(t.Context(), id, ref)) | |||
| 217 | + | ||||
| 218 | + | generation, digest, err := repo.readHead(id) | |||
| 219 | + | require.NoError(t, err) | |||
| 220 | + | require.Equal(t, ref.Generation, generation) | |||
| 221 | + | require.Equal(t, ports.SnapshotDigest(ref.ManifestDigest), digest) | |||
| 222 | + | after, err := os.Stat(repo.headPath(id)) | |||
| 223 | + | require.NoError(t, err) | |||
| 224 | + | require.Equal(t, tt.wantWrite, before == nil || !os.SameFile(before, after), "HEAD rewrite") | |||
| 225 | + | }) | |||
| 226 | + | } | |||
| 227 | + | } | |||
| 228 | + | ||||
| 189 | func TestForwardOrphanRetryValidatesCheckpointBeforeCatalogueAdvance(t *testing.T) { | 229 | func TestForwardOrphanRetryValidatesCheckpointBeforeCatalogueAdvance(t *testing.T) { | ||
| 190 | for _, tt := range []struct { | 230 | for _, tt := range []struct { | ||
| 191 | name string | 231 | name string | ||
M internal/adapters/snapshot/load.go +6 -0
| @@ -78,6 +78,12 @@ func (r *Repository) ReconcileCheckpoint(ctx context.Context, id domain.Incarnat | |||
| 78 | 78 | } | |
| 79 | 79 | lock := r.lockSession(key) | |
| 80 | 80 | defer r.unlockSession(lock) | |
| 81 | + | // HEAD normally already names the committed checkpoint. Skip the full | |
| 82 | + | // reload and the durable rewrite then; only a divergent or unreadable HEAD | |
| 83 | + | // needs repair, and repair still validates the checkpoint first. | |
| 84 | + | if generation, digest, err := r.readHead(id); err == nil && generation == ref.Generation && digest == ref.ManifestDigest { | |
| 85 | + | return nil | |
| 86 | + | } | |
| 81 | 87 | _, manifest, err := r.loadCheckpointLocked(ctx, id, "", ref) | |
| 82 | 88 | if err != nil { | |
| 83 | 89 | return fmt.Errorf("validate checkpoint before HEAD repair: %w", err) | |
| @@ -78,6 +78,12 @@ func (r *Repository) ReconcileCheckpoint(ctx context.Context, id domain.Incarnat | |||||
| 78 | } | 78 | } | ||
| 79 | lock := r.lockSession(key) | 79 | lock := r.lockSession(key) | ||
| 80 | defer r.unlockSession(lock) | 80 | defer r.unlockSession(lock) | ||
| 81 | + | // HEAD normally already names the committed checkpoint. Skip the full | |||
| 82 | + | // reload and the durable rewrite then; only a divergent or unreadable HEAD | |||
| 83 | + | // needs repair, and repair still validates the checkpoint first. | |||
| 84 | + | if generation, digest, err := r.readHead(id); err == nil && generation == ref.Generation && digest == ref.ManifestDigest { | |||
| 85 | + | return nil | |||
| 86 | + | } | |||
| 81 | _, manifest, err := r.loadCheckpointLocked(ctx, id, "", ref) | 87 | _, manifest, err := r.loadCheckpointLocked(ctx, id, "", ref) | ||
| 82 | if err != nil { | 88 | if err != nil { | ||
| 83 | return fmt.Errorf("validate checkpoint before HEAD repair: %w", err) | 89 | return fmt.Errorf("validate checkpoint before HEAD repair: %w", err) | ||
M internal/adapters/snapshot/maintenance_gc.go +89 -85
| @@ -13,77 +13,59 @@ import ( | |||
| 13 | 13 | codec "github.com/bnema/vev/internal/usecase/snapshot" | |
| 14 | 14 | ) | |
| 15 | 15 | ||
| 16 | - | // CollectGarbage applies the complete snapshot retention policy. The keep map | |
| 17 | - | // must come from a catalogue that loaded and validated successfully: only then | |
| 18 | - | // is an incarnation absent from keep known to be an orphan. | |
| 19 | - | 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) { | |
| 20 | 18 | if err := ctx.Err(); err != nil { | |
| 21 | - | return err | |
| 19 | + | return nil, err | |
| 22 | 20 | } | |
| 23 | - | sessions := filepath.Join(r.dir, repositorySessionsDir) | |
| 24 | - | entries, err := r.readGarbageCollectionDirectory(sessions) | |
| 21 | + | entries, err := r.readGarbageCollectionDirectory(filepath.Join(r.dir, repositorySessionsDir)) | |
| 25 | 22 | if errors.Is(err, os.ErrNotExist) { | |
| 26 | - | return nil | |
| 23 | + | return nil, nil | |
| 27 | 24 | } | |
| 28 | 25 | if err != nil { | |
| 29 | - | return fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err)) | |
| 26 | + | return nil, fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err)) | |
| 30 | 27 | } | |
| 31 | - | ||
| 32 | - | var collected error | |
| 28 | + | ids := make([]domain.IncarnationID, 0, len(entries)) | |
| 33 | 29 | for _, entry := range entries { | |
| 34 | - | if err := ctx.Err(); err != nil { | |
| 35 | - | return errors.Join(collected, err) | |
| 36 | - | } | |
| 37 | - | if !entry.IsDir() { | |
| 38 | - | continue | |
| 39 | - | } | |
| 40 | 30 | var id domain.IncarnationID | |
| 41 | - | if err := id.UnmarshalText([]byte(entry.Name())); err != nil { | |
| 42 | - | continue | |
| 43 | - | } | |
| 44 | - | committed, known := keep[id] | |
| 45 | - | if !known { | |
| 46 | - | key, keyErr := incarnationKey(id) | |
| 47 | - | if keyErr != nil { | |
| 48 | - | collected = errors.Join(collected, keyErr) | |
| 49 | - | continue | |
| 50 | - | } | |
| 51 | - | lock := r.lockSession(key) | |
| 52 | - | _, known = keep[id] | |
| 53 | - | if !known { | |
| 54 | - | err = r.removeIncarnationLocked(id) | |
| 55 | - | } | |
| 56 | - | r.unlockSession(lock) | |
| 57 | - | if err != nil { | |
| 58 | - | collected = errors.Join(collected, fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err))) | |
| 59 | - | continue | |
| 60 | - | } | |
| 61 | - | if !known { | |
| 62 | - | r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation") | |
| 63 | - | } | |
| 64 | - | continue | |
| 65 | - | } | |
| 66 | - | if err := r.pruneGenerations(ctx, id, committed); err != nil { | |
| 67 | - | collected = errors.Join(collected, err) | |
| 31 | + | if entry.IsDir() && id.UnmarshalText([]byte(entry.Name())) == nil { | |
| 32 | + | ids = append(ids, id) | |
| 68 | 33 | } | |
| 69 | 34 | } | |
| 70 | - | if err := r.syncDirectory(sessions); err != nil { | |
| 71 | - | collected = errors.Join(collected, fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err))) | |
| 72 | - | } | |
| 73 | - | return collected | |
| 35 | + | return ids, nil | |
| 74 | 36 | } | |
| 75 | 37 | ||
| 76 | - | // pruneGenerations keeps the committed generation and its immediate | |
| 77 | - | // predecessor. Everything else, including a forward orphan newer than the | |
| 78 | - | // catalogue commit, is removed before unreferenced objects are swept. | |
| 79 | - | 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 | + | } | |
| 80 | 45 | key, err := incarnationKey(id) | |
| 81 | 46 | if err != nil { | |
| 82 | 47 | return err | |
| 83 | 48 | } | |
| 84 | 49 | lock := r.lockSession(key) | |
| 85 | 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 | + | } | |
| 86 | 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 { | |
| 87 | 69 | generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations) | |
| 88 | 70 | entries, err := r.readGarbageCollectionDirectory(generationsDir) | |
| 89 | 71 | if errors.Is(err, os.ErrNotExist) { | |
| @@ -104,12 +86,16 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation | |||
| 104 | 86 | if err != nil { | |
| 105 | 87 | return fmt.Errorf("mark snapshot objects for %s: %w", id.String(), err) | |
| 106 | 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, committed); err != nil { | |
| 92 | + | return err | |
| 93 | + | } | |
| 107 | 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. | |
| 108 | 97 | removed := 0 | |
| 109 | 98 | for _, entry := range entries { | |
| 110 | - | if err := ctx.Err(); err != nil { | |
| 111 | - | return err | |
| 112 | - | } | |
| 113 | 99 | generation, canonical := parseGenerationFilename(entry.Name()) | |
| 114 | 100 | if !canonical || entry.IsDir() { | |
| 115 | 101 | continue | |
| @@ -128,30 +114,28 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation | |||
| 128 | 114 | } | |
| 129 | 115 | r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-generations", "removed", removed) | |
| 130 | 116 | } | |
| 131 | - | if err := r.removeHeadForDiscardedGenerations(id, kept); err != nil { | |
| 132 | - | return err | |
| 133 | - | } | |
| 134 | 117 | return r.sweepUnreferencedObjects(ctx, id, references) | |
| 135 | 118 | } | |
| 136 | 119 | ||
| 137 | - | // removeHeadForDiscardedGenerations removes a repository HEAD only when its | |
| 138 | - | // generation was removed by the retention policy. In particular, an | |
| 139 | - | // incarnation with no committed checkpoint retains no generations, so its | |
| 140 | - | // crash-published HEAD cannot make a retry conflict with an orphan. | |
| 141 | - | func (r *Repository) removeHeadForDiscardedGenerations(id domain.IncarnationID, kept map[uint64]struct{}) error { | |
| 142 | - | removeHead := len(kept) == 0 | |
| 143 | - | if !removeHead { | |
| 144 | - | generation, _, err := r.readHead(id) | |
| 145 | - | if errors.Is(err, os.ErrNotExist) { | |
| 120 | + | // reconcileHeadForRetention makes HEAD name the committed checkpoint. GC runs | |
| 121 | + | // under the mutation fence, so no publication is in flight and any other HEAD | |
| 122 | + | // (missing, malformed, a forward orphan, an older generation, or a digest | |
| 123 | + | // mismatch) is damage: a live session's next publication must find its | |
| 124 | + | // committed parent. Without a commit, no generation is retained and HEAD is | |
| 125 | + | // removed so a crash-published HEAD cannot make a retry conflict with an orphan. | |
| 126 | + | func (r *Repository) reconcileHeadForRetention(id domain.IncarnationID, committed domain.CheckpointRef) error { | |
| 127 | + | if committed.Generation > 0 { | |
| 128 | + | generation, digest, err := r.readHead(id) | |
| 129 | + | if err != nil && !errors.Is(err, os.ErrNotExist) && !errors.Is(err, ErrInvalidHEAD) { | |
| 130 | + | return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | |
| 131 | + | } | |
| 132 | + | if err == nil && generation == committed.Generation && digest == committed.ManifestDigest { | |
| 146 | 133 | return nil | |
| 147 | 134 | } | |
| 148 | - | if err != nil { | |
| 149 | - | return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | |
| 135 | + | // referencesForGenerations already validated the committed manifest digest. | |
| 136 | + | if err := r.writeMutable(r.headPath(id), marshalHead(committed.Generation, committed.ManifestDigest)); err != nil { | |
| 137 | + | return fmt.Errorf("rewind snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | |
| 150 | 138 | } | |
| 151 | - | _, retained := kept[generation] | |
| 152 | - | removeHead = !retained | |
| 153 | - | } | |
| 154 | - | if !removeHead { | |
| 155 | 139 | return nil | |
| 156 | 140 | } | |
| 157 | 141 | if err := r.remove(r.headPath(id)); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| @@ -229,6 +213,21 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D | |||
| 229 | 213 | if err != nil { | |
| 230 | 214 | return nil, err | |
| 231 | 215 | } | |
| 216 | + | return readAndCloseDirectory(directory) | |
| 217 | + | } | |
| 218 | + | ||
| 219 | + | // readRootDirectory lists rel through an already pinned repository root, so a | |
| 220 | + | // sweep over thousands of objects does not reopen and revalidate the root for | |
| 221 | + | // every shard. | |
| 222 | + | func readRootDirectory(root *os.Root, rel string) ([]os.DirEntry, error) { | |
| 223 | + | directory, err := openRootDirectory(root, rel) | |
| 224 | + | if err != nil { | |
| 225 | + | return nil, err | |
| 226 | + | } | |
| 227 | + | return readAndCloseDirectory(directory) | |
| 228 | + | } | |
| 229 | + | ||
| 230 | + | func readAndCloseDirectory(directory *os.File) (entries []os.DirEntry, err error) { | |
| 232 | 231 | defer func() { | |
| 233 | 232 | if closeErr := directory.Close(); closeErr != nil { | |
| 234 | 233 | err = errors.Join(err, closeErr) | |
| @@ -237,9 +236,17 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D | |||
| 237 | 236 | return directory.ReadDir(-1) | |
| 238 | 237 | } | |
| 239 | 238 | ||
| 240 | - | func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.IncarnationID, references map[ports.SnapshotDigest]struct{}) error { | |
| 241 | - | objectsRoot := filepath.Join(r.sessionPath(id), repositoryObjectsDir) | |
| 242 | - | shards, err := r.readGarbageCollectionDirectory(objectsRoot) | |
| 239 | + | func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.IncarnationID, references map[ports.SnapshotDigest]struct{}) (err error) { | |
| 240 | + | objectsRoot, ok := r.repositoryRelative(filepath.Join(r.sessionPath(id), repositoryObjectsDir)) | |
| 241 | + | if !ok { | |
| 242 | + | return fmt.Errorf("snapshot path outside repository") | |
| 243 | + | } | |
| 244 | + | root, err := r.openRoot() | |
| 245 | + | if err != nil { | |
| 246 | + | return fmt.Errorf("open snapshot root for %s: %w", id.String(), safeFilesystemError(err)) | |
| 247 | + | } | |
| 248 | + | defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }() | |
| 249 | + | shards, err := readRootDirectory(root, objectsRoot) | |
| 243 | 250 | if errors.Is(err, os.ErrNotExist) { | |
| 244 | 251 | return nil | |
| 245 | 252 | } | |
| @@ -255,11 +262,14 @@ func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.Inc | |||
| 255 | 262 | continue | |
| 256 | 263 | } | |
| 257 | 264 | dir := filepath.Join(objectsRoot, shard.Name()) | |
| 258 | - | objects, err := r.readGarbageCollectionDirectory(dir) | |
| 265 | + | objects, err := readRootDirectory(root, dir) | |
| 259 | 266 | if err != nil { | |
| 260 | 267 | return fmt.Errorf("read snapshot object shard for %s: %w", id.String(), safeFilesystemError(err)) | |
| 261 | 268 | } | |
| 262 | - | shardRemoved := false | |
| 269 | + | // Object unlinks are deliberately not fsynced per shard. Objects are | |
| 270 | + | // immutable and content-addressed: an unlink lost to a crash only brings | |
| 271 | + | // back an unreferenced object, which the next pass collects again, and a | |
| 272 | + | // later publication revalidates any existing object before reuse. | |
| 263 | 273 | for _, object := range objects { | |
| 264 | 274 | digest, canonical := parseObjectDigest(object.Name()) | |
| 265 | 275 | if !canonical || object.IsDir() { | |
| @@ -268,16 +278,10 @@ func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.Inc | |||
| 268 | 278 | if _, retained := references[digest]; retained { | |
| 269 | 279 | continue | |
| 270 | 280 | } | |
| 271 | - | if err := r.remove(filepath.Join(dir, object.Name())); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| 281 | + | if err := root.Remove(filepath.Join(dir, object.Name())); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| 272 | 282 | return fmt.Errorf("remove snapshot object for %s: %w", id.String(), safeFilesystemError(err)) | |
| 273 | 283 | } | |
| 274 | 284 | removed++ | |
| 275 | - | shardRemoved = true | |
| 276 | - | } | |
| 277 | - | if shardRemoved { | |
| 278 | - | if err := r.syncDirectory(dir); err != nil { | |
| 279 | - | return fmt.Errorf("sync snapshot object shard for %s: %w", id.String(), safeFilesystemError(err)) | |
| 280 | - | } | |
| 281 | 285 | } | |
| 282 | 286 | } | |
| 283 | 287 | if removed > 0 { | |
| @@ -13,77 +13,59 @@ import ( | |||||
| 13 | codec "github.com/bnema/vev/internal/usecase/snapshot" | 13 | codec "github.com/bnema/vev/internal/usecase/snapshot" | ||
| 14 | ) | 14 | ) | ||
| 15 | 15 | ||||
| 16 | - | // CollectGarbage applies the complete snapshot retention policy. The keep map | 16 | + | // SnapshotIncarnations lists the canonical incarnation directories. |
| 17 | - | // must come from a catalogue that loaded and validated successfully: only then | 17 | + | func (r *Repository) SnapshotIncarnations(ctx context.Context) ([]domain.IncarnationID, error) { |
| 18 | - | // is an incarnation absent from keep known to be an orphan. | |||
| 19 | - | func (r *Repository) CollectGarbage(ctx context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error { | |||
| 20 | if err := ctx.Err(); err != nil { | 18 | if err := ctx.Err(); err != nil { | ||
| 21 | - | return err | 19 | + | return nil, err |
| 22 | } | 20 | } | ||
| 23 | - | sessions := filepath.Join(r.dir, repositorySessionsDir) | 21 | + | entries, err := r.readGarbageCollectionDirectory(filepath.Join(r.dir, repositorySessionsDir)) |
| 24 | - | entries, err := r.readGarbageCollectionDirectory(sessions) | |||
| 25 | if errors.Is(err, os.ErrNotExist) { | 22 | if errors.Is(err, os.ErrNotExist) { | ||
| 26 | - | return nil | 23 | + | return nil, nil |
| 27 | } | 24 | } | ||
| 28 | if err != nil { | 25 | if err != nil { | ||
| 29 | - | return fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err)) | 26 | + | return nil, fmt.Errorf("read snapshot incarnations: %w", safeFilesystemError(err)) |
| 30 | } | 27 | } | ||
| 31 | - | 28 | + | ids := make([]domain.IncarnationID, 0, len(entries)) | |
| 32 | - | var collected error | |||
| 33 | for _, entry := range entries { | 29 | for _, entry := range entries { | ||
| 34 | - | if err := ctx.Err(); err != nil { | |||
| 35 | - | return errors.Join(collected, err) | |||
| 36 | - | } | |||
| 37 | - | if !entry.IsDir() { | |||
| 38 | - | continue | |||
| 39 | - | } | |||
| 40 | var id domain.IncarnationID | 30 | var id domain.IncarnationID | ||
| 41 | - | if err := id.UnmarshalText([]byte(entry.Name())); err != nil { | 31 | + | if entry.IsDir() && id.UnmarshalText([]byte(entry.Name())) == nil { |
| 42 | - | continue | 32 | + | ids = append(ids, id) |
| 43 | - | } | |||
| 44 | - | committed, known := keep[id] | |||
| 45 | - | if !known { | |||
| 46 | - | key, keyErr := incarnationKey(id) | |||
| 47 | - | if keyErr != nil { | |||
| 48 | - | collected = errors.Join(collected, keyErr) | |||
| 49 | - | continue | |||
| 50 | - | } | |||
| 51 | - | lock := r.lockSession(key) | |||
| 52 | - | _, known = keep[id] | |||
| 53 | - | if !known { | |||
| 54 | - | err = r.removeIncarnationLocked(id) | |||
| 55 | - | } | |||
| 56 | - | r.unlockSession(lock) | |||
| 57 | - | if err != nil { | |||
| 58 | - | collected = errors.Join(collected, fmt.Errorf("remove orphan snapshot incarnation %s: %w", id.String(), safeFilesystemError(err))) | |||
| 59 | - | continue | |||
| 60 | - | } | |||
| 61 | - | if !known { | |||
| 62 | - | r.log.Info("snapshot_garbage_collected", "incarnation", id.String(), "action", "remove-incarnation") | |||
| 63 | - | } | |||
| 64 | - | continue | |||
| 65 | - | } | |||
| 66 | - | if err := r.pruneGenerations(ctx, id, committed); err != nil { | |||
| 67 | - | collected = errors.Join(collected, err) | |||
| 68 | } | 33 | } | ||
| 69 | } | 34 | } | ||
| 70 | - | if err := r.syncDirectory(sessions); err != nil { | 35 | + | return ids, nil |
| 71 | - | collected = errors.Join(collected, fmt.Errorf("sync snapshot sessions directory: %w", safeFilesystemError(err))) | |||
| 72 | - | } | |||
| 73 | - | return collected | |||
| 74 | } | 36 | } | ||
| 75 | 37 | ||||
| 76 | - | // pruneGenerations keeps the committed generation and its immediate | 38 | + | // CollectIncarnationGarbage applies the retention policy to one incarnation. |
| 77 | - | // predecessor. Everything else, including a forward orphan newer than the | 39 | + | // keep must come from a catalogue that loaded and validated successfully: only |
| 78 | - | // catalogue commit, is removed before unreferenced objects are swept. | 40 | + | // then is a nil keep known to mark an orphan. |
| 79 | - | 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 | + | } | |||
| 80 | key, err := incarnationKey(id) | 45 | key, err := incarnationKey(id) | ||
| 81 | if err != nil { | 46 | if err != nil { | ||
| 82 | return err | 47 | return err | ||
| 83 | } | 48 | } | ||
| 84 | lock := r.lockSession(key) | 49 | lock := r.lockSession(key) | ||
| 85 | 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 | + | } | |||
| 86 | 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 { | |||
| 87 | generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations) | 69 | generationsDir := filepath.Join(r.sessionPath(id), repositoryGenerations) | ||
| 88 | entries, err := r.readGarbageCollectionDirectory(generationsDir) | 70 | entries, err := r.readGarbageCollectionDirectory(generationsDir) | ||
| 89 | if errors.Is(err, os.ErrNotExist) { | 71 | if errors.Is(err, os.ErrNotExist) { | ||
| @@ -104,12 +86,16 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation | |||||
| 104 | if err != nil { | 86 | if err != nil { | ||
| 105 | 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) | ||
| 106 | } | 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, committed); err != nil { | |||
| 92 | + | return err | |||
| 93 | + | } | |||
| 107 | 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. | |||
| 108 | removed := 0 | 97 | removed := 0 | ||
| 109 | for _, entry := range entries { | 98 | for _, entry := range entries { | ||
| 110 | - | if err := ctx.Err(); err != nil { | |||
| 111 | - | return err | |||
| 112 | - | } | |||
| 113 | generation, canonical := parseGenerationFilename(entry.Name()) | 99 | generation, canonical := parseGenerationFilename(entry.Name()) | ||
| 114 | if !canonical || entry.IsDir() { | 100 | if !canonical || entry.IsDir() { | ||
| 115 | continue | 101 | continue | ||
| @@ -128,30 +114,28 @@ func (r *Repository) pruneGenerations(ctx context.Context, id domain.Incarnation | |||||
| 128 | } | 114 | } | ||
| 129 | 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) | ||
| 130 | } | 116 | } | ||
| 131 | - | if err := r.removeHeadForDiscardedGenerations(id, kept); err != nil { | |||
| 132 | - | return err | |||
| 133 | - | } | |||
| 134 | return r.sweepUnreferencedObjects(ctx, id, references) | 117 | return r.sweepUnreferencedObjects(ctx, id, references) | ||
| 135 | } | 118 | } | ||
| 136 | 119 | ||||
| 137 | - | // removeHeadForDiscardedGenerations removes a repository HEAD only when its | 120 | + | // reconcileHeadForRetention makes HEAD name the committed checkpoint. GC runs |
| 138 | - | // generation was removed by the retention policy. In particular, an | 121 | + | // under the mutation fence, so no publication is in flight and any other HEAD |
| 139 | - | // incarnation with no committed checkpoint retains no generations, so its | 122 | + | // (missing, malformed, a forward orphan, an older generation, or a digest |
| 140 | - | // crash-published HEAD cannot make a retry conflict with an orphan. | 123 | + | // mismatch) is damage: a live session's next publication must find its |
| 141 | - | func (r *Repository) removeHeadForDiscardedGenerations(id domain.IncarnationID, kept map[uint64]struct{}) error { | 124 | + | // committed parent. Without a commit, no generation is retained and HEAD is |
| 142 | - | removeHead := len(kept) == 0 | 125 | + | // removed so a crash-published HEAD cannot make a retry conflict with an orphan. |
| 143 | - | if !removeHead { | 126 | + | func (r *Repository) reconcileHeadForRetention(id domain.IncarnationID, committed domain.CheckpointRef) error { |
| 144 | - | generation, _, err := r.readHead(id) | 127 | + | if committed.Generation > 0 { |
| 145 | - | if errors.Is(err, os.ErrNotExist) { | 128 | + | generation, digest, err := r.readHead(id) |
| 129 | + | if err != nil && !errors.Is(err, os.ErrNotExist) && !errors.Is(err, ErrInvalidHEAD) { | |||
| 130 | + | return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | |||
| 131 | + | } | |||
| 132 | + | if err == nil && generation == committed.Generation && digest == committed.ManifestDigest { | |||
| 146 | return nil | 133 | return nil | ||
| 147 | } | 134 | } | ||
| 148 | - | if err != nil { | 135 | + | // referencesForGenerations already validated the committed manifest digest. |
| 149 | - | return fmt.Errorf("read snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | 136 | + | if err := r.writeMutable(r.headPath(id), marshalHead(committed.Generation, committed.ManifestDigest)); err != nil { |
| 137 | + | return fmt.Errorf("rewind snapshot HEAD for %s: %w", id.String(), safeFilesystemError(err)) | |||
| 150 | } | 138 | } | ||
| 151 | - | _, retained := kept[generation] | |||
| 152 | - | removeHead = !retained | |||
| 153 | - | } | |||
| 154 | - | if !removeHead { | |||
| 155 | return nil | 139 | return nil | ||
| 156 | } | 140 | } | ||
| 157 | if err := r.remove(r.headPath(id)); err != nil && !errors.Is(err, os.ErrNotExist) { | 141 | if err := r.remove(r.headPath(id)); err != nil && !errors.Is(err, os.ErrNotExist) { | ||
| @@ -229,6 +213,21 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D | |||||
| 229 | if err != nil { | 213 | if err != nil { | ||
| 230 | return nil, err | 214 | return nil, err | ||
| 231 | } | 215 | } | ||
| 216 | + | return readAndCloseDirectory(directory) | |||
| 217 | + | } | |||
| 218 | + | ||||
| 219 | + | // readRootDirectory lists rel through an already pinned repository root, so a | |||
| 220 | + | // sweep over thousands of objects does not reopen and revalidate the root for | |||
| 221 | + | // every shard. | |||
| 222 | + | func readRootDirectory(root *os.Root, rel string) ([]os.DirEntry, error) { | |||
| 223 | + | directory, err := openRootDirectory(root, rel) | |||
| 224 | + | if err != nil { | |||
| 225 | + | return nil, err | |||
| 226 | + | } | |||
| 227 | + | return readAndCloseDirectory(directory) | |||
| 228 | + | } | |||
| 229 | + | ||||
| 230 | + | func readAndCloseDirectory(directory *os.File) (entries []os.DirEntry, err error) { | |||
| 232 | defer func() { | 231 | defer func() { | ||
| 233 | if closeErr := directory.Close(); closeErr != nil { | 232 | if closeErr := directory.Close(); closeErr != nil { | ||
| 234 | err = errors.Join(err, closeErr) | 233 | err = errors.Join(err, closeErr) | ||
| @@ -237,9 +236,17 @@ func (r *Repository) readGarbageCollectionDirectory(path string) (entries []os.D | |||||
| 237 | return directory.ReadDir(-1) | 236 | return directory.ReadDir(-1) | ||
| 238 | } | 237 | } | ||
| 239 | 238 | ||||
| 240 | - | func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.IncarnationID, references map[ports.SnapshotDigest]struct{}) error { | 239 | + | func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.IncarnationID, references map[ports.SnapshotDigest]struct{}) (err error) { |
| 241 | - | objectsRoot := filepath.Join(r.sessionPath(id), repositoryObjectsDir) | 240 | + | objectsRoot, ok := r.repositoryRelative(filepath.Join(r.sessionPath(id), repositoryObjectsDir)) |
| 242 | - | shards, err := r.readGarbageCollectionDirectory(objectsRoot) | 241 | + | if !ok { |
| 242 | + | return fmt.Errorf("snapshot path outside repository") | |||
| 243 | + | } | |||
| 244 | + | root, err := r.openRoot() | |||
| 245 | + | if err != nil { | |||
| 246 | + | return fmt.Errorf("open snapshot root for %s: %w", id.String(), safeFilesystemError(err)) | |||
| 247 | + | } | |||
| 248 | + | defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }() | |||
| 249 | + | shards, err := readRootDirectory(root, objectsRoot) | |||
| 243 | if errors.Is(err, os.ErrNotExist) { | 250 | if errors.Is(err, os.ErrNotExist) { | ||
| 244 | return nil | 251 | return nil | ||
| 245 | } | 252 | } | ||
| @@ -255,11 +262,14 @@ func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.Inc | |||||
| 255 | continue | 262 | continue | ||
| 256 | } | 263 | } | ||
| 257 | dir := filepath.Join(objectsRoot, shard.Name()) | 264 | dir := filepath.Join(objectsRoot, shard.Name()) | ||
| 258 | - | objects, err := r.readGarbageCollectionDirectory(dir) | 265 | + | objects, err := readRootDirectory(root, dir) |
| 259 | if err != nil { | 266 | if err != nil { | ||
| 260 | return fmt.Errorf("read snapshot object shard for %s: %w", id.String(), safeFilesystemError(err)) | 267 | return fmt.Errorf("read snapshot object shard for %s: %w", id.String(), safeFilesystemError(err)) | ||
| 261 | } | 268 | } | ||
| 262 | - | shardRemoved := false | 269 | + | // Object unlinks are deliberately not fsynced per shard. Objects are |
| 270 | + | // immutable and content-addressed: an unlink lost to a crash only brings | |||
| 271 | + | // back an unreferenced object, which the next pass collects again, and a | |||
| 272 | + | // later publication revalidates any existing object before reuse. | |||
| 263 | for _, object := range objects { | 273 | for _, object := range objects { | ||
| 264 | digest, canonical := parseObjectDigest(object.Name()) | 274 | digest, canonical := parseObjectDigest(object.Name()) | ||
| 265 | if !canonical || object.IsDir() { | 275 | if !canonical || object.IsDir() { | ||
| @@ -268,16 +278,10 @@ func (r *Repository) sweepUnreferencedObjects(ctx context.Context, id domain.Inc | |||||
| 268 | if _, retained := references[digest]; retained { | 278 | if _, retained := references[digest]; retained { | ||
| 269 | continue | 279 | continue | ||
| 270 | } | 280 | } | ||
| 271 | - | if err := r.remove(filepath.Join(dir, object.Name())); err != nil && !errors.Is(err, os.ErrNotExist) { | 281 | + | if err := root.Remove(filepath.Join(dir, object.Name())); err != nil && !errors.Is(err, os.ErrNotExist) { |
| 272 | return fmt.Errorf("remove snapshot object for %s: %w", id.String(), safeFilesystemError(err)) | 282 | return fmt.Errorf("remove snapshot object for %s: %w", id.String(), safeFilesystemError(err)) | ||
| 273 | } | 283 | } | ||
| 274 | removed++ | 284 | removed++ | ||
| 275 | - | shardRemoved = true | |||
| 276 | - | } | |||
| 277 | - | if shardRemoved { | |||
| 278 | - | if err := r.syncDirectory(dir); err != nil { | |||
| 279 | - | return fmt.Errorf("sync snapshot object shard for %s: %w", id.String(), safeFilesystemError(err)) | |||
| 280 | - | } | |||
| 281 | } | 285 | } | ||
| 282 | } | 286 | } | ||
| 283 | if removed > 0 { | 287 | if removed > 0 { | ||
M internal/adapters/snapshot/maintenance_gc_test.go +56 -5
| @@ -20,9 +20,9 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) { | |||
| 20 | 20 | require.NoError(t, repository.Publish(context.Background(), publication)) | |
| 21 | 21 | ||
| 22 | 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 | 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 | 26 | _, err := os.Stat(repository.headPath(publication.IncarnationID)) | |
| 27 | 27 | require.ErrorIs(t, err, os.ErrNotExist) | |
| 28 | 28 | require.Empty(t, garbageCollectionGenerations(t, repository, publication.IncarnationID)) | |
| @@ -32,11 +32,62 @@ func TestCollectGarbageClearsUncommittedCheckpointHead(t *testing.T) { | |||
| 32 | 32 | require.NoError(t, repository.Publish(context.Background(), publication)) | |
| 33 | 33 | committed := domain.CheckpointRef{Generation: publication.Generation, ManifestDigest: sha256.Sum256(publication.Manifest)} | |
| 34 | 34 | keep[publication.IncarnationID] = committed | |
| 35 | - | require.NoError(t, repository.CollectGarbage(context.Background(), keep)) | |
| 35 | + | require.NoError(t, collectAll(context.Background(), repository, keep)) | |
| 36 | 36 | _, err = repository.LoadCheckpoint(context.Background(), publication.IncarnationID, publication.Name, committed) | |
| 37 | 37 | require.NoError(t, err) | |
| 38 | 38 | } | |
| 39 | 39 | ||
| 40 | + | func TestCollectIncarnationGarbageKeepsHeadPublishable(t *testing.T) { | |
| 41 | + | t.Parallel() | |
| 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 | + | // Generation 2 is retained as the committed parent, but HEAD must still | |
| 57 | + | // name the commit or the next publication is rejected. | |
| 58 | + | {name: "HEAD naming a retained older generation is moved forward", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |
| 59 | + | manifest, err := os.ReadFile(repository.manifestPath(id, 2)) | |
| 60 | + | require.NoError(t, err) | |
| 61 | + | require.NoError(t, os.WriteFile(repository.headPath(id), marshalHead(2, sha256.Sum256(manifest)), 0o600)) | |
| 62 | + | }}, | |
| 63 | + | {name: "HEAD with a mismatched digest is rewritten", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |
| 64 | + | require.NoError(t, os.WriteFile(repository.headPath(id), marshalHead(3, ports.SnapshotDigest{1}), 0o600)) | |
| 65 | + | }}, | |
| 66 | + | {name: "malformed HEAD is restored", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |
| 67 | + | require.NoError(t, os.WriteFile(repository.headPath(id), []byte("corrupt"), 0o600)) | |
| 68 | + | }}, | |
| 69 | + | } | |
| 70 | + | for _, tt := range tests { | |
| 71 | + | t.Run(tt.name, func(t *testing.T) { | |
| 72 | + | t.Parallel() | |
| 73 | + | repository := NewRepository(privateDir(t)) | |
| 74 | + | publications := publishGarbageCollectionGenerations(t, repository, "work", 4) | |
| 75 | + | id := publications[0].IncarnationID | |
| 76 | + | committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)} | |
| 77 | + | tt.prepare(t, repository, id) | |
| 78 | + | ||
| 79 | + | require.NoError(t, repository.CollectIncarnationGarbage(t.Context(), id, &committed)) | |
| 80 | + | ||
| 81 | + | generation, digest, err := repository.readHead(id) | |
| 82 | + | require.NoError(t, err) | |
| 83 | + | require.Equal(t, committed.Generation, generation) | |
| 84 | + | require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest) | |
| 85 | + | // The live session retries generation 4 against its committed parent. | |
| 86 | + | require.NoError(t, repository.Publish(t.Context(), publications[3])) | |
| 87 | + | }) | |
| 88 | + | } | |
| 89 | + | } | |
| 90 | + | ||
| 40 | 91 | func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) { | |
| 41 | 92 | for _, tt := range []struct { | |
| 42 | 93 | name string | |
| @@ -74,7 +125,7 @@ func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) { | |||
| 74 | 125 | committed = tt.mutate(t, repository, id, committed) | |
| 75 | 126 | before := snapshotRepositoryFiles(t, repository.dir) | |
| 76 | 127 | ||
| 77 | - | require.Error(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed})) | |
| 128 | + | require.Error(t, collectAll(t.Context(), repository, map[domain.IncarnationID]domain.CheckpointRef{id: committed})) | |
| 78 | 129 | require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed") | |
| 79 | 130 | }) | |
| 80 | 131 | } | |
| @@ -103,7 +154,7 @@ func TestCollectGarbage(t *testing.T) { | |||
| 103 | 154 | keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)} | |
| 104 | 155 | } | |
| 105 | 156 | ||
| 106 | - | require.NoError(t, repository.CollectGarbage(context.Background(), keep)) | |
| 157 | + | require.NoError(t, collectAll(context.Background(), repository, keep)) | |
| 107 | 158 | if !tt.known { | |
| 108 | 159 | _, err := os.Stat(repository.sessionPath(id)) | |
| 109 | 160 | 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,11 +32,62 @@ 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] = committed | 34 | 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 TestCollectIncarnationGarbageKeepsHeadPublishable(t *testing.T) { | |||
| 41 | + | t.Parallel() | |||
| 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 | + | // Generation 2 is retained as the committed parent, but HEAD must still | |||
| 57 | + | // name the commit or the next publication is rejected. | |||
| 58 | + | {name: "HEAD naming a retained older generation is moved forward", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |||
| 59 | + | manifest, err := os.ReadFile(repository.manifestPath(id, 2)) | |||
| 60 | + | require.NoError(t, err) | |||
| 61 | + | require.NoError(t, os.WriteFile(repository.headPath(id), marshalHead(2, sha256.Sum256(manifest)), 0o600)) | |||
| 62 | + | }}, | |||
| 63 | + | {name: "HEAD with a mismatched digest is rewritten", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |||
| 64 | + | require.NoError(t, os.WriteFile(repository.headPath(id), marshalHead(3, ports.SnapshotDigest{1}), 0o600)) | |||
| 65 | + | }}, | |||
| 66 | + | {name: "malformed HEAD is restored", prepare: func(t *testing.T, repository *Repository, id domain.IncarnationID) { | |||
| 67 | + | require.NoError(t, os.WriteFile(repository.headPath(id), []byte("corrupt"), 0o600)) | |||
| 68 | + | }}, | |||
| 69 | + | } | |||
| 70 | + | for _, tt := range tests { | |||
| 71 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 72 | + | t.Parallel() | |||
| 73 | + | repository := NewRepository(privateDir(t)) | |||
| 74 | + | publications := publishGarbageCollectionGenerations(t, repository, "work", 4) | |||
| 75 | + | id := publications[0].IncarnationID | |||
| 76 | + | committed := domain.CheckpointRef{Generation: 3, ManifestDigest: sha256.Sum256(publications[2].Manifest)} | |||
| 77 | + | tt.prepare(t, repository, id) | |||
| 78 | + | ||||
| 79 | + | require.NoError(t, repository.CollectIncarnationGarbage(t.Context(), id, &committed)) | |||
| 80 | + | ||||
| 81 | + | generation, digest, err := repository.readHead(id) | |||
| 82 | + | require.NoError(t, err) | |||
| 83 | + | require.Equal(t, committed.Generation, generation) | |||
| 84 | + | require.Equal(t, ports.SnapshotDigest(committed.ManifestDigest), digest) | |||
| 85 | + | // The live session retries generation 4 against its committed parent. | |||
| 86 | + | require.NoError(t, repository.Publish(t.Context(), publications[3])) | |||
| 87 | + | }) | |||
| 88 | + | } | |||
| 89 | + | } | |||
| 90 | + | ||||
| 40 | func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) { | 91 | func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) { | ||
| 41 | for _, tt := range []struct { | 92 | for _, tt := range []struct { | ||
| 42 | name string | 93 | name string | ||
| @@ -74,7 +125,7 @@ func TestCollectGarbageRejectsMalformedAuthorityWithoutMutation(t *testing.T) { | |||||
| 74 | committed = tt.mutate(t, repository, id, committed) | 125 | committed = tt.mutate(t, repository, id, committed) | ||
| 75 | before := snapshotRepositoryFiles(t, repository.dir) | 126 | before := snapshotRepositoryFiles(t, repository.dir) | ||
| 76 | 127 | ||||
| 77 | - | require.Error(t, repository.CollectGarbage(t.Context(), map[domain.IncarnationID]domain.CheckpointRef{id: committed})) | 128 | + | require.Error(t, collectAll(t.Context(), repository, map[domain.IncarnationID]domain.CheckpointRef{id: committed})) |
| 78 | require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed") | 129 | require.Equal(t, before, snapshotRepositoryFiles(t, repository.dir), "malformed catalogue authority must fail closed") | ||
| 79 | }) | 130 | }) | ||
| 80 | } | 131 | } | ||
| @@ -103,7 +154,7 @@ func TestCollectGarbage(t *testing.T) { | |||||
| 103 | keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)} | 154 | keep[id] = domain.CheckpointRef{Generation: tt.committed, ManifestDigest: sha256.Sum256(publication.Manifest)} | ||
| 104 | } | 155 | } | ||
| 105 | 156 | ||||
| 106 | - | require.NoError(t, repository.CollectGarbage(context.Background(), keep)) | 157 | + | require.NoError(t, collectAll(context.Background(), repository, keep)) |
| 107 | if !tt.known { | 158 | if !tt.known { | ||
| 108 | _, err := os.Stat(repository.sessionPath(id)) | 159 | _, err := os.Stat(repository.sessionPath(id)) | ||
| 109 | require.ErrorIs(t, err, os.ErrNotExist) | 160 | 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) { | |||
| 45 | 45 | t.Fatal("LoadCheckpoint succeeded through replaced configured-root symlink") | |
| 46 | 46 | } | |
| 47 | 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 { | |
| 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") | |
| 50 | 50 | } | |
| 51 | 51 | loaded, err := loadPublication(context.Background(), external, externalPublication) | |
| 52 | 52 | if err != nil { | |
| @@ -94,8 +94,8 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) { | |||
| 94 | 94 | t.Fatal("LoadCheckpoint succeeded through symlinked repository component") | |
| 95 | 95 | } | |
| 96 | 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 { | |
| 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") | |
| 99 | 99 | } | |
| 100 | 100 | got, err := os.ReadFile(guard) | |
| 101 | 101 | 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 +37 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "os" | |
| 6 | 6 | "path/filepath" | |
| 7 | + | "strings" | |
| 7 | 8 | "syscall" | |
| 8 | 9 | "testing" | |
| 9 | 10 | ||
| @@ -62,6 +63,42 @@ func TestReadBoundedRejectsUnsafeFiles(t *testing.T) { | |||
| 62 | 63 | } | |
| 63 | 64 | } | |
| 64 | 65 | ||
| 66 | + | func TestReadSizedFile(t *testing.T) { | |
| 67 | + | const limit = 16 | |
| 68 | + | tests := []struct { | |
| 69 | + | name string | |
| 70 | + | content string | |
| 71 | + | hintSize int | |
| 72 | + | want string | |
| 73 | + | }{ | |
| 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"}, | |
| 79 | + | } | |
| 80 | + | for _, tt := range tests { | |
| 81 | + | t.Run(tt.name, func(t *testing.T) { | |
| 82 | + | got, err := readSizedFile(strings.NewReader(tt.content), tt.hintSize, limit) | |
| 83 | + | if err != nil { | |
| 84 | + | t.Fatal(err) | |
| 85 | + | } | |
| 86 | + | if string(got) != tt.want { | |
| 87 | + | t.Fatalf("readSizedFile = %q, want %q", got, tt.want) | |
| 88 | + | } | |
| 89 | + | }) | |
| 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 | + | } | |
| 100 | + | } | |
| 101 | + | ||
| 65 | 102 | func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) { | |
| 66 | 103 | pub := repositoryPublication(t, "named", 1, []byte("state")) | |
| 67 | 104 | manifest, err := codec.UnmarshalManifest(pub.Manifest) | |
| @@ -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 | ||||
| @@ -62,6 +63,42 @@ func TestReadBoundedRejectsUnsafeFiles(t *testing.T) { | |||||
| 62 | } | 63 | } | ||
| 63 | } | 64 | } | ||
| 64 | 65 | ||||
| 66 | + | func TestReadSizedFile(t *testing.T) { | |||
| 67 | + | const limit = 16 | |||
| 68 | + | tests := []struct { | |||
| 69 | + | name string | |||
| 70 | + | content string | |||
| 71 | + | hintSize int | |||
| 72 | + | want string | |||
| 73 | + | }{ | |||
| 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"}, | |||
| 79 | + | } | |||
| 80 | + | for _, tt := range tests { | |||
| 81 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 82 | + | got, err := readSizedFile(strings.NewReader(tt.content), tt.hintSize, limit) | |||
| 83 | + | if err != nil { | |||
| 84 | + | t.Fatal(err) | |||
| 85 | + | } | |||
| 86 | + | if string(got) != tt.want { | |||
| 87 | + | t.Fatalf("readSizedFile = %q, want %q", got, tt.want) | |||
| 88 | + | } | |||
| 89 | + | }) | |||
| 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 | + | } | |||
| 100 | + | } | |||
| 101 | + | ||||
| 65 | func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) { | 102 | func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) { | ||
| 66 | pub := repositoryPublication(t, "named", 1, []byte("state")) | 103 | pub := repositoryPublication(t, "named", 1, []byte("state")) | ||
| 67 | manifest, err := codec.UnmarshalManifest(pub.Manifest) | 104 | manifest, err := codec.UnmarshalManifest(pub.Manifest) | ||
M internal/adapters/snapshot/safe_path.go +5 -0
| @@ -82,6 +82,11 @@ func (r *Repository) openDirectory(path string) (file *os.File, err error) { | |||
| 82 | 82 | } | |
| 83 | 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 | 90 | if err := rejectFinalSymlink(root, rel); err != nil { | |
| 86 | 91 | return nil, err | |
| 87 | 92 | } | |
| @@ -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, err | 91 | return nil, err | ||
| 87 | } | 92 | } | ||
M internal/adapters/snapshot/test_helpers_test.go +22 -0
| @@ -1,13 +1,35 @@ | |||
| 1 | 1 | package snapshot | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 4 | 6 | "path/filepath" | |
| 5 | 7 | "testing" | |
| 6 | 8 | ||
| 9 | + | "github.com/bnema/vev/internal/domain" | |
| 10 | + | ||
| 7 | 11 | vt "github.com/bnema/vev-vt" | |
| 8 | 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 | 33 | func privateDir(t *testing.T) string { | |
| 12 | 34 | t.Helper() | |
| 13 | 35 | return filepath.Join(t.TempDir(), "vev") | |
| @@ -1,13 +1,35 @@ | |||||
| 1 | package snapshot | 1 | 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 +8 -6
| @@ -1041,12 +1041,10 @@ func TestLifecycleOwnershipOutlivesMaintenanceWriter(t *testing.T) { | |||
| 1041 | 1041 | shutdownClock, | |
| 1042 | 1042 | discardLog(), | |
| 1043 | 1043 | daemon.WithSnapshotRepository(repository), | |
| 1044 | - | daemon.WithDurableMaintenance(catalogue, maintenanceRepository), | |
| 1044 | + | daemon.WithRecoveryCoordinator(recovery.NewCoordinator(catalogue, maintenanceRepository, nil)), | |
| 1045 | + | daemon.WithSnapshotGarbageCollection(), | |
| 1045 | 1046 | daemon.WithCatalogue(catalogue, nil), | |
| 1046 | 1047 | ) | |
| 1047 | - | if err := d.CollectStartupGarbage(ctx); err != nil { | |
| 1048 | - | return err | |
| 1049 | - | } | |
| 1050 | 1048 | listener, err := ipc.Listen(runtimeDir) | |
| 1051 | 1049 | if err != nil { | |
| 1052 | 1050 | return err | |
| @@ -1055,7 +1053,7 @@ func TestLifecycleOwnershipOutlivesMaintenanceWriter(t *testing.T) { | |||
| 1055 | 1053 | }) | |
| 1056 | 1054 | }() | |
| 1057 | 1055 | ||
| 1058 | - | awaitLifecycleStage(t, maintenanceRepository.entered, "pre-publication maintenance repository call") | |
| 1056 | + | awaitLifecycleStage(t, maintenanceRepository.entered, "background maintenance repository call") | |
| 1059 | 1057 | cancel() | |
| 1060 | 1058 | select { | |
| 1061 | 1059 | case <-callbackReturned: | |
| @@ -1086,7 +1084,11 @@ type lifecycleBlockingMaintenanceRepository struct { | |||
| 1086 | 1084 | returnOnce sync.Once | |
| 1087 | 1085 | } | |
| 1088 | 1086 | ||
| 1089 | - | 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 { | |
| 1090 | 1092 | r.enterOnce.Do(func() { close(r.entered) }) | |
| 1091 | 1093 | <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call. | |
| 1092 | 1094 | r.returnOnce.Do(func() { close(r.returned) }) | |
| @@ -1041,12 +1041,10 @@ 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 | - | if err := d.CollectStartupGarbage(ctx); err != nil { | |||
| 1048 | - | return err | |||
| 1049 | - | } | |||
| 1050 | listener, err := ipc.Listen(runtimeDir) | 1048 | listener, err := ipc.Listen(runtimeDir) | ||
| 1051 | if err != nil { | 1049 | if err != nil { | ||
| 1052 | return err | 1050 | return err | ||
| @@ -1055,7 +1053,7 @@ func TestLifecycleOwnershipOutlivesMaintenanceWriter(t *testing.T) { | |||||
| 1055 | }) | 1053 | }) | ||
| 1056 | }() | 1054 | }() | ||
| 1057 | 1055 | ||||
| 1058 | - | awaitLifecycleStage(t, maintenanceRepository.entered, "pre-publication maintenance repository call") | 1056 | + | awaitLifecycleStage(t, maintenanceRepository.entered, "background maintenance repository call") |
| 1059 | cancel() | 1057 | cancel() | ||
| 1060 | select { | 1058 | select { | ||
| 1061 | case <-callbackReturned: | 1059 | case <-callbackReturned: | ||
| @@ -1086,7 +1084,11 @@ type lifecycleBlockingMaintenanceRepository struct { | |||||
| 1086 | returnOnce sync.Once | 1084 | returnOnce sync.Once | ||
| 1087 | } | 1085 | } | ||
| 1088 | 1086 | ||||
| 1089 | - | 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 { | |||
| 1090 | r.enterOnce.Do(func() { close(r.entered) }) | 1092 | r.enterOnce.Do(func() { close(r.entered) }) | ||
| 1091 | <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call. | 1093 | <-r.release // Intentionally ignore cancellation: lifecycle ownership must outlive this call. | ||
| 1092 | r.returnOnce.Do(func() { close(r.returned) }) | 1094 | r.returnOnce.Do(func() { close(r.returned) }) | ||
M internal/app/run.go +8 -29
| @@ -704,19 +704,6 @@ func logStartupRecoveryCounts(log *slog.Logger, records []domain.CatalogueRecord | |||
| 704 | 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 | - | prepare func(*daemon.Daemon) error, | |
| 710 | - | listen func() (wire.Listener, error), | |
| 711 | - | ) (*daemon.Daemon, wire.Listener, error) { | |
| 712 | - | d := construct() | |
| 713 | - | if err := prepare(d); err != nil { | |
| 714 | - | return d, nil, err | |
| 715 | - | } | |
| 716 | - | ln, err := listen() | |
| 717 | - | return d, ln, err | |
| 718 | - | } | |
| 719 | - | ||
| 720 | 707 | func runDaemonOwned(ctx context.Context) (retErr error) { | |
| 721 | 708 | log, logCloser, err := configureLogging(logging.Daemon, true) | |
| 722 | 709 | if err != nil { | |
| @@ -788,7 +775,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||
| 788 | 775 | } | |
| 789 | 776 | logCatalogueRecovery(log, opened.Records, recoveryMode) | |
| 790 | 777 | daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator)) | |
| 791 | - | daemonOpts = append(daemonOpts, daemon.WithDurableMaintenance(opened.Catalogue, snapshotRepository)) | |
| 778 | + | daemonOpts = append(daemonOpts, daemon.WithSnapshotGarbageCollection()) | |
| 792 | 779 | log.Info("session persistence enabled", "path", storePath) | |
| 793 | 780 | daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records)) | |
| 794 | 781 | ||
| @@ -831,23 +818,15 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||
| 831 | 818 | } | |
| 832 | 819 | }() | |
| 833 | 820 | ||
| 834 | - | d, ln, err := constructDaemonBeforeSocketPublication( | |
| 835 | - | func() *daemon.Daemon { return daemon.New(pty.NewFactory(), clk, log, daemonOpts...) }, | |
| 836 | - | func(d *daemon.Daemon) error { | |
| 837 | - | if err := d.CollectStartupGarbage(ctx); err != nil { | |
| 838 | - | // GC is best-effort, but it is fully finished before socket | |
| 839 | - | // publication. A failed pass leaves durable state untouched and | |
| 840 | - | // restoration retains its per-session failure isolation. | |
| 841 | - | log.Warn("snapshot_garbage_collection_failed", "err", err) | |
| 842 | - | } | |
| 843 | - | return nil | |
| 844 | - | }, | |
| 845 | - | func() (wire.Listener, error) { return listenDaemon(ipc.SocketDir(), observer) }, | |
| 846 | - | ) | |
| 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) | |
| 847 | 826 | if err != nil { | |
| 848 | 827 | closeErr := opened.Catalogue.Close() | |
| 849 | - | log.Error("daemon startup preparation failed", "socket_dir", ipc.SocketDir(), "err", err) | |
| 850 | - | 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) | |
| 851 | 830 | } | |
| 852 | 831 | defer func() { _ = ln.Close() }() | |
| 853 | 832 | log.Info("daemon starting", "socket", ln.Addr()) | |
| @@ -704,19 +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 | - | prepare func(*daemon.Daemon) error, | |||
| 710 | - | listen func() (wire.Listener, error), | |||
| 711 | - | ) (*daemon.Daemon, wire.Listener, error) { | |||
| 712 | - | d := construct() | |||
| 713 | - | if err := prepare(d); err != nil { | |||
| 714 | - | return d, nil, err | |||
| 715 | - | } | |||
| 716 | - | ln, err := listen() | |||
| 717 | - | return d, ln, err | |||
| 718 | - | } | |||
| 719 | - | ||||
| 720 | func runDaemonOwned(ctx context.Context) (retErr error) { | 707 | func runDaemonOwned(ctx context.Context) (retErr error) { | ||
| 721 | log, logCloser, err := configureLogging(logging.Daemon, true) | 708 | log, logCloser, err := configureLogging(logging.Daemon, true) | ||
| 722 | if err != nil { | 709 | if err != nil { | ||
| @@ -788,7 +775,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||||
| 788 | } | 775 | } | ||
| 789 | logCatalogueRecovery(log, opened.Records, recoveryMode) | 776 | logCatalogueRecovery(log, opened.Records, recoveryMode) | ||
| 790 | daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator)) | 777 | daemonOpts = append(daemonOpts, daemon.WithRecoveryCoordinator(coordinator)) | ||
| 791 | - | daemonOpts = append(daemonOpts, daemon.WithDurableMaintenance(opened.Catalogue, snapshotRepository)) | 778 | + | daemonOpts = append(daemonOpts, daemon.WithSnapshotGarbageCollection()) |
| 792 | log.Info("session persistence enabled", "path", storePath) | 779 | log.Info("session persistence enabled", "path", storePath) | ||
| 793 | daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records)) | 780 | daemonOpts = append(daemonOpts, daemon.WithCatalogue(opened.Catalogue, opened.Records)) | ||
| 794 | 781 | ||||
| @@ -831,23 +818,15 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||||
| 831 | } | 818 | } | ||
| 832 | }() | 819 | }() | ||
| 833 | 820 | ||||
| 834 | - | d, ln, err := constructDaemonBeforeSocketPublication( | 821 | + | // The catalogue-backed registry exists before the socket is published. |
| 835 | - | func() *daemon.Daemon { return daemon.New(pty.NewFactory(), clk, log, daemonOpts...) }, | 822 | + | // Snapshot GC runs in the background once restoration finishes, so it never |
| 836 | - | func(d *daemon.Daemon) error { | 823 | + | // delays socket publication. |
| 837 | - | if err := d.CollectStartupGarbage(ctx); err != nil { | 824 | + | d := daemon.New(pty.NewFactory(), clk, log, daemonOpts...) |
| 838 | - | // GC is best-effort, but it is fully finished before socket | 825 | + | ln, err := listenDaemon(ipc.SocketDir(), observer) |
| 839 | - | // publication. A failed pass leaves durable state untouched and | |||
| 840 | - | // restoration retains its per-session failure isolation. | |||
| 841 | - | log.Warn("snapshot_garbage_collection_failed", "err", err) | |||
| 842 | - | } | |||
| 843 | - | return nil | |||
| 844 | - | }, | |||
| 845 | - | func() (wire.Listener, error) { return listenDaemon(ipc.SocketDir(), observer) }, | |||
| 846 | - | ) | |||
| 847 | if err != nil { | 826 | if err != nil { | ||
| 848 | closeErr := opened.Catalogue.Close() | 827 | closeErr := opened.Catalogue.Close() | ||
| 849 | - | log.Error("daemon startup preparation failed", "socket_dir", ipc.SocketDir(), "err", err) | 828 | + | log.Error("daemon listen failed", "socket_dir", ipc.SocketDir(), "err", err) |
| 850 | - | return errors.Join(fmt.Errorf("vev: prepare daemon startup: %w", err), closeErr) | 829 | + | return errors.Join(fmt.Errorf("vev: daemon listen: %w", err), closeErr) |
| 851 | } | 830 | } | ||
| 852 | defer func() { _ = ln.Close() }() | 831 | defer func() { _ = ln.Close() }() | ||
| 853 | log.Info("daemon starting", "socket", ln.Addr()) | 832 | log.Info("daemon starting", "socket", ln.Addr()) | ||
M internal/app/run_test.go +0 -23
| @@ -21,7 +21,6 @@ import ( | |||
| 21 | 21 | "github.com/bnema/vev/internal/ports" | |
| 22 | 22 | "github.com/bnema/vev/internal/protocol" | |
| 23 | 23 | "github.com/bnema/vev/internal/protocol/wire" | |
| 24 | - | "github.com/bnema/vev/internal/usecase/daemon" | |
| 25 | 24 | "github.com/bnema/vev/pkg/kv" | |
| 26 | 25 | "github.com/bnema/vev/pkg/safedir" | |
| 27 | 26 | "github.com/stretchr/testify/require" | |
| @@ -169,28 +168,6 @@ func TestLifecycleOwnershipAcceptsOwnerOnlyVariantLock(t *testing.T) { | |||
| 169 | 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(*daemon.Daemon) error { | |
| 180 | - | require.Equal(t, []string{"catalogue-registry"}, events) | |
| 181 | - | events = append(events, "startup-garbage-collection") | |
| 182 | - | return nil | |
| 183 | - | }, | |
| 184 | - | func() (wire.Listener, error) { | |
| 185 | - | require.Equal(t, []string{"catalogue-registry", "startup-garbage-collection"}, events) | |
| 186 | - | events = append(events, "socket-publication") | |
| 187 | - | return nil, nil | |
| 188 | - | }, | |
| 189 | - | ) | |
| 190 | - | require.NoError(t, err) | |
| 191 | - | require.Equal(t, []string{"catalogue-registry", "startup-garbage-collection", "socket-publication"}, events) | |
| 192 | - | } | |
| 193 | - | ||
| 194 | 171 | func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) { | |
| 195 | 172 | t.Run("ownership prefix is exact", func(t *testing.T) { | |
| 196 | 173 | 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,28 +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(*daemon.Daemon) error { | |||
| 180 | - | require.Equal(t, []string{"catalogue-registry"}, events) | |||
| 181 | - | events = append(events, "startup-garbage-collection") | |||
| 182 | - | return nil | |||
| 183 | - | }, | |||
| 184 | - | func() (wire.Listener, error) { | |||
| 185 | - | require.Equal(t, []string{"catalogue-registry", "startup-garbage-collection"}, events) | |||
| 186 | - | events = append(events, "socket-publication") | |||
| 187 | - | return nil, nil | |||
| 188 | - | }, | |||
| 189 | - | ) | |||
| 190 | - | require.NoError(t, err) | |||
| 191 | - | require.Equal(t, []string{"catalogue-registry", "startup-garbage-collection", "socket-publication"}, events) | |||
| 192 | - | } | |||
| 193 | - | ||||
| 194 | func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) { | 171 | func TestLifecycleOwnershipPrecedesDaemonStartup(t *testing.T) { | ||
| 195 | t.Run("ownership prefix is exact", func(t *testing.T) { | 172 | t.Run("ownership prefix is exact", func(t *testing.T) { | ||
| 196 | var events []string | 173 | var events []string | ||
M internal/ports/mocks/recovery_mock.go +86 -18
| @@ -560,59 +560,65 @@ func (_m *MockSnapshotRepository) EXPECT() *MockSnapshotRepository_Expecter { | |||
| 560 | 560 | return &MockSnapshotRepository_Expecter{mock: &_m.Mock} | |
| 561 | 561 | } | |
| 562 | 562 | ||
| 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) | |
| 566 | 566 | ||
| 567 | 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 | 571 | 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) | |
| 574 | 574 | } else { | |
| 575 | 575 | r0 = ret.Error(0) | |
| 576 | 576 | } | |
| 577 | 577 | 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' | |
| 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 { | |
| 582 | 582 | *mock.Call | |
| 583 | 583 | } | |
| 584 | 584 | ||
| 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)} | |
| 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 | 594 | _c.Call.Run(func(args mock.Arguments) { | |
| 594 | 595 | var arg0 context.Context | |
| 595 | 596 | if args[0] != nil { | |
| 596 | 597 | arg0 = args[0].(context.Context) | |
| 597 | 598 | } | |
| 598 | - | var arg1 map[domain.IncarnationID]domain.CheckpointRef | |
| 599 | + | var arg1 domain.IncarnationID | |
| 599 | 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 | 607 | run( | |
| 603 | 608 | arg0, | |
| 604 | 609 | arg1, | |
| 610 | + | arg2, | |
| 605 | 611 | ) | |
| 606 | 612 | }) | |
| 607 | 613 | 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 | 617 | _c.Call.Return(err) | |
| 612 | 618 | 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 | 622 | _c.Call.Return(run) | |
| 617 | 623 | return _c | |
| 618 | 624 | } | |
| @@ -871,3 +877,65 @@ func (_c *MockSnapshotRepository_ReconcileCheckpoint_Call) RunAndReturn(run func | |||
| 871 | 877 | _c.Call.Return(run) | |
| 872 | 878 | 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 | + | } | |
| @@ -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 MockSnapshotRepository | 563 | + | // 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 error | 571 | 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 r0 | 577 | 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.Call | 582 | *mock.Call | ||
| 583 | } | 583 | } | ||
| 584 | 584 | ||||
| 585 | - | // CollectGarbage is a helper method to define mock.On call | 585 | + | // CollectIncarnationGarbage is a helper method to define mock.On call |
| 586 | - | // - context1 context.Context | 586 | + | // - ctx context.Context |
| 587 | - | // - incarnationIDToCheckpointRef map[domain.IncarnationID]domain.CheckpointRef | 587 | + | // - 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.Context | 595 | 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.CheckpointRef | 599 | + | 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 _c | 613 | 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 _c | 618 | 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 _c | 623 | 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 _c | 878 | 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 { | |||
| 44 | 44 | LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error) | |
| 45 | 45 | ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error | |
| 46 | 46 | 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 | |
| 48 | 53 | } | |
| @@ -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) error | 45 | ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error | ||
| 46 | DeleteIncarnation(context.Context, domain.IncarnationID) error | 46 | 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 | |||
| 48 | } | 53 | } | ||
M internal/usecase/daemon/daemon.go +13 -13
| @@ -297,10 +297,11 @@ type Daemon struct { | |||
| 297 | 297 | catalogueRecords []domain.CatalogueRecord | |
| 298 | 298 | catalogueRecordsProvided bool | |
| 299 | 299 | // snapshotRepository is the sole checkpoint storage contract. | |
| 300 | - | snapshotRepository ports.SnapshotRepository | |
| 301 | - | recovery *recoveryusecase.Coordinator | |
| 302 | - | maintenanceWorkerCancel context.CancelFunc | |
| 303 | - | maintenanceWorkerDone chan struct{} | |
| 300 | + | snapshotRepository ports.SnapshotRepository | |
| 301 | + | recovery *recoveryusecase.Coordinator | |
| 302 | + | snapshotGarbageCollection bool | |
| 303 | + | maintenanceWorkerCancel context.CancelFunc | |
| 304 | + | maintenanceWorkerDone chan struct{} | |
| 304 | 305 | // restoreWorkerDone is the restoration goroutine's ownership signal. Startup | |
| 305 | 306 | // restoration reconciles durable checkpoints, so it is a durable writer and | |
| 306 | 307 | // is guarded by snapshotWorkerMu with the other two. | |
| @@ -555,15 +556,10 @@ func WithNoticeStore(store ports.NoticeStore) Option { | |||
| 555 | 556 | return func(d *Daemon) { d.noticeStore = store } | |
| 556 | 557 | } | |
| 557 | 558 | ||
| 558 | - | // WithDurableMaintenance retains the application wiring for the one | |
| 559 | - | // pre-publication GC pass. Standalone users without an explicitly supplied | |
| 560 | - | // coordinator receive the same canonical coordinator path. | |
| 561 | - | func WithDurableMaintenance(catalogue ports.Catalogue, repository ports.SnapshotRepository) Option { | |
| 562 | - | return func(d *Daemon) { | |
| 563 | - | if d.recovery == nil { | |
| 564 | - | d.recovery = recoveryusecase.NewCoordinator(catalogue, repository, nil) | |
| 565 | - | } | |
| 566 | - | } | |
| 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 } | |
| 567 | 563 | } | |
| 568 | 564 | ||
| 569 | 565 | // WithCwdReader overrides the process cwd reader used for persistence tests. | |
| @@ -830,6 +826,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.ServerListener) error { | |||
| 830 | 826 | d.snapshotRepositorySaver(d.serveCtx) | |
| 831 | 827 | }) | |
| 832 | 828 | d.startSnapshotRestoration() | |
| 829 | + | d.startDurableMaintenance() | |
| 833 | 830 | } else { | |
| 834 | 831 | d.closeRestoreDone() | |
| 835 | 832 | } | |
| @@ -926,6 +923,9 @@ func (d *Daemon) shutdownAll(reason uint8) (checkpointIncomplete bool) { | |||
| 926 | 923 | // shutdownAll and Serve's exit path. It is separate from purgeAllSessions, | |
| 927 | 924 | // which leaves every daemon-wide shutdown fact untouched. | |
| 928 | 925 | func (d *Daemon) terminateAllForShutdown(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | |
| 926 | + | // Stop background GC first so it releases the recovery fence before final | |
| 927 | + | // checkpoints are published under the shutdown deadline. | |
| 928 | + | d.cancelDurableMaintenance() | |
| 929 | 929 | d.closeMoveLifecycles() | |
| 930 | 930 | d.mu.Lock() | |
| 931 | 931 | d.closing = true | |
| @@ -297,10 +297,11 @@ type Daemon struct { | |||||
| 297 | catalogueRecords []domain.CatalogueRecord | 297 | catalogueRecords []domain.CatalogueRecord | ||
| 298 | catalogueRecordsProvided bool | 298 | catalogueRecordsProvided bool | ||
| 299 | // snapshotRepository is the sole checkpoint storage contract. | 299 | // snapshotRepository is the sole checkpoint storage contract. | ||
| 300 | - | snapshotRepository ports.SnapshotRepository | 300 | + | snapshotRepository ports.SnapshotRepository |
| 301 | - | recovery *recoveryusecase.Coordinator | 301 | + | recovery *recoveryusecase.Coordinator |
| 302 | - | maintenanceWorkerCancel context.CancelFunc | 302 | + | snapshotGarbageCollection bool |
| 303 | - | maintenanceWorkerDone chan struct{} | 303 | + | maintenanceWorkerCancel context.CancelFunc |
| 304 | + | maintenanceWorkerDone chan struct{} | |||
| 304 | // restoreWorkerDone is the restoration goroutine's ownership signal. Startup | 305 | // restoreWorkerDone is the restoration goroutine's ownership signal. Startup | ||
| 305 | // restoration reconciles durable checkpoints, so it is a durable writer and | 306 | // restoration reconciles durable checkpoints, so it is a durable writer and | ||
| 306 | // is guarded by snapshotWorkerMu with the other two. | 307 | // is guarded by snapshotWorkerMu with the other two. | ||
| @@ -555,15 +556,10 @@ func WithNoticeStore(store ports.NoticeStore) Option { | |||||
| 555 | return func(d *Daemon) { d.noticeStore = store } | 556 | return func(d *Daemon) { d.noticeStore = store } | ||
| 556 | } | 557 | } | ||
| 557 | 558 | ||||
| 558 | - | // WithDurableMaintenance retains the application wiring for the one | 559 | + | // WithSnapshotGarbageCollection runs background snapshot GC through the |
| 559 | - | // pre-publication GC pass. Standalone users without an explicitly supplied | 560 | + | // recovery coordinator installed by WithRecoveryCoordinator. |
| 560 | - | // coordinator receive the same canonical coordinator path. | 561 | + | func WithSnapshotGarbageCollection() Option { |
| 561 | - | func WithDurableMaintenance(catalogue ports.Catalogue, repository ports.SnapshotRepository) Option { | 562 | + | return func(d *Daemon) { d.snapshotGarbageCollection = true } |
| 562 | - | return func(d *Daemon) { | |||
| 563 | - | if d.recovery == nil { | |||
| 564 | - | d.recovery = recoveryusecase.NewCoordinator(catalogue, repository, nil) | |||
| 565 | - | } | |||
| 566 | - | } | |||
| 567 | } | 563 | } | ||
| 568 | 564 | ||||
| 569 | // WithCwdReader overrides the process cwd reader used for persistence tests. | 565 | // WithCwdReader overrides the process cwd reader used for persistence tests. | ||
| @@ -830,6 +826,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.ServerListener) error { | |||||
| 830 | d.snapshotRepositorySaver(d.serveCtx) | 826 | d.snapshotRepositorySaver(d.serveCtx) | ||
| 831 | }) | 827 | }) | ||
| 832 | d.startSnapshotRestoration() | 828 | d.startSnapshotRestoration() | ||
| 829 | + | d.startDurableMaintenance() | |||
| 833 | } else { | 830 | } else { | ||
| 834 | d.closeRestoreDone() | 831 | d.closeRestoreDone() | ||
| 835 | } | 832 | } | ||
| @@ -926,6 +923,9 @@ func (d *Daemon) shutdownAll(reason uint8) (checkpointIncomplete bool) { | |||||
| 926 | // shutdownAll and Serve's exit path. It is separate from purgeAllSessions, | 923 | // shutdownAll and Serve's exit path. It is separate from purgeAllSessions, | ||
| 927 | // which leaves every daemon-wide shutdown fact untouched. | 924 | // which leaves every daemon-wide shutdown fact untouched. | ||
| 928 | func (d *Daemon) terminateAllForShutdown(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | 925 | func (d *Daemon) terminateAllForShutdown(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | ||
| 926 | + | // Stop background GC first so it releases the recovery fence before final | |||
| 927 | + | // checkpoints are published under the shutdown deadline. | |||
| 928 | + | d.cancelDurableMaintenance() | |||
| 929 | d.closeMoveLifecycles() | 929 | d.closeMoveLifecycles() | ||
| 930 | d.mu.Lock() | 930 | d.mu.Lock() | ||
| 931 | d.closing = true | 931 | d.closing = true | ||
M internal/usecase/daemon/durable_authority_test.go +4 -1
| @@ -18,7 +18,10 @@ type testLifecycleRepository struct{ ports.SnapshotRepository } | |||
| 18 | 18 | func (testLifecycleRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error { | |
| 19 | 19 | 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 | 25 | return nil | |
| 23 | 26 | } | |
| 24 | 27 | ||
| @@ -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 nil | 19 | 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 nil | 25 | return nil | ||
| 23 | } | 26 | } | ||
| 24 | 27 | ||||
M internal/usecase/daemon/maintenance.go +70 -6
| @@ -3,17 +3,81 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "time" | |
| 6 | 7 | ) | |
| 7 | 8 | ||
| 8 | - | // CollectStartupGarbage runs the one coordinator-owned GC pass before the | |
| 9 | - | // daemon socket is published. | |
| 10 | - | func (d *Daemon) CollectStartupGarbage(ctx context.Context) error { | |
| 11 | - | if d == nil || d.recovery == nil { | |
| 12 | - | return nil | |
| 9 | + | // snapshotGarbageCollectionInterval spaces the periodic snapshot GC passes. A | |
| 10 | + | // long-running daemon otherwise accumulates superseded generations and objects | |
| 11 | + | // until its next restart, which then pays for the whole backlog. | |
| 12 | + | const snapshotGarbageCollectionInterval = 30 * time.Minute | |
| 13 | + | ||
| 14 | + | // startDurableMaintenance runs snapshot GC off the startup critical path: once | |
| 15 | + | // catalogue restoration has finished, then periodically. The recovery | |
| 16 | + | // coordinator fences each incarnation's step against catalogue mutations and | |
| 17 | + | // checkpoint publications, so a mutation waits for one incarnation at most. | |
| 18 | + | func (d *Daemon) startDurableMaintenance() { | |
| 19 | + | if d == nil || d.recovery == nil || !d.snapshotGarbageCollection { | |
| 20 | + | return | |
| 21 | + | } | |
| 22 | + | d.snapshotWorkerMu.Lock() | |
| 23 | + | if d.maintenanceWorkerCancel != nil { | |
| 24 | + | d.snapshotWorkerMu.Unlock() | |
| 25 | + | return | |
| 26 | + | } | |
| 27 | + | ctx, cancel := context.WithCancel(d.serveCtx) | |
| 28 | + | d.maintenanceWorkerCancel = cancel | |
| 29 | + | done := make(chan struct{}) | |
| 30 | + | d.maintenanceWorkerDone = done | |
| 31 | + | d.snapshotWorkerMu.Unlock() | |
| 32 | + | ||
| 33 | + | go func() { | |
| 34 | + | defer close(done) | |
| 35 | + | d.runDurableMaintenance(ctx) | |
| 36 | + | }() | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | // cancelDurableMaintenance stops the GC worker without waiting for it; | |
| 40 | + | // WaitDurableWriters owns the join. | |
| 41 | + | func (d *Daemon) cancelDurableMaintenance() { | |
| 42 | + | d.snapshotWorkerMu.Lock() | |
| 43 | + | defer d.snapshotWorkerMu.Unlock() | |
| 44 | + | d.cancelDurableMaintenanceLocked() | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | // cancelDurableMaintenanceLocked requires snapshotWorkerMu. | |
| 48 | + | func (d *Daemon) cancelDurableMaintenanceLocked() { | |
| 49 | + | if d.maintenanceWorkerCancel != nil { | |
| 50 | + | d.maintenanceWorkerCancel() | |
| 51 | + | } | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | func (d *Daemon) runDurableMaintenance(ctx context.Context) { | |
| 55 | + | // Restoration reads committed checkpoints; collecting afterwards keeps the | |
| 56 | + | // pass from competing with it for disk and for the coordinator fence. | |
| 57 | + | select { | |
| 58 | + | case <-ctx.Done(): | |
| 59 | + | return | |
| 60 | + | case <-d.restoreDone: | |
| 13 | 61 | } | |
| 62 | + | for { | |
| 63 | + | if err := d.collectSnapshotGarbage(ctx); err != nil && ctx.Err() == nil { | |
| 64 | + | d.log.Warn("snapshot_garbage_collection_failed", "err", err) | |
| 65 | + | } | |
| 66 | + | timer := d.clock.NewTimer(snapshotGarbageCollectionInterval) | |
| 67 | + | select { | |
| 68 | + | case <-ctx.Done(): | |
| 69 | + | timer.Stop() | |
| 70 | + | return | |
| 71 | + | case <-timer.C(): | |
| 72 | + | } | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | // collectSnapshotGarbage runs one coordinator-owned GC pass. | |
| 77 | + | func (d *Daemon) collectSnapshotGarbage(ctx context.Context) error { | |
| 14 | 78 | incarnations, err := d.recovery.CollectGarbage(ctx) | |
| 15 | 79 | if err != nil { | |
| 16 | - | return fmt.Errorf("collect startup snapshots: %w", err) | |
| 80 | + | return fmt.Errorf("collect snapshots: %w", err) | |
| 17 | 81 | } | |
| 18 | 82 | d.log.Info("snapshot_garbage_collection_complete", "incarnations", incarnations) | |
| 19 | 83 | return nil | |
| @@ -3,17 +3,81 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "time" | |||
| 6 | ) | 7 | ) | ||
| 7 | 8 | ||||
| 8 | - | // CollectStartupGarbage runs the one coordinator-owned GC pass before the | 9 | + | // snapshotGarbageCollectionInterval spaces the periodic snapshot GC passes. A |
| 9 | - | // daemon socket is published. | 10 | + | // long-running daemon otherwise accumulates superseded generations and objects |
| 10 | - | func (d *Daemon) CollectStartupGarbage(ctx context.Context) error { | 11 | + | // until its next restart, which then pays for the whole backlog. |
| 11 | - | if d == nil || d.recovery == nil { | 12 | + | const snapshotGarbageCollectionInterval = 30 * time.Minute |
| 12 | - | return nil | 13 | + | |
| 14 | + | // startDurableMaintenance runs snapshot GC off the startup critical path: once | |||
| 15 | + | // catalogue restoration has finished, then periodically. The recovery | |||
| 16 | + | // coordinator fences each incarnation's step against catalogue mutations and | |||
| 17 | + | // checkpoint publications, so a mutation waits for one incarnation at most. | |||
| 18 | + | func (d *Daemon) startDurableMaintenance() { | |||
| 19 | + | if d == nil || d.recovery == nil || !d.snapshotGarbageCollection { | |||
| 20 | + | return | |||
| 21 | + | } | |||
| 22 | + | d.snapshotWorkerMu.Lock() | |||
| 23 | + | if d.maintenanceWorkerCancel != nil { | |||
| 24 | + | d.snapshotWorkerMu.Unlock() | |||
| 25 | + | return | |||
| 26 | + | } | |||
| 27 | + | ctx, cancel := context.WithCancel(d.serveCtx) | |||
| 28 | + | d.maintenanceWorkerCancel = cancel | |||
| 29 | + | done := make(chan struct{}) | |||
| 30 | + | d.maintenanceWorkerDone = done | |||
| 31 | + | d.snapshotWorkerMu.Unlock() | |||
| 32 | + | ||||
| 33 | + | go func() { | |||
| 34 | + | defer close(done) | |||
| 35 | + | d.runDurableMaintenance(ctx) | |||
| 36 | + | }() | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | // cancelDurableMaintenance stops the GC worker without waiting for it; | |||
| 40 | + | // WaitDurableWriters owns the join. | |||
| 41 | + | func (d *Daemon) cancelDurableMaintenance() { | |||
| 42 | + | d.snapshotWorkerMu.Lock() | |||
| 43 | + | defer d.snapshotWorkerMu.Unlock() | |||
| 44 | + | d.cancelDurableMaintenanceLocked() | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | // cancelDurableMaintenanceLocked requires snapshotWorkerMu. | |||
| 48 | + | func (d *Daemon) cancelDurableMaintenanceLocked() { | |||
| 49 | + | if d.maintenanceWorkerCancel != nil { | |||
| 50 | + | d.maintenanceWorkerCancel() | |||
| 51 | + | } | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | func (d *Daemon) runDurableMaintenance(ctx context.Context) { | |||
| 55 | + | // Restoration reads committed checkpoints; collecting afterwards keeps the | |||
| 56 | + | // pass from competing with it for disk and for the coordinator fence. | |||
| 57 | + | select { | |||
| 58 | + | case <-ctx.Done(): | |||
| 59 | + | return | |||
| 60 | + | case <-d.restoreDone: | |||
| 13 | } | 61 | } | ||
| 62 | + | for { | |||
| 63 | + | if err := d.collectSnapshotGarbage(ctx); err != nil && ctx.Err() == nil { | |||
| 64 | + | d.log.Warn("snapshot_garbage_collection_failed", "err", err) | |||
| 65 | + | } | |||
| 66 | + | timer := d.clock.NewTimer(snapshotGarbageCollectionInterval) | |||
| 67 | + | select { | |||
| 68 | + | case <-ctx.Done(): | |||
| 69 | + | timer.Stop() | |||
| 70 | + | return | |||
| 71 | + | case <-timer.C(): | |||
| 72 | + | } | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | // collectSnapshotGarbage runs one coordinator-owned GC pass. | |||
| 77 | + | func (d *Daemon) collectSnapshotGarbage(ctx context.Context) error { | |||
| 14 | incarnations, err := d.recovery.CollectGarbage(ctx) | 78 | incarnations, err := d.recovery.CollectGarbage(ctx) | ||
| 15 | if err != nil { | 79 | if err != nil { | ||
| 16 | - | return fmt.Errorf("collect startup snapshots: %w", err) | 80 | + | return fmt.Errorf("collect snapshots: %w", err) |
| 17 | } | 81 | } | ||
| 18 | d.log.Info("snapshot_garbage_collection_complete", "incarnations", incarnations) | 82 | d.log.Info("snapshot_garbage_collection_complete", "incarnations", incarnations) | ||
| 19 | return nil | 83 | return nil | ||
M internal/usecase/daemon/maintenance_test.go +63 -20
| @@ -4,38 +4,81 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "testing" | |
| 7 | + | "testing/synctest" | |
| 8 | + | "time" | |
| 7 | 9 | ||
| 8 | 10 | "github.com/bnema/vev/internal/domain" | |
| 9 | 11 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 12 | + | recoveryusecase "github.com/bnema/vev/internal/usecase/recovery" | |
| 10 | 13 | "github.com/stretchr/testify/mock" | |
| 11 | 14 | "github.com/stretchr/testify/require" | |
| 12 | 15 | ) | |
| 13 | 16 | ||
| 14 | - | func TestStartupGarbageCollectionUsesValidatedCatalogue(t *testing.T) { | |
| 17 | + | func TestSnapshotGarbageCollectionRunsAfterRestorationThenPeriodically(t *testing.T) { | |
| 15 | 18 | committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}} | |
| 16 | - | records := []domain.CatalogueRecord{ | |
| 17 | - | {Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}, | |
| 18 | - | {Name: "fresh", IncarnationID: domain.IncarnationID{2}}, | |
| 19 | + | records := []domain.CatalogueRecord{{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}} | |
| 20 | + | id := records[0].IncarnationID | |
| 21 | + | ||
| 22 | + | tests := []struct { | |
| 23 | + | name string | |
| 24 | + | recordsErr error | |
| 25 | + | wantCollect bool | |
| 26 | + | }{ | |
| 27 | + | {name: "collects after restoration and on every tick", wantCollect: true}, | |
| 28 | + | {name: "unreadable catalogue never collects", recordsErr: errors.New("catalogue read failed")}, | |
| 19 | 29 | } | |
| 20 | - | catalogue := portsmocks.NewMockCatalogue(t) | |
| 21 | - | catalogue.EXPECT().Records().Return(records, nil).Once() | |
| 22 | - | repository := portsmocks.NewMockSnapshotRepository(t) | |
| 23 | - | repository.EXPECT().CollectGarbage(mock.Anything, mock.MatchedBy(func(keep map[domain.IncarnationID]domain.CheckpointRef) bool { | |
| 24 | - | return len(keep) == 2 && keep[records[0].IncarnationID] == committed && keep[records[1].IncarnationID] == (domain.CheckpointRef{}) | |
| 25 | - | })).Return(nil).Once() | |
| 26 | - | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | |
| 27 | - | WithDurableMaintenance(catalogue, repository)(d) | |
| 30 | + | for _, tt := range tests { | |
| 31 | + | t.Run(tt.name, func(t *testing.T) { | |
| 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 | + | } | |
| 44 | + | ||
| 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() | |
| 28 | 50 | ||
| 29 | - | require.NoError(t, d.CollectStartupGarbage(context.Background())) | |
| 51 | + | synctest.Wait() | |
| 52 | + | require.Empty(t, clk.timers, "GC must wait for restoration to finish") | |
| 53 | + | d.closeRestoreDone() | |
| 54 | + | ||
| 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 | + | } | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | d.serveCancel() | |
| 69 | + | d.WaitDurableWriters() | |
| 70 | + | }) | |
| 71 | + | }) | |
| 72 | + | } | |
| 30 | 73 | } | |
| 31 | 74 | ||
| 32 | - | func TestStartupGarbageCollectionSkipsUnknownCatalogueState(t *testing.T) { | |
| 33 | - | catalogue := portsmocks.NewMockCatalogue(t) | |
| 34 | - | catalogue.EXPECT().Records().Return(nil, errors.New("catalogue read failed")).Once() | |
| 35 | - | repository := portsmocks.NewMockSnapshotRepository(t) | |
| 75 | + | func TestSnapshotGarbageCollectionRequiresExplicitOption(t *testing.T) { | |
| 36 | 76 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | |
| 37 | - | WithDurableMaintenance(catalogue, repository)(d) | |
| 77 | + | coordinator := recoveryusecase.NewCoordinator(portsmocks.NewMockCatalogue(t), portsmocks.NewMockSnapshotRepository(t), nil) | |
| 78 | + | WithRecoveryCoordinator(coordinator)(d) | |
| 79 | + | d.startDurableMaintenance() | |
| 38 | 80 | ||
| 39 | - | require.Error(t, d.CollectStartupGarbage(context.Background())) | |
| 40 | - | repository.AssertNotCalled(t, "CollectGarbage", mock.Anything, mock.Anything) | |
| 81 | + | d.snapshotWorkerMu.Lock() | |
| 82 | + | defer d.snapshotWorkerMu.Unlock() | |
| 83 | + | require.Nil(t, d.maintenanceWorkerDone) | |
| 41 | 84 | } | |
| @@ -4,38 +4,81 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | + | "testing/synctest" | |||
| 8 | + | "time" | |||
| 7 | 9 | ||||
| 8 | "github.com/bnema/vev/internal/domain" | 10 | "github.com/bnema/vev/internal/domain" | ||
| 9 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | 11 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | ||
| 12 | + | recoveryusecase "github.com/bnema/vev/internal/usecase/recovery" | |||
| 10 | "github.com/stretchr/testify/mock" | 13 | "github.com/stretchr/testify/mock" | ||
| 11 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 12 | ) | 15 | ) | ||
| 13 | 16 | ||||
| 14 | - | func TestStartupGarbageCollectionUsesValidatedCatalogue(t *testing.T) { | 17 | + | func TestSnapshotGarbageCollectionRunsAfterRestorationThenPeriodically(t *testing.T) { |
| 15 | committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}} | 18 | committed := domain.CheckpointRef{Generation: 3, ManifestDigest: [32]byte{3}} | ||
| 16 | - | records := []domain.CatalogueRecord{ | 19 | + | records := []domain.CatalogueRecord{{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}} |
| 17 | - | {Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed}, | 20 | + | id := records[0].IncarnationID |
| 18 | - | {Name: "fresh", IncarnationID: domain.IncarnationID{2}}, | 21 | + | |
| 22 | + | tests := []struct { | |||
| 23 | + | name string | |||
| 24 | + | recordsErr error | |||
| 25 | + | wantCollect bool | |||
| 26 | + | }{ | |||
| 27 | + | {name: "collects after restoration and on every tick", wantCollect: true}, | |||
| 28 | + | {name: "unreadable catalogue never collects", recordsErr: errors.New("catalogue read failed")}, | |||
| 19 | } | 29 | } | ||
| 20 | - | catalogue := portsmocks.NewMockCatalogue(t) | 30 | + | for _, tt := range tests { |
| 21 | - | catalogue.EXPECT().Records().Return(records, nil).Once() | 31 | + | t.Run(tt.name, func(t *testing.T) { |
| 22 | - | repository := portsmocks.NewMockSnapshotRepository(t) | 32 | + | synctest.Test(t, func(t *testing.T) { |
| 23 | - | repository.EXPECT().CollectGarbage(mock.Anything, mock.MatchedBy(func(keep map[domain.IncarnationID]domain.CheckpointRef) bool { | 33 | + | const passes = 2 |
| 24 | - | return len(keep) == 2 && keep[records[0].IncarnationID] == committed && keep[records[1].IncarnationID] == (domain.CheckpointRef{}) | 34 | + | catalogue := portsmocks.NewMockCatalogue(t) |
| 25 | - | })).Return(nil).Once() | 35 | + | repository := portsmocks.NewMockSnapshotRepository(t) |
| 26 | - | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | 36 | + | catalogue.EXPECT().Records().Return(records, tt.recordsErr).Times(passes) |
| 27 | - | WithDurableMaintenance(catalogue, repository)(d) | 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 | + | } | |||
| 44 | + | ||||
| 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() | |||
| 28 | 50 | ||||
| 29 | - | require.NoError(t, d.CollectStartupGarbage(context.Background())) | 51 | + | synctest.Wait() |
| 52 | + | require.Empty(t, clk.timers, "GC must wait for restoration to finish") | |||
| 53 | + | d.closeRestoreDone() | |||
| 54 | + | ||||
| 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 | + | } | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | d.serveCancel() | |||
| 69 | + | d.WaitDurableWriters() | |||
| 70 | + | }) | |||
| 71 | + | }) | |||
| 72 | + | } | |||
| 30 | } | 73 | } | ||
| 31 | 74 | ||||
| 32 | - | func TestStartupGarbageCollectionSkipsUnknownCatalogueState(t *testing.T) { | 75 | + | func TestSnapshotGarbageCollectionRequiresExplicitOption(t *testing.T) { |
| 33 | - | catalogue := portsmocks.NewMockCatalogue(t) | |||
| 34 | - | catalogue.EXPECT().Records().Return(nil, errors.New("catalogue read failed")).Once() | |||
| 35 | - | repository := portsmocks.NewMockSnapshotRepository(t) | |||
| 36 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | 76 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | ||
| 37 | - | WithDurableMaintenance(catalogue, repository)(d) | 77 | + | coordinator := recoveryusecase.NewCoordinator(portsmocks.NewMockCatalogue(t), portsmocks.NewMockSnapshotRepository(t), nil) |
| 78 | + | WithRecoveryCoordinator(coordinator)(d) | |||
| 79 | + | d.startDurableMaintenance() | |||
| 38 | 80 | ||||
| 39 | - | require.Error(t, d.CollectStartupGarbage(context.Background())) | 81 | + | d.snapshotWorkerMu.Lock() |
| 40 | - | repository.AssertNotCalled(t, "CollectGarbage", mock.Anything, mock.Anything) | 82 | + | defer d.snapshotWorkerMu.Unlock() |
| 83 | + | require.Nil(t, d.maintenanceWorkerDone) | |||
| 41 | } | 84 | } | ||
M internal/usecase/daemon/session.go +1 -1
| @@ -1272,7 +1272,7 @@ func (d *Daemon) beginSnapshotPurge(_ string, _ domain.IncarnationID) error { | |||
| 1272 | 1272 | } | |
| 1273 | 1273 | ||
| 1274 | 1274 | // finishSnapshotPurge removes catalogue metadata first, then deletes the | |
| 1275 | - | // incarnation directory. Startup garbage collection removes the directory if | |
| 1275 | + | // incarnation directory. Background garbage collection removes the directory if | |
| 1276 | 1276 | // the second step is interrupted. | |
| 1277 | 1277 | func (d *Daemon) finishSnapshotPurge(ctx context.Context, name string, incarnation domain.IncarnationID, createdAt int64) error { | |
| 1278 | 1278 | if !d.persistEnabled { | |
| @@ -1272,7 +1272,7 @@ func (d *Daemon) beginSnapshotPurge(_ string, _ domain.IncarnationID) error { | |||||
| 1272 | } | 1272 | } | ||
| 1273 | 1273 | ||||
| 1274 | // finishSnapshotPurge removes catalogue metadata first, then deletes the | 1274 | // finishSnapshotPurge removes catalogue metadata first, then deletes the | ||
| 1275 | - | // incarnation directory. Startup garbage collection removes the directory if | 1275 | + | // incarnation directory. Background garbage collection removes the directory if |
| 1276 | // the second step is interrupted. | 1276 | // the second step is interrupted. | ||
| 1277 | func (d *Daemon) finishSnapshotPurge(ctx context.Context, name string, incarnation domain.IncarnationID, createdAt int64) error { | 1277 | func (d *Daemon) finishSnapshotPurge(ctx context.Context, name string, incarnation domain.IncarnationID, createdAt int64) error { | ||
| 1278 | if !d.persistEnabled { | 1278 | if !d.persistEnabled { | ||
M internal/usecase/daemon/session_purge_retry_test.go +1 -1
| @@ -26,7 +26,7 @@ func (r *retryablePurgeRepository) DeleteIncarnation(ctx context.Context, _ doma | |||
| 26 | 26 | return r.deleteErr | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | - | func TestLivePurgeLeavesFailedDirectoryDeletionForStartupGarbageCollection(t *testing.T) { | |
| 29 | + | func TestLivePurgeLeavesFailedDirectoryDeletionForBackgroundGarbageCollection(t *testing.T) { | |
| 30 | 30 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | |
| 31 | 31 | repository := &retryablePurgeRepository{deleteErr: errors.New("delete failed")} | |
| 32 | 32 | WithSnapshotRepository(repository)(d) | |
| @@ -26,7 +26,7 @@ func (r *retryablePurgeRepository) DeleteIncarnation(ctx context.Context, _ doma | |||||
| 26 | return r.deleteErr | 26 | 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_restore_incremental.go +5 -2
| @@ -428,9 +428,12 @@ func generationObject(generation ports.SnapshotGeneration, ref snapcodec.ObjectR | |||
| 428 | 428 | if !ok || uint32(len(data)) != ref.Size || sha256.Sum256(data) != ref.Digest { | |
| 429 | 429 | return nil, fmt.Errorf("snapshot: missing object") | |
| 430 | 430 | } | |
| 431 | - | gotKind, payload, err := snapcodec.UnmarshalObject(data) | |
| 431 | + | // The payload aliases the loaded object. A generation's objects are private | |
| 432 | + | // to this restore and are only decoded, never mutated, so copying here would | |
| 433 | + | // double the transient footprint of every restored history chunk. | |
| 434 | + | gotKind, payload, err := snapcodec.PreflightObject(data) | |
| 432 | 435 | if err != nil || gotKind != kind { | |
| 433 | 436 | return nil, fmt.Errorf("snapshot: invalid object") | |
| 434 | 437 | } | |
| 435 | - | return append([]byte(nil), payload...), nil | |
| 438 | + | return payload, nil | |
| 436 | 439 | } | |
| @@ -428,9 +428,12 @@ func generationObject(generation ports.SnapshotGeneration, ref snapcodec.ObjectR | |||||
| 428 | if !ok || uint32(len(data)) != ref.Size || sha256.Sum256(data) != ref.Digest { | 428 | if !ok || uint32(len(data)) != ref.Size || sha256.Sum256(data) != ref.Digest { | ||
| 429 | return nil, fmt.Errorf("snapshot: missing object") | 429 | return nil, fmt.Errorf("snapshot: missing object") | ||
| 430 | } | 430 | } | ||
| 431 | - | gotKind, payload, err := snapcodec.UnmarshalObject(data) | 431 | + | // The payload aliases the loaded object. A generation's objects are private |
| 432 | + | // to this restore and are only decoded, never mutated, so copying here would | |||
| 433 | + | // double the transient footprint of every restored history chunk. | |||
| 434 | + | gotKind, payload, err := snapcodec.PreflightObject(data) | |||
| 432 | if err != nil || gotKind != kind { | 435 | if err != nil || gotKind != kind { | ||
| 433 | return nil, fmt.Errorf("snapshot: invalid object") | 436 | return nil, fmt.Errorf("snapshot: invalid object") | ||
| 434 | } | 437 | } | ||
| 435 | - | return append([]byte(nil), payload...), nil | 438 | + | return payload, nil |
| 436 | } | 439 | } | ||
M internal/usecase/daemon/snapshot_resume_publication_test.go +5 -1
| @@ -117,7 +117,11 @@ func (r *snapshotLifecycleRepository) DeleteIncarnation(ctx context.Context, id | |||
| 117 | 117 | 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 | 125 | return ctx.Err() | |
| 122 | 126 | } | |
| 123 | 127 | ||
| @@ -117,7 +117,11 @@ func (r *snapshotLifecycleRepository) DeleteIncarnation(ctx context.Context, id | |||||
| 117 | return nil | 117 | 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 | |||
| 113 | 113 | func (noOpSnapshotRepository) DeleteIncarnation(context.Context, domain.IncarnationID) error { | |
| 114 | 114 | 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 | 120 | return nil | |
| 118 | 121 | } | |
| 119 | 122 | ||
| @@ -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 nil | 114 | 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 nil | 120 | return nil | ||
| 118 | } | 121 | } | ||
| 119 | 122 | ||||
M internal/usecase/daemon/snapshot_worker.go +1 -3
| @@ -262,9 +262,7 @@ func (d *Daemon) WaitDurableWriters() { | |||
| 262 | 262 | func (d *Daemon) requestDurableWriterStop() (context.CancelFunc, <-chan struct{}) { | |
| 263 | 263 | d.snapshotWorkerMu.Lock() | |
| 264 | 264 | defer d.snapshotWorkerMu.Unlock() | |
| 265 | - | if d.maintenanceWorkerCancel != nil { | |
| 266 | - | d.maintenanceWorkerCancel() | |
| 267 | - | } | |
| 265 | + | d.cancelDurableMaintenanceLocked() | |
| 268 | 266 | cancel := d.snapshotWorkerCancel | |
| 269 | 267 | if cancel == nil { | |
| 270 | 268 | 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.snapshotWorkerCancel | 266 | cancel := d.snapshotWorkerCancel | ||
| 269 | if cancel == nil { | 267 | if cancel == nil { | ||
| 270 | return nil, nil | 268 | return nil, nil | ||
M internal/usecase/recovery/coordinator.go +43 -18
| @@ -85,10 +85,9 @@ type Coordinator struct { | |||
| 85 | 85 | catalogue ports.Catalogue | |
| 86 | 86 | repository ports.SnapshotRepository | |
| 87 | 87 | locks *KeyLocks | |
| 88 | - | // mutationMu serializes the startup GC snapshot and collection with every | |
| 89 | - | // catalogue or checkpoint mutation. Startup normally provides this fence by | |
| 90 | - | // running before socket publication; retaining it here also makes the | |
| 91 | - | // coordinator safe when used directly by a composition root. | |
| 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. | |
| 92 | 91 | mutationMu sync.Mutex | |
| 93 | 92 | random io.Reader | |
| 94 | 93 | } | |
| @@ -223,10 +222,16 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation | |||
| 223 | 222 | return c.repository.DeleteIncarnation(ctx, incarnation) | |
| 224 | 223 | } | |
| 225 | 224 | ||
| 226 | - | // CollectGarbage takes a catalogue snapshot and applies retention while all | |
| 227 | - | // catalogue mutations and checkpoint publications are fenced. Its keep map is | |
| 228 | - | // therefore never stale relative to an operation that creates an incarnation | |
| 229 | - | // 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. | |
| 230 | 235 | func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) { | |
| 231 | 236 | if c == nil || c.catalogue == nil || c.repository == nil { | |
| 232 | 237 | return 0, errors.New("recovery: incomplete garbage collection dependencies") | |
| @@ -234,23 +239,43 @@ func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) { | |||
| 234 | 239 | if err := ctx.Err(); err != nil { | |
| 235 | 240 | return 0, err | |
| 236 | 241 | } | |
| 242 | + | ids, err := c.repository.SnapshotIncarnations(ctx) | |
| 243 | + | if err != nil { | |
| 244 | + | return 0, err | |
| 245 | + | } | |
| 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 | + | } | |
| 256 | + | } | |
| 257 | + | return len(ids), collected | |
| 258 | + | } | |
| 259 | + | ||
| 260 | + | func (c *Coordinator) collectIncarnation(ctx context.Context, id domain.IncarnationID) error { | |
| 237 | 261 | c.mutationMu.Lock() | |
| 238 | 262 | defer c.mutationMu.Unlock() | |
| 239 | - | ||
| 263 | + | // A catalogue read failure never produces a keep decision, so destructive | |
| 264 | + | // collection cannot run without known-good catalogue state. | |
| 240 | 265 | records, err := c.catalogue.Records() | |
| 241 | 266 | if err != nil { | |
| 242 | - | return 0, err | |
| 267 | + | return fmt.Errorf("%w: %w", errGarbageCollectionCatalogue, err) | |
| 243 | 268 | } | |
| 244 | - | keep := make(map[domain.IncarnationID]domain.CheckpointRef, len(records)) | |
| 269 | + | var keep *domain.CheckpointRef | |
| 245 | 270 | for _, record := range records { | |
| 246 | - | if record.Committed != nil { | |
| 247 | - | keep[record.IncarnationID] = *record.Committed | |
| 271 | + | if record.IncarnationID != id { | |
| 248 | 272 | continue | |
| 249 | 273 | } | |
| 250 | - | keep[record.IncarnationID] = domain.CheckpointRef{} | |
| 251 | - | } | |
| 252 | - | if err := c.repository.CollectGarbage(ctx, keep); err != nil { | |
| 253 | - | return len(keep), err | |
| 274 | + | keep = &domain.CheckpointRef{} | |
| 275 | + | if record.Committed != nil { | |
| 276 | + | *keep = *record.Committed | |
| 277 | + | } | |
| 278 | + | break | |
| 254 | 279 | } | |
| 255 | - | return len(keep), nil | |
| 280 | + | return c.repository.CollectIncarnationGarbage(ctx, id, keep) | |
| 256 | 281 | } | |
| @@ -85,10 +85,9 @@ type Coordinator struct { | |||||
| 85 | catalogue ports.Catalogue | 85 | catalogue ports.Catalogue | ||
| 86 | repository ports.SnapshotRepository | 86 | repository ports.SnapshotRepository | ||
| 87 | locks *KeyLocks | 87 | locks *KeyLocks | ||
| 88 | - | // mutationMu serializes the startup GC snapshot and collection with every | 88 | + | // mutationMu serializes every catalogue or checkpoint mutation with each |
| 89 | - | // catalogue or checkpoint mutation. Startup normally provides this fence by | 89 | + | // per-incarnation GC step. It is the only fence: GC runs while the daemon |
| 90 | - | // running before socket publication; retaining it here also makes the | 90 | + | // serves, so a step holds it for one incarnation at a time. |
| 91 | - | // coordinator safe when used directly by a composition root. | |||
| 92 | mutationMu sync.Mutex | 91 | mutationMu sync.Mutex | ||
| 93 | random io.Reader | 92 | random io.Reader | ||
| 94 | } | 93 | } | ||
| @@ -223,10 +222,16 @@ func (c *Coordinator) DeleteExact(ctx context.Context, name string, incarnation | |||||
| 223 | return c.repository.DeleteIncarnation(ctx, incarnation) | 222 | return c.repository.DeleteIncarnation(ctx, incarnation) | ||
| 224 | } | 223 | } | ||
| 225 | 224 | ||||
| 226 | - | // CollectGarbage takes a catalogue snapshot and applies retention while all | 225 | + | // errGarbageCollectionCatalogue stops a pass: without a readable catalogue no |
| 227 | - | // catalogue mutations and checkpoint publications are fenced. Its keep map is | 226 | + | // incarnation can be classified. |
| 228 | - | // therefore never stale relative to an operation that creates an incarnation | 227 | + | var errGarbageCollectionCatalogue = errors.New("recovery: read catalogue for garbage collection") |
| 229 | - | // 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. | |||
| 230 | func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) { | 235 | func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) { | ||
| 231 | if c == nil || c.catalogue == nil || c.repository == nil { | 236 | if c == nil || c.catalogue == nil || c.repository == nil { | ||
| 232 | return 0, errors.New("recovery: incomplete garbage collection dependencies") | 237 | return 0, errors.New("recovery: incomplete garbage collection dependencies") | ||
| @@ -234,23 +239,43 @@ func (c *Coordinator) CollectGarbage(ctx context.Context) (int, error) { | |||||
| 234 | if err := ctx.Err(); err != nil { | 239 | if err := ctx.Err(); err != nil { | ||
| 235 | return 0, err | 240 | return 0, err | ||
| 236 | } | 241 | } | ||
| 242 | + | ids, err := c.repository.SnapshotIncarnations(ctx) | |||
| 243 | + | if err != nil { | |||
| 244 | + | return 0, err | |||
| 245 | + | } | |||
| 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 | + | } | |||
| 256 | + | } | |||
| 257 | + | return len(ids), collected | |||
| 258 | + | } | |||
| 259 | + | ||||
| 260 | + | func (c *Coordinator) collectIncarnation(ctx context.Context, id domain.IncarnationID) error { | |||
| 237 | c.mutationMu.Lock() | 261 | c.mutationMu.Lock() | ||
| 238 | defer c.mutationMu.Unlock() | 262 | defer c.mutationMu.Unlock() | ||
| 239 | - | 263 | + | // A catalogue read failure never produces a keep decision, so destructive | |
| 264 | + | // collection cannot run without known-good catalogue state. | |||
| 240 | records, err := c.catalogue.Records() | 265 | records, err := c.catalogue.Records() | ||
| 241 | if err != nil { | 266 | if err != nil { | ||
| 242 | - | return 0, err | 267 | + | return fmt.Errorf("%w: %w", errGarbageCollectionCatalogue, err) |
| 243 | } | 268 | } | ||
| 244 | - | keep := make(map[domain.IncarnationID]domain.CheckpointRef, len(records)) | 269 | + | var keep *domain.CheckpointRef |
| 245 | for _, record := range records { | 270 | for _, record := range records { | ||
| 246 | - | if record.Committed != nil { | 271 | + | if record.IncarnationID != id { |
| 247 | - | keep[record.IncarnationID] = *record.Committed | |||
| 248 | continue | 272 | continue | ||
| 249 | } | 273 | } | ||
| 250 | - | keep[record.IncarnationID] = domain.CheckpointRef{} | 274 | + | keep = &domain.CheckpointRef{} |
| 251 | - | } | 275 | + | if record.Committed != nil { |
| 252 | - | if err := c.repository.CollectGarbage(ctx, keep); err != nil { | 276 | + | *keep = *record.Committed |
| 253 | - | return len(keep), err | 277 | + | } |
| 278 | + | break | |||
| 254 | } | 279 | } | ||
| 255 | - | return len(keep), nil | 280 | + | return c.repository.CollectIncarnationGarbage(ctx, id, keep) |
| 256 | } | 281 | } | ||
M internal/usecase/recovery/discard.go +1 -1
| @@ -14,7 +14,7 @@ var ( | |||
| 14 | 14 | // Discard throws away a broken session's persisted state and replaces it with a | |
| 15 | 15 | // fresh incarnation. It is retry-idempotent, not crash-resumable: a crash | |
| 16 | 16 | // leaves either the old record (rerun discard) or a new record plus an orphan | |
| 17 | - | // directory that startup garbage collection removes. | |
| 17 | + | // directory that background garbage collection removes. | |
| 18 | 18 | func (c *Coordinator) Discard(ctx context.Context, name string) error { | |
| 19 | 19 | if c == nil || c.catalogue == nil || c.repository == nil || c.locks == nil || c.random == nil { | |
| 20 | 20 | return errors.New("recovery: incomplete discard dependencies") | |
| @@ -14,7 +14,7 @@ var ( | |||||
| 14 | // Discard throws away a broken session's persisted state and replaces it with a | 14 | // Discard throws away a broken session's persisted state and replaces it with a | ||
| 15 | // fresh incarnation. It is retry-idempotent, not crash-resumable: a crash | 15 | // fresh incarnation. It is retry-idempotent, not crash-resumable: a crash | ||
| 16 | // leaves either the old record (rerun discard) or a new record plus an orphan | 16 | // leaves either the old record (rerun discard) or a new record plus an orphan | ||
| 17 | - | // directory that startup garbage collection removes. | 17 | + | // directory that background garbage collection removes. |
| 18 | func (c *Coordinator) Discard(ctx context.Context, name string) error { | 18 | func (c *Coordinator) Discard(ctx context.Context, name string) error { | ||
| 19 | if c == nil || c.catalogue == nil || c.repository == nil || c.locks == nil || c.random == nil { | 19 | if c == nil || c.catalogue == nil || c.repository == nil || c.locks == nil || c.random == nil { | ||
| 20 | return errors.New("recovery: incomplete discard dependencies") | 20 | return errors.New("recovery: incomplete discard dependencies") | ||
M internal/usecase/recovery/transactions_test.go +177 -58
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "bytes" | |
| 5 | 5 | "context" | |
| 6 | 6 | "errors" | |
| 7 | + | "slices" | |
| 7 | 8 | "sync" | |
| 8 | 9 | "testing" | |
| 9 | 10 | ||
| @@ -113,51 +114,75 @@ type transactionRepository struct { | |||
| 113 | 114 | releaseDelete chan struct{} | |
| 114 | 115 | } | |
| 115 | 116 | ||
| 117 | + | // garbageCollectionFenceRepository models per-incarnation retention. The first | |
| 118 | + | // CollectIncarnationGarbage call blocks until released, so a test can race a | |
| 119 | + | // mutation against a step that already read its keep decision. | |
| 116 | 120 | type garbageCollectionFenceRepository struct { | |
| 117 | 121 | ports.SnapshotRepository | |
| 118 | - | mu sync.Mutex | |
| 119 | - | events []string | |
| 120 | - | collectionEntered chan struct{} | |
| 121 | - | releaseCollection chan struct{} | |
| 122 | - | collectionFinished bool | |
| 123 | - | publishBeforeGC bool | |
| 124 | - | incarnations map[domain.IncarnationID]struct{} | |
| 125 | - | generations map[domain.IncarnationID]map[uint64]struct{} | |
| 126 | - | } | |
| 127 | - | ||
| 128 | - | func (r *garbageCollectionFenceRepository) CollectGarbage(_ context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error { | |
| 129 | - | close(r.collectionEntered) | |
| 130 | - | <-r.releaseCollection | |
| 122 | + | mu sync.Mutex | |
| 123 | + | events []string | |
| 124 | + | collectionEntered chan struct{} | |
| 125 | + | releaseCollection chan struct{} | |
| 126 | + | blocked bool | |
| 127 | + | incarnations map[domain.IncarnationID]struct{} | |
| 128 | + | generations map[domain.IncarnationID]map[uint64]struct{} | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | func (r *garbageCollectionFenceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) { | |
| 131 | 132 | r.mu.Lock() | |
| 132 | 133 | defer r.mu.Unlock() | |
| 134 | + | seen := make(map[domain.IncarnationID]struct{}) | |
| 133 | 135 | for id := range r.incarnations { | |
| 134 | - | if _, known := keep[id]; !known { | |
| 135 | - | delete(r.incarnations, id) | |
| 136 | - | } | |
| 136 | + | seen[id] = struct{}{} | |
| 137 | 137 | } | |
| 138 | - | for id, generations := range r.generations { | |
| 139 | - | committed, known := keep[id] | |
| 140 | - | if !known { | |
| 141 | - | delete(r.generations, id) | |
| 142 | - | continue | |
| 143 | - | } | |
| 144 | - | for generation := range generations { | |
| 145 | - | if generation != committed.Generation && generation+1 != committed.Generation { | |
| 146 | - | delete(generations, generation) | |
| 138 | + | for id := range r.generations { | |
| 139 | + | seen[id] = struct{}{} | |
| 140 | + | } | |
| 141 | + | ids := make([]domain.IncarnationID, 0, len(seen)) | |
| 142 | + | for id := range seen { | |
| 143 | + | ids = append(ids, id) | |
| 144 | + | } | |
| 145 | + | // Deterministic order lets tests pause between known steps. | |
| 146 | + | slices.SortFunc(ids, func(a, b domain.IncarnationID) int { return bytes.Compare(a[:], b[:]) }) | |
| 147 | + | return ids, nil | |
| 148 | + | } | |
| 149 | + | ||
| 150 | + | func (r *garbageCollectionFenceRepository) CollectIncarnationGarbage(_ context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error { | |
| 151 | + | r.mu.Lock() | |
| 152 | + | block := !r.blocked | |
| 153 | + | r.blocked = true | |
| 154 | + | r.mu.Unlock() | |
| 155 | + | if block { | |
| 156 | + | close(r.collectionEntered) | |
| 157 | + | <-r.releaseCollection | |
| 158 | + | } | |
| 159 | + | r.mu.Lock() | |
| 160 | + | defer r.mu.Unlock() | |
| 161 | + | if keep == nil { | |
| 162 | + | delete(r.incarnations, id) | |
| 163 | + | delete(r.generations, id) | |
| 164 | + | } else { | |
| 165 | + | for generation := range r.generations[id] { | |
| 166 | + | if generation != keep.Generation && generation+1 != keep.Generation { | |
| 167 | + | delete(r.generations[id], generation) | |
| 147 | 168 | } | |
| 148 | 169 | } | |
| 149 | 170 | } | |
| 150 | - | r.collectionFinished = true | |
| 151 | 171 | r.events = append(r.events, "collect") | |
| 152 | 172 | return nil | |
| 153 | 173 | } | |
| 154 | 174 | ||
| 175 | + | func (r *garbageCollectionFenceRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error { | |
| 176 | + | r.mu.Lock() | |
| 177 | + | defer r.mu.Unlock() | |
| 178 | + | delete(r.incarnations, id) | |
| 179 | + | delete(r.generations, id) | |
| 180 | + | return nil | |
| 181 | + | } | |
| 182 | + | ||
| 155 | 183 | func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error { | |
| 156 | 184 | r.mu.Lock() | |
| 157 | 185 | defer r.mu.Unlock() | |
| 158 | - | if !r.collectionFinished { | |
| 159 | - | r.publishBeforeGC = true | |
| 160 | - | } | |
| 161 | 186 | if r.generations == nil { | |
| 162 | 187 | r.generations = make(map[domain.IncarnationID]map[uint64]struct{}) | |
| 163 | 188 | } | |
| @@ -192,10 +217,10 @@ func (r *garbageCollectionFenceRepository) hasGeneration(id domain.IncarnationID | |||
| 192 | 217 | return ok | |
| 193 | 218 | } | |
| 194 | 219 | ||
| 195 | - | func (r *garbageCollectionFenceRepository) eventLog() ([]string, bool) { | |
| 220 | + | func (r *garbageCollectionFenceRepository) eventLog() []string { | |
| 196 | 221 | r.mu.Lock() | |
| 197 | 222 | defer r.mu.Unlock() | |
| 198 | - | return append([]string(nil), r.events...), r.publishBeforeGC | |
| 223 | + | return append([]string(nil), r.events...) | |
| 199 | 224 | } | |
| 200 | 225 | ||
| 201 | 226 | func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error { | |
| @@ -381,45 +406,42 @@ func TestResetIncompatibleStaleAuthorityIsNoOp(t *testing.T) { | |||
| 381 | 406 | } | |
| 382 | 407 | } | |
| 383 | 408 | ||
| 384 | - | func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) { | |
| 409 | + | func TestGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) { | |
| 385 | 410 | t.Parallel() | |
| 386 | 411 | ||
| 387 | - | t.Run("a stale keep snapshot cannot remove a newly created incarnation", func(t *testing.T) { | |
| 412 | + | t.Run("a keep decision is re-read under the fence for every incarnation", func(t *testing.T) { | |
| 388 | 413 | t.Parallel() | |
| 414 | + | // The pass lists incarnation 9 (an orphan) and pauses after step 1; | |
| 415 | + | // "new" is then created with ID 9, so step 9 must re-read the catalogue | |
| 416 | + | // and keep it. A keep decision read once per pass would remove it. | |
| 417 | + | reused := domain.IncarnationID{9} | |
| 389 | 418 | catalogue := newTransactionCatalogue() | |
| 390 | 419 | repository := &garbageCollectionFenceRepository{ | |
| 391 | - | collectionEntered: make(chan struct{}), | |
| 392 | - | releaseCollection: make(chan struct{}), | |
| 420 | + | blocked: true, | |
| 421 | + | incarnations: map[domain.IncarnationID]struct{}{{1}: {}, reused: {}}, | |
| 393 | 422 | } | |
| 394 | 423 | catalogue.onCreate = repository.addIncarnation | |
| 395 | - | coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16))) | |
| 424 | + | coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(nil)) | |
| 425 | + | ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})} | |
| 396 | 426 | ||
| 397 | 427 | collected := make(chan error, 1) | |
| 398 | 428 | go func() { | |
| 399 | - | _, err := coordinator.CollectGarbage(context.Background()) | |
| 429 | + | _, err := coordinator.CollectGarbage(ctx) | |
| 400 | 430 | collected <- err | |
| 401 | 431 | }() | |
| 402 | - | <-repository.collectionEntered | |
| 432 | + | <-ctx.reached | |
| 433 | + | require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "step 1 ran first") | |
| 403 | 434 | ||
| 404 | - | created := make(chan error, 1) | |
| 405 | - | go func() { | |
| 406 | - | _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new"}) | |
| 407 | - | created <- err | |
| 408 | - | }() | |
| 409 | - | close(repository.releaseCollection) | |
| 435 | + | _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new", IncarnationID: reused}) | |
| 436 | + | require.NoError(t, err) | |
| 437 | + | close(ctx.release) | |
| 410 | 438 | ||
| 411 | 439 | require.NoError(t, <-collected) | |
| 412 | - | require.NoError(t, <-created) | |
| 413 | - | got, ok, err := catalogue.Record("new") | |
| 414 | - | require.NoError(t, err) | |
| 415 | - | require.True(t, ok) | |
| 416 | - | require.NotZero(t, got.IncarnationID) | |
| 417 | - | require.True(t, repository.hasIncarnation(got.IncarnationID), "GC's stale keep snapshot must finish before the new incarnation is published") | |
| 418 | - | events, _ := repository.eventLog() | |
| 419 | - | require.Equal(t, []string{"collect"}, events) | |
| 440 | + | require.True(t, repository.hasIncarnation(reused), "a step after the creation must keep the new incarnation") | |
| 441 | + | require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "a true orphan is still removed") | |
| 420 | 442 | }) | |
| 421 | 443 | ||
| 422 | - | t.Run("a stale keep snapshot cannot prune a newly committed generation", func(t *testing.T) { | |
| 444 | + | t.Run("a stale keep decision cannot prune a newly committed generation", func(t *testing.T) { | |
| 423 | 445 | t.Parallel() | |
| 424 | 446 | committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}} | |
| 425 | 447 | record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed} | |
| @@ -455,13 +477,110 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications( | |||
| 455 | 477 | ||
| 456 | 478 | require.NoError(t, <-collected) | |
| 457 | 479 | require.NoError(t, <-published) | |
| 458 | - | events, publishedBeforeGC := repository.eventLog() | |
| 459 | - | require.Equal(t, []string{"collect", "publish"}, events) | |
| 460 | - | require.False(t, publishedBeforeGC) | |
| 461 | - | require.True(t, repository.hasGeneration(record.IncarnationID, 2), "GC's stale keep snapshot must finish before generation two is published") | |
| 480 | + | require.Equal(t, []string{"collect", "publish"}, repository.eventLog()) | |
| 481 | + | require.True(t, repository.hasGeneration(record.IncarnationID, 2), "the fenced step must finish before generation two is published") | |
| 462 | 482 | }) | |
| 463 | 483 | } | |
| 464 | 484 | ||
| 485 | + | // betweenStepsContext pauses a GC pass at its first cancellation check after | |
| 486 | + | // one incarnation step has completed, outside any fenced step. | |
| 487 | + | type betweenStepsContext struct { | |
| 488 | + | context.Context | |
| 489 | + | repository *garbageCollectionFenceRepository | |
| 490 | + | once sync.Once | |
| 491 | + | reached chan struct{} | |
| 492 | + | release chan struct{} | |
| 493 | + | } | |
| 494 | + | ||
| 495 | + | func (c *betweenStepsContext) Err() error { | |
| 496 | + | if len(c.repository.eventLog()) == 1 { | |
| 497 | + | c.once.Do(func() { | |
| 498 | + | close(c.reached) | |
| 499 | + | <-c.release | |
| 500 | + | }) | |
| 501 | + | } | |
| 502 | + | return c.Context.Err() | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | // TestDurableMutationWaitsForAtMostOneIncarnation proves the fence is released | |
| 506 | + | // between incarnation steps, so every mutation entry point can run while a | |
| 507 | + | // pass is still in progress instead of waiting for the whole pass. | |
| 508 | + | func TestDurableMutationWaitsForAtMostOneIncarnation(t *testing.T) { | |
| 509 | + | t.Parallel() | |
| 510 | + | committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}} | |
| 511 | + | work := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed} | |
| 512 | + | broken := degradedTransactionRecord() | |
| 513 | + | broken.IncarnationID = domain.IncarnationID{2} | |
| 514 | + | tests := []struct { | |
| 515 | + | name string | |
| 516 | + | mutate func(*Coordinator) error | |
| 517 | + | }{ | |
| 518 | + | {name: "create", mutate: func(c *Coordinator) error { | |
| 519 | + | _, err := c.Create(context.Background(), domain.CatalogueRecord{Name: "new"}) | |
| 520 | + | return err | |
| 521 | + | }}, | |
| 522 | + | {name: "rename", mutate: func(c *Coordinator) error { | |
| 523 | + | _, err := c.Rename(context.Background(), work.Name, "renamed") | |
| 524 | + | return err | |
| 525 | + | }}, | |
| 526 | + | {name: "delete", mutate: func(c *Coordinator) error { return c.Delete(context.Background(), work.Name) }}, | |
| 527 | + | {name: "exact delete", mutate: func(c *Coordinator) error { | |
| 528 | + | return c.DeleteExact(context.Background(), work.Name, work.IncarnationID, work.CreatedAt) | |
| 529 | + | }}, | |
| 530 | + | {name: "mark broken", mutate: func(c *Coordinator) error { | |
| 531 | + | _, _, err := c.MarkBroken(context.Background(), work.Name, work.IncarnationID, "restore failed") | |
| 532 | + | return err | |
| 533 | + | }}, | |
| 534 | + | {name: "discard", mutate: func(c *Coordinator) error { return c.Discard(context.Background(), broken.Name) }}, | |
| 535 | + | {name: "reset incompatible", mutate: func(c *Coordinator) error { | |
| 536 | + | _, _, err := c.ResetIncompatible(context.Background(), work.Name, work.IncarnationID, committed) | |
| 537 | + | return err | |
| 538 | + | }}, | |
| 539 | + | {name: "checkpoint publication", mutate: func(c *Coordinator) error { | |
| 540 | + | _, err := c.PublishCheckpoint(context.Background(), work.Name, ports.SnapshotPublication{ | |
| 541 | + | Name: work.Name, IncarnationID: work.IncarnationID, Generation: 2, | |
| 542 | + | ParentCheckpoint: &committed, Manifest: []byte("generation two"), | |
| 543 | + | }) | |
| 544 | + | return err | |
| 545 | + | }}, | |
| 546 | + | } | |
| 547 | + | for _, tt := range tests { | |
| 548 | + | t.Run(tt.name, func(t *testing.T) { | |
| 549 | + | t.Parallel() | |
| 550 | + | // Two orphan incarnations give the pass two fenced steps. | |
| 551 | + | repository := &garbageCollectionFenceRepository{ | |
| 552 | + | blocked: true, | |
| 553 | + | incarnations: map[domain.IncarnationID]struct{}{{7}: {}, {8}: {}}, | |
| 554 | + | } | |
| 555 | + | coordinator := NewCoordinator(newTransactionCatalogue(work, broken), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 32))) | |
| 556 | + | ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})} | |
| 557 | + | ||
| 558 | + | collected := make(chan error, 1) | |
| 559 | + | go func() { | |
| 560 | + | _, err := coordinator.CollectGarbage(ctx) | |
| 561 | + | collected <- err | |
| 562 | + | }() | |
| 563 | + | <-ctx.reached | |
| 564 | + | ||
| 565 | + | // TryLock first so a regression that holds the fence across the pass | |
| 566 | + | // fails here instead of deadlocking the mutation. | |
| 567 | + | require.True(t, coordinator.mutationMu.TryLock(), "the fence must be free between incarnation steps") | |
| 568 | + | coordinator.mutationMu.Unlock() | |
| 569 | + | require.NoError(t, tt.mutate(coordinator)) | |
| 570 | + | ||
| 571 | + | close(ctx.release) | |
| 572 | + | require.NoError(t, <-collected) | |
| 573 | + | steps := 0 | |
| 574 | + | for _, event := range repository.eventLog() { | |
| 575 | + | if event == "collect" { | |
| 576 | + | steps++ | |
| 577 | + | } | |
| 578 | + | } | |
| 579 | + | require.Equal(t, 2, steps, "both incarnation steps ran") | |
| 580 | + | }) | |
| 581 | + | } | |
| 582 | + | } | |
| 583 | + | ||
| 465 | 584 | func TestMarkBrokenCannotOverwriteDiscardReplacement(t *testing.T) { | |
| 466 | 585 | t.Parallel() | |
| 467 | 586 | old := degradedTransactionRecord() | |
| @@ -597,7 +716,7 @@ func TestDiscardIsRetryIdempotent(t *testing.T) { | |||
| 597 | 716 | if tt.failBefore { | |
| 598 | 717 | require.Contains(t, repository.deletedIDs, old.IncarnationID) | |
| 599 | 718 | } else { | |
| 600 | - | require.Empty(t, repository.deletedIDs, "startup garbage collection owns the orphaned old directory") | |
| 719 | + | require.Empty(t, repository.deletedIDs, "background garbage collection owns the orphaned old directory") | |
| 601 | 720 | } | |
| 602 | 721 | }) | |
| 603 | 722 | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | + | "slices" | |||
| 7 | "sync" | 8 | "sync" | ||
| 8 | "testing" | 9 | "testing" | ||
| 9 | 10 | ||||
| @@ -113,51 +114,75 @@ type transactionRepository struct { | |||||
| 113 | releaseDelete chan struct{} | 114 | releaseDelete chan struct{} | ||
| 114 | } | 115 | } | ||
| 115 | 116 | ||||
| 117 | + | // garbageCollectionFenceRepository models per-incarnation retention. The first | |||
| 118 | + | // CollectIncarnationGarbage call blocks until released, so a test can race a | |||
| 119 | + | // mutation against a step that already read its keep decision. | |||
| 116 | type garbageCollectionFenceRepository struct { | 120 | type garbageCollectionFenceRepository struct { | ||
| 117 | ports.SnapshotRepository | 121 | ports.SnapshotRepository | ||
| 118 | - | mu sync.Mutex | 122 | + | mu sync.Mutex |
| 119 | - | events []string | 123 | + | events []string |
| 120 | - | collectionEntered chan struct{} | 124 | + | collectionEntered chan struct{} |
| 121 | - | releaseCollection chan struct{} | 125 | + | releaseCollection chan struct{} |
| 122 | - | collectionFinished bool | 126 | + | blocked bool |
| 123 | - | publishBeforeGC bool | 127 | + | incarnations map[domain.IncarnationID]struct{} |
| 124 | - | incarnations map[domain.IncarnationID]struct{} | 128 | + | generations map[domain.IncarnationID]map[uint64]struct{} |
| 125 | - | generations map[domain.IncarnationID]map[uint64]struct{} | 129 | + | } |
| 126 | - | } | 130 | + | |
| 127 | - | 131 | + | func (r *garbageCollectionFenceRepository) SnapshotIncarnations(context.Context) ([]domain.IncarnationID, error) { | |
| 128 | - | func (r *garbageCollectionFenceRepository) CollectGarbage(_ context.Context, keep map[domain.IncarnationID]domain.CheckpointRef) error { | |||
| 129 | - | close(r.collectionEntered) | |||
| 130 | - | <-r.releaseCollection | |||
| 131 | r.mu.Lock() | 132 | r.mu.Lock() | ||
| 132 | defer r.mu.Unlock() | 133 | defer r.mu.Unlock() | ||
| 134 | + | seen := make(map[domain.IncarnationID]struct{}) | |||
| 133 | for id := range r.incarnations { | 135 | for id := range r.incarnations { | ||
| 134 | - | if _, known := keep[id]; !known { | 136 | + | seen[id] = struct{}{} |
| 135 | - | delete(r.incarnations, id) | |||
| 136 | - | } | |||
| 137 | } | 137 | } | ||
| 138 | - | for id, generations := range r.generations { | 138 | + | for id := range r.generations { |
| 139 | - | committed, known := keep[id] | 139 | + | seen[id] = struct{}{} |
| 140 | - | if !known { | 140 | + | } |
| 141 | - | delete(r.generations, id) | 141 | + | ids := make([]domain.IncarnationID, 0, len(seen)) |
| 142 | - | continue | 142 | + | for id := range seen { |
| 143 | - | } | 143 | + | ids = append(ids, id) |
| 144 | - | for generation := range generations { | 144 | + | } |
| 145 | - | if generation != committed.Generation && generation+1 != committed.Generation { | 145 | + | // Deterministic order lets tests pause between known steps. |
| 146 | - | delete(generations, generation) | 146 | + | slices.SortFunc(ids, func(a, b domain.IncarnationID) int { return bytes.Compare(a[:], b[:]) }) |
| 147 | + | return ids, nil | |||
| 148 | + | } | |||
| 149 | + | ||||
| 150 | + | func (r *garbageCollectionFenceRepository) CollectIncarnationGarbage(_ context.Context, id domain.IncarnationID, keep *domain.CheckpointRef) error { | |||
| 151 | + | r.mu.Lock() | |||
| 152 | + | block := !r.blocked | |||
| 153 | + | r.blocked = true | |||
| 154 | + | r.mu.Unlock() | |||
| 155 | + | if block { | |||
| 156 | + | close(r.collectionEntered) | |||
| 157 | + | <-r.releaseCollection | |||
| 158 | + | } | |||
| 159 | + | r.mu.Lock() | |||
| 160 | + | defer r.mu.Unlock() | |||
| 161 | + | if keep == nil { | |||
| 162 | + | delete(r.incarnations, id) | |||
| 163 | + | delete(r.generations, id) | |||
| 164 | + | } else { | |||
| 165 | + | for generation := range r.generations[id] { | |||
| 166 | + | if generation != keep.Generation && generation+1 != keep.Generation { | |||
| 167 | + | delete(r.generations[id], generation) | |||
| 147 | } | 168 | } | ||
| 148 | } | 169 | } | ||
| 149 | } | 170 | } | ||
| 150 | - | r.collectionFinished = true | |||
| 151 | r.events = append(r.events, "collect") | 171 | r.events = append(r.events, "collect") | ||
| 152 | return nil | 172 | return nil | ||
| 153 | } | 173 | } | ||
| 154 | 174 | ||||
| 175 | + | func (r *garbageCollectionFenceRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error { | |||
| 176 | + | r.mu.Lock() | |||
| 177 | + | defer r.mu.Unlock() | |||
| 178 | + | delete(r.incarnations, id) | |||
| 179 | + | delete(r.generations, id) | |||
| 180 | + | return nil | |||
| 181 | + | } | |||
| 182 | + | ||||
| 155 | func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error { | 183 | func (r *garbageCollectionFenceRepository) Publish(_ context.Context, publication ports.SnapshotPublication) error { | ||
| 156 | r.mu.Lock() | 184 | r.mu.Lock() | ||
| 157 | defer r.mu.Unlock() | 185 | defer r.mu.Unlock() | ||
| 158 | - | if !r.collectionFinished { | |||
| 159 | - | r.publishBeforeGC = true | |||
| 160 | - | } | |||
| 161 | if r.generations == nil { | 186 | if r.generations == nil { | ||
| 162 | r.generations = make(map[domain.IncarnationID]map[uint64]struct{}) | 187 | r.generations = make(map[domain.IncarnationID]map[uint64]struct{}) | ||
| 163 | } | 188 | } | ||
| @@ -192,10 +217,10 @@ func (r *garbageCollectionFenceRepository) hasGeneration(id domain.IncarnationID | |||||
| 192 | return ok | 217 | return ok | ||
| 193 | } | 218 | } | ||
| 194 | 219 | ||||
| 195 | - | func (r *garbageCollectionFenceRepository) eventLog() ([]string, bool) { | 220 | + | func (r *garbageCollectionFenceRepository) eventLog() []string { |
| 196 | r.mu.Lock() | 221 | r.mu.Lock() | ||
| 197 | defer r.mu.Unlock() | 222 | defer r.mu.Unlock() | ||
| 198 | - | return append([]string(nil), r.events...), r.publishBeforeGC | 223 | + | return append([]string(nil), r.events...) |
| 199 | } | 224 | } | ||
| 200 | 225 | ||||
| 201 | func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error { | 226 | func (r *transactionRepository) DeleteIncarnation(_ context.Context, id domain.IncarnationID) error { | ||
| @@ -381,45 +406,42 @@ func TestResetIncompatibleStaleAuthorityIsNoOp(t *testing.T) { | |||||
| 381 | } | 406 | } | ||
| 382 | } | 407 | } | ||
| 383 | 408 | ||||
| 384 | - | func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) { | 409 | + | func TestGarbageCollectionFencesNewIncarnationsAndCheckpointPublications(t *testing.T) { |
| 385 | t.Parallel() | 410 | t.Parallel() | ||
| 386 | 411 | ||||
| 387 | - | t.Run("a stale keep snapshot cannot remove a newly created incarnation", func(t *testing.T) { | 412 | + | t.Run("a keep decision is re-read under the fence for every incarnation", func(t *testing.T) { |
| 388 | t.Parallel() | 413 | t.Parallel() | ||
| 414 | + | // The pass lists incarnation 9 (an orphan) and pauses after step 1; | |||
| 415 | + | // "new" is then created with ID 9, so step 9 must re-read the catalogue | |||
| 416 | + | // and keep it. A keep decision read once per pass would remove it. | |||
| 417 | + | reused := domain.IncarnationID{9} | |||
| 389 | catalogue := newTransactionCatalogue() | 418 | catalogue := newTransactionCatalogue() | ||
| 390 | repository := &garbageCollectionFenceRepository{ | 419 | repository := &garbageCollectionFenceRepository{ | ||
| 391 | - | collectionEntered: make(chan struct{}), | 420 | + | blocked: true, |
| 392 | - | releaseCollection: make(chan struct{}), | 421 | + | incarnations: map[domain.IncarnationID]struct{}{{1}: {}, reused: {}}, |
| 393 | } | 422 | } | ||
| 394 | catalogue.onCreate = repository.addIncarnation | 423 | catalogue.onCreate = repository.addIncarnation | ||
| 395 | - | coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(bytes.Repeat([]byte{2}, 16))) | 424 | + | coordinator := NewCoordinator(catalogue, repository, bytes.NewReader(nil)) |
| 425 | + | ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})} | |||
| 396 | 426 | ||||
| 397 | collected := make(chan error, 1) | 427 | collected := make(chan error, 1) | ||
| 398 | go func() { | 428 | go func() { | ||
| 399 | - | _, err := coordinator.CollectGarbage(context.Background()) | 429 | + | _, err := coordinator.CollectGarbage(ctx) |
| 400 | collected <- err | 430 | collected <- err | ||
| 401 | }() | 431 | }() | ||
| 402 | - | <-repository.collectionEntered | 432 | + | <-ctx.reached |
| 433 | + | require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "step 1 ran first") | |||
| 403 | 434 | ||||
| 404 | - | created := make(chan error, 1) | 435 | + | _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new", IncarnationID: reused}) |
| 405 | - | go func() { | 436 | + | require.NoError(t, err) |
| 406 | - | _, err := coordinator.Create(context.Background(), domain.CatalogueRecord{Name: "new"}) | 437 | + | close(ctx.release) |
| 407 | - | created <- err | |||
| 408 | - | }() | |||
| 409 | - | close(repository.releaseCollection) | |||
| 410 | 438 | ||||
| 411 | require.NoError(t, <-collected) | 439 | require.NoError(t, <-collected) | ||
| 412 | - | require.NoError(t, <-created) | 440 | + | require.True(t, repository.hasIncarnation(reused), "a step after the creation must keep the new incarnation") |
| 413 | - | got, ok, err := catalogue.Record("new") | 441 | + | require.False(t, repository.hasIncarnation(domain.IncarnationID{1}), "a true orphan is still removed") |
| 414 | - | require.NoError(t, err) | |||
| 415 | - | require.True(t, ok) | |||
| 416 | - | require.NotZero(t, got.IncarnationID) | |||
| 417 | - | require.True(t, repository.hasIncarnation(got.IncarnationID), "GC's stale keep snapshot must finish before the new incarnation is published") | |||
| 418 | - | events, _ := repository.eventLog() | |||
| 419 | - | require.Equal(t, []string{"collect"}, events) | |||
| 420 | }) | 442 | }) | ||
| 421 | 443 | ||||
| 422 | - | t.Run("a stale keep snapshot cannot prune a newly committed generation", func(t *testing.T) { | 444 | + | t.Run("a stale keep decision cannot prune a newly committed generation", func(t *testing.T) { |
| 423 | t.Parallel() | 445 | t.Parallel() | ||
| 424 | committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}} | 446 | committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}} | ||
| 425 | record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed} | 447 | record := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed} | ||
| @@ -455,13 +477,110 @@ func TestStartupGarbageCollectionFencesNewIncarnationsAndCheckpointPublications( | |||||
| 455 | 477 | ||||
| 456 | require.NoError(t, <-collected) | 478 | require.NoError(t, <-collected) | ||
| 457 | require.NoError(t, <-published) | 479 | require.NoError(t, <-published) | ||
| 458 | - | events, publishedBeforeGC := repository.eventLog() | 480 | + | require.Equal(t, []string{"collect", "publish"}, repository.eventLog()) |
| 459 | - | require.Equal(t, []string{"collect", "publish"}, events) | 481 | + | require.True(t, repository.hasGeneration(record.IncarnationID, 2), "the fenced step must finish before generation two is published") |
| 460 | - | require.False(t, publishedBeforeGC) | |||
| 461 | - | require.True(t, repository.hasGeneration(record.IncarnationID, 2), "GC's stale keep snapshot must finish before generation two is published") | |||
| 462 | }) | 482 | }) | ||
| 463 | } | 483 | } | ||
| 464 | 484 | ||||
| 485 | + | // betweenStepsContext pauses a GC pass at its first cancellation check after | |||
| 486 | + | // one incarnation step has completed, outside any fenced step. | |||
| 487 | + | type betweenStepsContext struct { | |||
| 488 | + | context.Context | |||
| 489 | + | repository *garbageCollectionFenceRepository | |||
| 490 | + | once sync.Once | |||
| 491 | + | reached chan struct{} | |||
| 492 | + | release chan struct{} | |||
| 493 | + | } | |||
| 494 | + | ||||
| 495 | + | func (c *betweenStepsContext) Err() error { | |||
| 496 | + | if len(c.repository.eventLog()) == 1 { | |||
| 497 | + | c.once.Do(func() { | |||
| 498 | + | close(c.reached) | |||
| 499 | + | <-c.release | |||
| 500 | + | }) | |||
| 501 | + | } | |||
| 502 | + | return c.Context.Err() | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | // TestDurableMutationWaitsForAtMostOneIncarnation proves the fence is released | |||
| 506 | + | // between incarnation steps, so every mutation entry point can run while a | |||
| 507 | + | // pass is still in progress instead of waiting for the whole pass. | |||
| 508 | + | func TestDurableMutationWaitsForAtMostOneIncarnation(t *testing.T) { | |||
| 509 | + | t.Parallel() | |||
| 510 | + | committed := domain.CheckpointRef{Generation: 1, ManifestDigest: [32]byte{1}} | |||
| 511 | + | work := domain.CatalogueRecord{Name: "work", IncarnationID: domain.IncarnationID{1}, Committed: &committed} | |||
| 512 | + | broken := degradedTransactionRecord() | |||
| 513 | + | broken.IncarnationID = domain.IncarnationID{2} | |||
| 514 | + | tests := []struct { | |||
| 515 | + | name string | |||
| 516 | + | mutate func(*Coordinator) error | |||
| 517 | + | }{ | |||
| 518 | + | {name: "create", mutate: func(c *Coordinator) error { | |||
| 519 | + | _, err := c.Create(context.Background(), domain.CatalogueRecord{Name: "new"}) | |||
| 520 | + | return err | |||
| 521 | + | }}, | |||
| 522 | + | {name: "rename", mutate: func(c *Coordinator) error { | |||
| 523 | + | _, err := c.Rename(context.Background(), work.Name, "renamed") | |||
| 524 | + | return err | |||
| 525 | + | }}, | |||
| 526 | + | {name: "delete", mutate: func(c *Coordinator) error { return c.Delete(context.Background(), work.Name) }}, | |||
| 527 | + | {name: "exact delete", mutate: func(c *Coordinator) error { | |||
| 528 | + | return c.DeleteExact(context.Background(), work.Name, work.IncarnationID, work.CreatedAt) | |||
| 529 | + | }}, | |||
| 530 | + | {name: "mark broken", mutate: func(c *Coordinator) error { | |||
| 531 | + | _, _, err := c.MarkBroken(context.Background(), work.Name, work.IncarnationID, "restore failed") | |||
| 532 | + | return err | |||
| 533 | + | }}, | |||
| 534 | + | {name: "discard", mutate: func(c *Coordinator) error { return c.Discard(context.Background(), broken.Name) }}, | |||
| 535 | + | {name: "reset incompatible", mutate: func(c *Coordinator) error { | |||
| 536 | + | _, _, err := c.ResetIncompatible(context.Background(), work.Name, work.IncarnationID, committed) | |||
| 537 | + | return err | |||
| 538 | + | }}, | |||
| 539 | + | {name: "checkpoint publication", mutate: func(c *Coordinator) error { | |||
| 540 | + | _, err := c.PublishCheckpoint(context.Background(), work.Name, ports.SnapshotPublication{ | |||
| 541 | + | Name: work.Name, IncarnationID: work.IncarnationID, Generation: 2, | |||
| 542 | + | ParentCheckpoint: &committed, Manifest: []byte("generation two"), | |||
| 543 | + | }) | |||
| 544 | + | return err | |||
| 545 | + | }}, | |||
| 546 | + | } | |||
| 547 | + | for _, tt := range tests { | |||
| 548 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 549 | + | t.Parallel() | |||
| 550 | + | // Two orphan incarnations give the pass two fenced steps. | |||
| 551 | + | repository := &garbageCollectionFenceRepository{ | |||
| 552 | + | blocked: true, | |||
| 553 | + | incarnations: map[domain.IncarnationID]struct{}{{7}: {}, {8}: {}}, | |||
| 554 | + | } | |||
| 555 | + | coordinator := NewCoordinator(newTransactionCatalogue(work, broken), repository, bytes.NewReader(bytes.Repeat([]byte{2}, 32))) | |||
| 556 | + | ctx := &betweenStepsContext{Context: context.Background(), repository: repository, reached: make(chan struct{}), release: make(chan struct{})} | |||
| 557 | + | ||||
| 558 | + | collected := make(chan error, 1) | |||
| 559 | + | go func() { | |||
| 560 | + | _, err := coordinator.CollectGarbage(ctx) | |||
| 561 | + | collected <- err | |||
| 562 | + | }() | |||
| 563 | + | <-ctx.reached | |||
| 564 | + | ||||
| 565 | + | // TryLock first so a regression that holds the fence across the pass | |||
| 566 | + | // fails here instead of deadlocking the mutation. | |||
| 567 | + | require.True(t, coordinator.mutationMu.TryLock(), "the fence must be free between incarnation steps") | |||
| 568 | + | coordinator.mutationMu.Unlock() | |||
| 569 | + | require.NoError(t, tt.mutate(coordinator)) | |||
| 570 | + | ||||
| 571 | + | close(ctx.release) | |||
| 572 | + | require.NoError(t, <-collected) | |||
| 573 | + | steps := 0 | |||
| 574 | + | for _, event := range repository.eventLog() { | |||
| 575 | + | if event == "collect" { | |||
| 576 | + | steps++ | |||
| 577 | + | } | |||
| 578 | + | } | |||
| 579 | + | require.Equal(t, 2, steps, "both incarnation steps ran") | |||
| 580 | + | }) | |||
| 581 | + | } | |||
| 582 | + | } | |||
| 583 | + | ||||
| 465 | func TestMarkBrokenCannotOverwriteDiscardReplacement(t *testing.T) { | 584 | func TestMarkBrokenCannotOverwriteDiscardReplacement(t *testing.T) { | ||
| 466 | t.Parallel() | 585 | t.Parallel() | ||
| 467 | old := degradedTransactionRecord() | 586 | old := degradedTransactionRecord() | ||
| @@ -597,7 +716,7 @@ func TestDiscardIsRetryIdempotent(t *testing.T) { | |||||
| 597 | if tt.failBefore { | 716 | if tt.failBefore { | ||
| 598 | require.Contains(t, repository.deletedIDs, old.IncarnationID) | 717 | require.Contains(t, repository.deletedIDs, old.IncarnationID) | ||
| 599 | } else { | 718 | } else { | ||
| 600 | - | require.Empty(t, repository.deletedIDs, "startup garbage collection owns the orphaned old directory") | 719 | + | require.Empty(t, repository.deletedIDs, "background garbage collection owns the orphaned old directory") |
| 601 | } | 720 | } | ||
| 602 | }) | 721 | }) | ||
| 603 | } | 722 | } | ||