Two changes to the import jobs' destructive path.
thumb_attached_import now deletes orphaned sidecars under `repair`,
matching the dead-source case on the derived side. An `ext-` file whose
owner is gone is unimportable — the FK on file_id would reject the row —
so leaving it means it is rediscovered every run, the tail never empties
and step 10e's gate never opens. Safe despite these being the
non-regenerable bytes: the preview is keyed to a file_id that no longer
exists, so nothing can reference it again. Unrecoverable and unreachable
are different things, and this is both.
And every deletion is now audited. A one-way migration removing
user-visible files should leave a trail that outlives the run history:
findings are per-run and get purged, whereas target: "audit" is
separable and retained. If a preview later turns out to be missing, this
is the only record saying the migration removed it and when.
`owner` carries the id the file belonged to — source_hash for
content-keyed, file_id for uploaded — because that is where an
investigation starts, and the raw logs cannot supply it: NEW BLOB names
the hash of the STORED BYTES, a different value from the sidecar's own
name, which is why grepping one against the other finds nothing.
reason is a stable key: `imported` (replaced by a verified blob),
`source_gone`, `orphaned`. The first lives inside verify_and_unlink so a
verified deletion cannot be logged inconsistently; the other two are
explicit, since those paths have nothing to verify against.
Running the import on a real install produced a store-then-discard loop:
NEW BLOB (CDC) immediately followed by MANIFEST DELETED, once per
sidecar. store_derived_blob wrote the bytes, the source-exists guard
refused the row, and `inserted == 0` released the reference again.
The refusal is right — `.thumbnails/` outlives years of deleted files,
and importing those would recreate exactly the orphan rows e4c78ae0
eliminated. The mistake was deciding it AFTER the write.
Now checked before the read and the store, via blob_exists (manifest
first, blob as fallback). Two costs it removes: a blob write plus a
manifest delete per dead sidecar on EVERY run, and a tail that never
empties — unimportable files are rediscovered forever, so the job never
reports zero and step 10e's gate never opens.
Reported as `sidecar_source_gone` so the scale is visible before
anything is removed, and deleted under `repair`. That is the one unlink
in this job needing no readback: there is nothing to read back and
nothing to regenerate from.
Counted separately in the completion log, because "skipped, source gone"
and "already present" mean different things to an operator deciding
whether the migration has converged.
Worth noting for anyone reading the raw logs: NEW BLOB names the hash of
the STORED BYTES, while the sidecar filename is the SOURCE hash. They
are different values, so grepping the log hash against .thumbnails finds
nothing. The new finding carries both.
Registered with interval None, so they ran only when someone remembered
to trigger them — which was your objection to gating anything on
operator timing. Now daily.
Not boot-time: that would delay readiness for a filesystem walk, and
both jobs are idempotent and resumable, so periodic is strictly better.
The tick deliberately does NOT delete. `repair` defaults false, so
scheduled runs import and stop; unlinking stays a deliberate operator
action, per no-silent-auto-repair. That splits the two halves the way
their risk differs — the backfill is safe to automate, removing files is
not.
Cost once drained is a read_dir over three directories returning
nothing, and after the directory itself is removed, not even that.
Completes step 10d. Same `?repair=true` opt-in and the same
readback-before-unlink as the derived half, and the check matters more
here: these sidecars hold the bytes that CANNOT be regenerated — a
client-uploaded PDF preview has no server-side render path — so the read
is the only thing between a migration and permanent loss, not
belt-and-braces.
verify_and_unlink is shared rather than copied. Two versions of "only
delete after proving the replacement is readable" would be two chances
to weaken one, and it is the rule the whole deletion step rests on.
Deletion covers the already-imported branch as well as fresh imports,
for the same reason as the derived job: a run without `repair` leaves
the sidecar behind, and a later run with it would otherwise see "already
imported" and never drain. Import first, enable deletion after, is the
expected operator sequence, so that branch is the common path.
Orphaned sidecars stay untouched — this job imports, it does not
reclaim, and a destructive default on a migration is what no-silent-
auto-repair forbids. Unverifiable ones are kept and reported, so the
next run retries.
With both halves draining, `.thumbnails/` can now actually empty and the
directory removal in the derived job can succeed — though not stably
until dual-write stops, since any render or upload recreates it.
Step 10d, for the content-keyed half. Opt-in via the existing `repair`
flag rather than a new one — the house rule is that a job does not
mutate on its default setting, so early runs import only and an operator
can inspect before committing.
Deleting from the job rather than a later release is what makes the
migration self-draining. Sidecars are LOCAL disk, so no release can know
whether every instance has finished; each instance draining itself needs
no coordination at all.
Verification before unlinking is the load-bearing part.
store_derived_blob reporting success is not proof the bytes are
retrievable — a backend that accepted a write it cannot serve would
otherwise have the last copy deleted on top of it. The blob is read back
and its length compared against the sidecar's; a failure keeps the file,
records a finding, and the next run retries. That read is the difference
between a migration and a data-loss bug.
Deletion applies to already-imported files too, not just fresh ones. A
run without `repair` leaves the sidecar behind, and a later run with it
would otherwise classify the file as "already imported" and never drain
it — and import-then-enable-deletion is the expected operator sequence,
so that is the common path rather than an edge case.
Directories are removed once genuinely empty, because ABSENCE is what
step 10e gates on, not emptiness: empty is momentary and an on-demand
render can repopulate it a second later, while absence is one-way and
cheaper to test (one stat, versus opendir/readdir/closedir). remove_dir
refuses a non-empty directory, so it needs no emptiness check and cannot
race a concurrent write into deleting live files.
thumb_attached_import still needs the same treatment; verify_and_unlink
should move somewhere shared rather than being copied into it.
The filter was strip_suffix(".webp"), but persist_rendered writes
{hash}.{format.ext()} — so any client not advertising WebP leaves
{hash}.jpg on disk. Correct only while the derived tier was WebP-only;
once variant carried the format (20261022000000) a JPEG sidecar became
ordinary content, and leaving it unclaimed would keep .thumbnails/
permanently non-empty — the very signal step 10e gates on. The migration
could never finish.
Both codecs are now claimed and the format comes from the file's own
extension, so a .jpg imports AS JPEG. Deriving the variant and
content_type from it rather than hardcoding WebP is the point: a
mislabelled row would serve the wrong codec to whoever the read path
then matched it for.
ThumbnailFormat::ALL exists so the claim list and the write path cannot
drift — adding a format without teaching the import about it would
strand that codec silently.
The `ext-` rejection now carries real weight. Previously .jpg was
rejected wholesale, so the two jobs could not overlap by construction;
now they share an extension and only the prefix separates them. Both
directions stay under test.
Caught by the cross-job assertion, which counts every real sidecar being
claimed exactly once — the fixture gained a .jpg and the total moved 3
to 4, which is the test noticing rather than a test to update.
76590160 changed `variant_of` to return `{size}.{ext}`, but that value
was also being used as the on-disk DIRECTORY. Reads became
`.thumbnails/preview.webp/{hash}.webp`, which does not exist, so every
sidecar counted as unreadable and thumb_derived_import restored nothing.
Caught by thumb_import_check.sh on the run after the migration — the
harness earning its keep twice now, since this is the second defect it
has caught that no unit test could.
They are genuinely two strings and are now named as such: `dir_name` for
the path, `variant` for the row key. The cursor keeps using the
directory, so a run paused before the migration resumes at the same
position rather than restarting.
Also stops podman's compose-provider banner from burying the script's
output. Filtered rather than discarded, so genuine psql errors still
surface — swallowing those would turn a broken query into a silently
wrong assertion. Suppressing it at the source needs
`[engine] compose_warning_logs = false` in containers.conf, which is
per-developer config and cannot be relied on in CI.
`content_derived_blobs.variant` held the size alone, so one source could
hold exactly one artifact per size regardless of codec. That surfaced
when the read order flipped in 10c: a JPEG request matched the WebP row
and would have been served the wrong codec — hidden previously because
the .jpg sidecar won first. The flip had to be gated to WebP, which
meant JPEG clients could never leave the sidecar, which meant the
sidecar could never be deleted.
It blocks transcodes harder: those are multi-format by nature, so two
output codecs of one source collide on the primary key without a format
term.
The axis goes inside the string rather than into a fourth PK column,
per the column's own rule — "new axes go inside this string, never into
new columns". Shape is {size}.{ext}: preview.webp, icon.jpg, later
720p.webp.
The backfill is deterministic, not a guess: store_derived_blob has only
ever written "image/webp" for thumbnails. content_type is checked anyway
rather than assumed — a row that fails the assumption is left alone and
counted in a warning, because the read path then simply misses it and
falls back to the sidecar, whereas guessing a codec would serve wrong
bytes. Idempotent via NOT LIKE '%.%', so a re-apply cannot produce
preview.webp.webp; verified on a scratch PG by applying it twice.
One helper builds the string, because it is a primary-key component:
a writer and reader that disagree do not fail loudly, they just never
find each other's rows and the derived tier silently looks empty. It
lives on the service's ThumbnailSize, not the port's — they are distinct
types, which the compiler pointed out after I put it on the wrong one.
The WebP gate on the read path is now removed: each codec has its own
row, so JPEG can finally reach the derived tier — the prerequisite for
deleting the sidecar for those clients.
file_attached_blobs keeps a bare size: store_external_thumbnail
re-encodes everything to JPEG, so it is single-format by construction
and a format term would cost a migration for nothing.
Two revisions from working through step 10.
**Deletion moves into the import jobs, not a release.** Sidecars are
local disk, so a release cannot know whether every instance has drained
— gating on "an empty tail" asks an operator to coordinate a fact
nothing reports, and there is no telling when or whether they trigger
the jobs at all. Each job unlinking what it has imported makes every
instance drain itself. Constrained three ways: verify the derived blob
reads back before unlinking (a store that reported success but landed
unreadable would otherwise take the last copy), only after the
read-order flip (or the derived tier takes its first production traffic
by accident), and opt-in, since a migration that deletes by default is
surprising. Scheduled tick rather than boot trigger — idempotent and
resumable, so periodic is safe, while walking .thumbnails/ at startup
delays readiness for nothing.
**Found while checking the dual-write assumption: it does not hold.**
store_derived_blob has ONE call site; fs::write(&thumb_path, …) has
five. get_thumbnail, generate_and_persist and
generate_all_sizes_background all persist sidecar-only. That breaks the
migration's premise rather than being untidy — on-demand renders keep
producing un-migrated state after the import runs, so the tail never
empties and the deletion gate never opens. One persist_thumbnail owning
sidecar + derived + moka is therefore a prerequisite, and it makes "stop
writing sidecars" a later one-line change instead of four edits. Noted
that ThumbnailService holds no DedupService, so it must be threaded
through.
Also corrects a claim I put in thumb_derived_import's own docs:
transcoding is NOT a later step. ImageTranscodeService exists and caches
.transcoded/{ext}/{file_id}.{ext}, so a third import is needed and it
must re-key file→content — legitimate only because a transcode is
derivable. Its .skip markers remain an open question.
Both imports run over ONE directory, where the two legacy shapes sit
side by side, so the property worth asserting spans them: together they
must claim every real sidecar exactly once, and neither may take the
other's. A job that drifted into the other's shape would content-key
user-supplied bytes — sharing one user's uploaded preview onto every
file with identical content — and no per-job test in isolation would
notice.
So the fixture is shared. `legacy_tree` builds a directory holding a
content-keyed .webp pair, an ext- upload, and a stray README, and both
test modules walk it: derived claims exactly the two hashes in sorted
order, attached claims exactly the ext- file, the two sets are disjoint,
and between them they account for all three real sidecars.
`sidecar_names` became an associated function taking the root instead of
reading `self`, which is what makes this testable at all — the walk is
the half that decides which files a job claims, and it needed no pool to
verify. Sorting is asserted rather than assumed, since the cursor
resumes by skipping everything at or before it and a stable order is the
only thing that makes that correct.
A missing size directory is covered too: normal on a fresh install, and
it must yield no work rather than abort the walk.
Not covered here, and it needs a pooled fixture that does not exist: the
round trip itself — store the blob, write the row, and confirm a COPY
inherits the preview. That belongs in the API-level harness, where the
legacy state can be manufactured through the real write path and then
stripped.
First half of step 10. Every server-rendered thumbnail written before
content_derived_blobs existed lives only as
{thumbnails_root}/{size}/{hash}.webp — local-disk state that another
instance cannot see, a backend migration does not carry, and no
consistency job covers. This walks those files into the blob store and
records the mapping, so the derived tier can become authoritative and
the sidecar can be deleted.
A registered JobRegistry tenant rather than a script: the volume is
unbounded, so it needs a cursor, resume, cooperative cancel and run
history, and an operator needs somewhere to watch it. Cursor is
{size_dir}/{filename} over a sorted walk, which totally orders the
traversal.
Idempotent by construction — each file is skipped when a row already
exists, and store_derived_blob is ON CONFLICT DO NOTHING with
release-on-conflict beneath it, so re-runs cannot inflate refcounts.
Re-running is the expected operator behaviour, since Phase 3 (deleting
the sidecars) is gated on a run reporting zero imported.
hash_from_sidecar_name deliberately rejects ext-{file_id}.jpg. Those
bytes are user-supplied and file-keyed; importing them here would
content-key them and share one user's uploaded preview onto every file
with identical content. They belong to thumb_attached_import. Both the
accept and the reject set are under test.
Unreadable files and store failures record a finding and continue: a
sidecar removed by a concurrent GC unlink between listing and read is
expected, not fatal, and the file is left in place for the next run.
Registered unconditionally rather than behind a flag — a migration
nobody can find is a migration nobody runs.
Edouard Vanbelle