The run gave the decisive fact: `derived=1`. A content_derived_blobs row
still points at the leftover blob, so GC is CORRECT to keep it — the
leak is the row, not the bytes. purge_derived_blobs only runs when the
SOURCE is reaped, so the question is why that never happened.
So the dump now prints the source hash and what still holds it:
src_files, src_manifest, src_blob. If the source has a live file the
answer is "not deleted"; if it has none but a positive refcount, a
release was missed upstream; if it has no row at all, the source was
reaped WITHOUT purging, which would be a real ordering bug in reap_blob.
Also fixes the dump reporting only one of three blobs. `docker compose
exec -T` reads stdin, so it consumed the rest of the here-string feeding
the loop — the other two were never queried and vanished silently. The
same silent-truncation shape the diagnosis exists to expose, in the
diagnosis. `< /dev/null` closes it.
52c31a68 added a per-leftover refcount dump to storage_cleanup_check.sh
but referenced COMPOSE_FILE, which that script never defines — only
thumb_import_check.sh does. It would have run `docker compose -f ""`,
failed, and been swallowed by the `|| true` guarding the loop.
A silent no-op: the diagnosis would print nothing and the failure would
look exactly as uninformative as the one it was written to explain. The
same shape as the three bugs this suite has already caught — an error
dressed up as an unremarkable result — and I wrote it into the tool
meant to find them.
The `|| true` stays, so one unreadable blob cannot abort the loop before
the others report.
Three blobs survived the sweep. Five seconds of async-unlink polling did
not remove them, so they were never queued — GC had not judged them
collectible, and the loop had already exited.
It broke on the FIRST zero-reap pass. A single zero only says nothing
was collectible at that instant: releases cascade, since reaping a
source drops the references its derived and attached rows held and
`on_blob_deleted` does that from spawned tasks, so a pass can land in
the gap between "source reaped" and "dependents released" and report
zero with work outstanding. The import jobs added a level to that chain,
which is when it started biting. Now two consecutive zeros, with the
bound raised to match — one extra trigger over an empty store is
cheaper than a false pass reporting a clean disk.
The rest is diagnosis, because a list of paths cannot tell the three
causes apart and they need opposite fixes: a positive refcount means a
release was missed, an orphan means the reap predicate has a gap, and a
row without a manifest means the registry is inconsistent. Each leftover
now reports its manifest and blob refcounts plus how many files, derived
rows and attached rows point at it — so if this is a real leak rather
than the race, the next run names it instead of costing another full
pass through the suite.
storage_cleanup_check.sh asserted a sidecar exists on disk after
fetching a thumbnail. Correct while `.thumbnails/` was the durable
store; wrong since 10d2 removed that write. The check failed on exactly
the behaviour it was meant to confirm.
Inverted rather than deleted, because the inverse is the more useful
guard: a sidecar reappearing means a write path regressed to the legacy
shape, which would silently make `.thumbnails/` un-emptyable and strand
step 10e forever — its gate is the directory being gone, and a single
recreated file holds it open.
The HTTP 200 above already proves the thumbnail works; this now proves
it got there the new way.
Both of the helper's streams are silenced at the call site. It reports
absence loudly — red banner plus a `find` dump — because absence used to
be the failure; here it is the expected result, and leaving that visible
would cry wolf on every clean run.
The disk check proves no BYTES are left. This proves no ROWS are, which
fails differently and worse: a stale storage.blobs row with nothing
behind it means a reference was never released, and dedup_gc will skip
it forever because its count never reaches zero. Silent, permanent, and
invisible to a check that only looks at the filesystem.
Zero is the right assertion, not "fewer than before". By this point the
suite has deleted its users, their drives and everything cascading
beneath, and the disk check has already insisted the blob store is
empty. A non-zero registry beside an empty disk is exactly the
divergence the consistency jobs report — caught here first because one
number is easier to read than a findings list.
Degrades to a warning if the endpoint is unavailable rather than
failing, so a build without the admin dedup surface still runs the rest.
The disk checks above prove nothing leaked. These prove the bookkeeping
behind them is honest: every refcount matches what the reference sources
hold, and no row points at bytes that are gone.
End of suite is the only place this is cheap. One database serves every
hurl file, so by here the counters have absorbed every upload, copy,
move, share, trash and purge the suite performed — across both copy
paths, the derived tier and the attached tier. Drift that no individual
test would notice, because each only inspects its own file, surfaces as a
mismatch.
Runs after the GC drain deliberately: mid-sweep state is legitimately
inconsistent — a manifest can sit at zero waiting for the next pass — so
checking earlier would report normal in-flight state as drift.
Zero findings is the assertion. These four tenants are read-only, so
anything they report is a real invariant violation rather than a repair
opportunity. A job missing from the build is skipped with a warning
instead of failing, so this does not break on a feature-gated build.
Unknown job names and unwrapped-vs-wrapped response shapes both degrade
to a visible warning rather than a silent pass: list_job_runs currently
returns a bare array, and the .runs/.items fallbacks exist so a future
wrapping does not quietly turn the whole check into a no-op.
One dedup_gc pass cannot fully drain now that thumbnails are derived
blobs. Reaping a source releases the references its derived artifacts
hold (each content_derived_blobs row pins a manifest), and those
releases happen mid-sweep — the derived chunks are stamped orphaned as
the pass is already walking past them, because
remove_manifest_reference deliberately does not unlink, to avoid racing
a concurrent upload re-referencing the same chunk. They are collectible
only on the NEXT sweep, which is why the check saw 15 leftover blobs.
Loops until a pass reclaims nothing rather than hardcoding two. Two is
correct only while the derivation graph is one level deep — a thumbnail
is derived from a file, nothing is derived from a thumbnail. That is a
property of the data, not an invariant the code enforces, so a fixed
count would silently under-drain the day transcodes-of-thumbnails or
E2E-wrapped derivatives exist, and the failure would surface as a
confusing leftover-file assertion rather than the design change it is.
Bounded at 3 with a warning if it does not settle.
Sleeps between passes. The JobRegistry serialises runs of the same job,
so a back-to-back trigger risks rejection as already-running — which
returns 0 reaped and would exit the loop early, declaring success with
blobs still on disk. A false pass is worse than a slow one. It also
gives the previous pass's detached unlink tasks (spawned by
on_blob_deleted, awaited by nothing) time to land.
Deliberately NOT fixed in production code: derived chunks land inside
the 1-hour orphan grace, so a second immediate sweep would collect
nothing there and the next scheduled run picks them up. A fixpoint loop
in garbage_collect would be dead code outside force=true, which is only
this test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
renable thumbnail test, ensure that blob lifecycle correctly
trigger thumbnail cleanup on blob deletion
need to call `/api/admin/internal/trigger-gc?force=true`
Edouard Vanbelle