Repository navigation
feat(coordinator): reap orphaned base snapshots (image-refresh storage leak) - #439
Merged
Merged
Conversation
…e leak) Each enabled-image re-bake/refresh captures a fresh per-image base snapshot and swaps `enabled_images.base_snapshot_id` to it (`upsert_enabled_image`'s ON CONFLICT), leaving the PRIOR base row dangling: `session_id IS NULL`, referenced by no `enabled_images` row. Nothing deleted it — `prune_session_snapshots` (checkpoint retention) is `session_id IS NOT NULL` only — and an orphan base keeps pinning its own disk+memory chunks via pin-set sources #3/#4 (`list_recoverable_snapshot_{disk,memory}_manifests`, which filter on `recoverable = TRUE` with no session predicate). So every image refresh permanently leaked one base snapshot's chunks (20-32 GB for the heavy dogfood images). Prod had 23 such orphans (~150 GB logical) going back 9 days, none in any GC queue. Add `prune_orphan_base_snapshots`, the `session_id IS NULL` mirror of `prune_session_snapshots`: delete base rows older than `ENGRAM_BASE_SNAPSHOT_RETENTION_HOURS` (default 24) that are referenced by no `enabled_images.base_snapshot_id` (live OR soft-deleted — soft-deleted lineage is intentionally still chunk-pinned, ADR 0021 P1.8), bumping `chunk_generation` in the same TX (GC-barrier symmetry). The existing chunk-GC (ADR 0016 Phase C) and snapshot-blob-GC (ADR 0028 addendum) sweeps then reclaim the now-unpinned chunks and portable `snapshots/<id>/` blobs — the reaper deletes nothing in BlobStorage directly. The `base_snapshot_id` FK (REFERENCES snapshots(id), no ON DELETE) is a hard backstop against ever deleting an in-use base. Spawned beside `checkpoint_retention`. A sweeper (not an inline delete-old-base in RefreshImage) so it both drains the existing backlog and survives restarts, per the "scanner drives transitions" convention. Also fixes the now-stale `snapshot_blob_pin_set` doc that claimed base rows are never deleted. Test (live-PG, wired into the existing Postgres-gated CI lane): a superseded orphan past grace is reaped; the current (enabled-image) base and a fresh orphan within grace are kept; session snapshots are untouched. The sibling checkpoint-retention test's template is pinned recent so the new global reaper can't collect it under local parallel runs (CI serializes the lane). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a coordinator sweeper that reaps orphaned per-image base snapshots — the storage leak behind the Jun-23/24 jump in GCS live-object bytes.
Why
Every enabled-image re-bake/
RefreshImagecaptures a fresh base snapshot and swapsenabled_images.base_snapshot_idto it (upsert_enabled_image'sON CONFLICT). The prior base row is left dangling:session_id IS NULL, pointed to by noenabled_imagesrow. Nothing deleted it:prune_session_snapshots(checkpoint retention) issession_id IS NOT NULLonly.list_recoverable_snapshot_{disk,memory}_manifests—recoverable = TRUE, no session predicate).So each refresh permanently leaked one base snapshot's chunks — 20–32 GB for the heavy dogfood images (
dev-brain,dev-engrams). Prod currently holds 23 orphaned bases (~150 GB logical) dating back 9 days, none in any GC queue.How
prune_orphan_base_snapshots— thesession_id IS NULLmirror ofprune_session_snapshots:enabled_imagesrow, including soft-deleted (their chunk lineage is intentionally still pinned, ADR 0021 P1.8).chunk_generationin the same TX (GC-barrier symmetry). The existing chunk-GC (ADR 0016 Phase C) and snapshot-blob-GC (ADR 0028 addendum) sweeps then reclaim the now-unpinned chunks and portablesnapshots/<id>/blobs — this reaper deletes nothing in BlobStorage directly.base_snapshot_idFK (REFERENCES snapshots(id), noON DELETE) is a hard backstop against deleting an in-use base even if the predicate regressed.checkpoint_retention; grace viaENGRAM_BASE_SNAPSHOT_RETENTION_HOURS(default 24).A sweeper (not an inline delete in
RefreshImage) so it both drains the existing 23-orphan backlog and prevents future leaks, per the "scanner drives transitions" convention.Test
Live-PG test wired into the existing Postgres-gated CI lane (
checkpoint_reconcile_live_pg): a superseded orphan past grace is reaped; the current enabled-image base and a fresh orphan within grace are kept; session snapshots are untouched. Validated against a real Postgres locally (just checkgreen: fmt + clippy-D warnings+ 1270 workspace tests).Also fixes the now-stale
snapshot_blob_pin_setdoc comment that asserted base rows are never deleted.Scope notes
session_id IS NOT NULL).checkpoint_retention(its closest sibling); the 24h sweeper drains the backlog automatically. For immediate prod reclaim we can run the equivalent one-shot SQL out of band.Roll
Coord-only; auto-rolls on merge. The 23 existing orphans age out within one grace window (24h) after deploy.
🤖 Generated with Claude Code