GC unreferenced declared screenshot captures via the manifest - #6055
Open
lukemelia wants to merge 1 commit into
Open
GC unreferenced declared screenshot captures via the manifest#6055lukemelia wants to merge 1 commit into
lukemelia wants to merge 1 commit into
Conversation
A declared-lane ledger row was only reclaimable when superseded (a newer generation under the same capture identity) or when its source was tombstoned. A slot that is renamed, deleted, or re-specced mints a new specHash, so its old row met neither arm and leaked its object permanently — and authors iterating on declarations mint these constantly. The sweep gains an 'unreferenced' arm: a declared-lane row whose spec hash no live prerendered_html row's screenshots manifest names is collectible. The manifest is the correct liveness set — ?name= serving is pinned to it, carry-forwards keep their hash in it across generations, and error renders preserve the last-known-good manifest — so a row it doesn't reference is unreachable by any serve. Two guards keep the arm conservative: a live prerendered row must exist (a source mid-first-index has none, and absence must not read as an empty roster), and every live row's rendered_at must be older than min-age, so a manifest published moments ago (mid-iteration churn, a capture failure the retry lane is still working) collects nothing until the state has held for a full window. Realm-copied rows answer to their own realm's manifest, which the copy keeps consistent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The new GC arm is implemented conservatively, is factored to avoid CASE/WHERE drift, and is covered by focused new Postgres-backed tests that validate the key edge cases described in the PR.
Pull request overview
This PR updates the MediaCache GC logic to reclaim “declared” screenshot ledger rows that are no longer reachable because their capture spec hash is no longer referenced by any live prerendered_html.screenshots manifest, closing a long-lived leak when slots are renamed/deleted/re-specced.
Changes:
- Add a new GC reason (
'unreferenced') and a Postgres-only SQL predicate that treats thescreenshotsmanifest as the liveness set for declared captures, with conservatism guards (requires a live prerendered row; requires manifest stability past min-age). - Extend the GC test suite with new Postgres-seeded scenarios covering unreferenced reclamation, stability guard behavior, carry-forward protection, realm scoping, and on-demand immunity.
File summaries
| File | Description |
|---|---|
| packages/runtime-common/tasks/media-cache-gc.ts | Updates task-level GC documentation to include the new “unreferenced” declared-slot reclamation arm. |
| packages/runtime-common/media-cache.ts | Adds 'unreferenced' to GC reasons and implements the manifest-based unreferenced candidate predicate used in both CASE and WHERE. |
| packages/realm-server/tests/media-cache-gc-test.ts | Adds targeted tests that exercise the new manifest-based reclamation behavior and its conservatism guards. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 this does
Closes the declared-lane leak in the MediaCache GC (CS-12728): a slot that is renamed, deleted, or re-specced mints a new
specHash, so its old ledger row was neversuperseded(no newer row shares its identity), nevertombstoned(the source lives), and had no expiry arm — the row and its object leaked permanently.findMediaCacheGcCandidatesgains anunreferencedarm: a declared-lane row whose spec hash no liveprerendered_htmlrow'sscreenshotsmanifest names is collectible. The manifest is the right liveness set:?name=serving is pinned to the manifest, so a row it doesn't reference is already unreachable by any serve;keyBy: 'file-content') keep their spec hash in the manifest across generations — which is exactly what protects their older-generation rows from this arm;Two guards keep the arm conservative:
rendered_atmust be older than GC min-age, so a manifest published moments ago — an author mid-iteration, a transient capture failure the retry lane is still working — collects nothing until the state has held for a full window.Realm-copied rows answer to their own realm's manifest (the copy duplicates both together, keeping them consistent). The predicate lives in one helper used by both the reason
CASEand theWHERE, mirroring the tombstone arm's no-drift pattern. The sweep's SQL is Postgres-only as before; this addsjsonb_eachover the manifest, with ajsonb_typeofguard so a malformed manifest can't error the whole sweep.Test plan
Seven new cases in the GC suite (real Postgres, direct row seeding): re-specced slot reclaimed while the referenced one survives; manifest-less live row reclaims all declared captures; a freshly published manifest collects nothing until min-age; carry-forward protection across generations; no-prerendered-row conservatism; per-realm manifest scoping for realm copies; and the on-demand lane's immunity to the arm. All 15 pre-existing GC/putMedia tests pass unchanged.
🤖 Generated with Claude Code