Original correlated-subquery form was O(files × manifests) and
O(blobs × manifests) — hit statement_timeout on a large production
customer's DB and hard-failed app boot (migration rolls back → sqlx
marks failed → next start also fails; only recovery was bumping the
role-level timeout manually before restart).
Rewrite:
* SET LOCAL statement_timeout = 0 (tx-scoped, auto-reset at COMMIT)
— lifts the safety net for THIS migration only, so operators
with restrictive session defaults can complete the one-time
repair without intervention.
* Both UPDATEs replaced with WITH ... UPDATE ... FROM CTE + LEFT
JOIN patterns — single scans per source table, linear total work.
* unnest(chunk_hashes) replaces b.hash = ANY(...) — cost is
O(Σ chunk-array lengths), not O(blobs × manifests). No GIN
index needed.
Measured on sandbox with 303 rows of drift: 570 ms in the original
form. New form on 200 rows drift, cache-warm: 15 ms. Second run on
clean data: 10 ms no-op — idempotency preserved.
Content semantics unchanged — same auditor formulas, same idempotence
guarantee, same content-safety guarantees; only algorithmic
complexity + statement_timeout scope changed.
Adds an Azurite service and a scenario that audits the Azure backend
through `?storage=azurite`. It is the only coverage of that code path in
the tree: `AzureBlobBackend` has unit tests for its name parser and
ordering, but nothing else speaks the protocol, and a paid account is
not an option for CI. Azurite implements the real Blob REST API, so this
exercises SharedKey signing, prefix/marker paging, and the 256-way shard
walk with its termination.
## Harness
`docker-compose.test.yml` gains an azurite service on 10000 (tmpfs, so
it dies with the stack). `spawn-db.sh` provisions the container itself,
because `AzureBlobBackend::initialize` verifies rather than creates —
signed by hand with curl + openssl rather than pulling a ~700 MB `az`
image for one PUT. Two traps are commented there: the account key is
base64 but HMAC wants raw bytes, and the canonicalized resource repeats
the account name (`/{acc}/{acc}/{container}`) because the emulator puts
in the path what real Azure puts in the host. Getting that wrong yields
403, not a hint.
The `azurite` entry is declared in `server.env` but never activated, so
the suite's active backend stays local and only this file reaches Azure.
## What it asserts, and what it cannot
A failure surfaces as `ok: false`, because an enumeration error now
fails the run rather than degrading to a per-row probe.
It deliberately asserts no finding count. The container starts empty and
the job's grace window is an hour, so a freshly-uploaded blob is skipped
in both directions by design — an audit here can only report zero, and
"zero findings" would pass whether enumeration worked or returned
nothing. The one positive assert, `scanned_count != 0`, therefore sits
on the local control, which does hold blobs; `scanned_count` accumulates
via `checkpoint`, which the empty-page early return skips.
## No cutover, deliberately
Putting real bytes in the container means `backend_migration
?storage=azurite`, which hangs on the first blob: `head_check` issues a
~40-byte ranged GET, `azure_core` 0.21 attaches
`x-ms-range-get-content-crc64` to anything under 4 MiB, Azurite 500s,
and the deterministic error is retried forever while
`migration_readonly` refuses writes app-wide. The full chain and the
rejected workaround are in the file header. The scenario is still
ordered last in `run.sh` — it is the only one needing a second service,
and the cutover comes back there once the official SDK lands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AzureBlobBackend` inherited the trait's `operation_not_supported`
default for `list_blob_hashes`, so every `backend_consistency` run on
Azure fell back to a per-row probe. That fallback walks `storage.blobs`
asking "are these bytes there", which structurally cannot find orphans:
bytes no row claims are invisible to anything starting from the
database, because you need a hash to ask about one and discovering
unknown hashes IS enumeration. Azure had half the coverage of local and
S3, in the direction that wastes space.
## Enumeration
The obstacle was the cursor contract. The caller advances ONE cursor
across both sides of the merge-join, feeding the same value to the
backend and to `WHERE hash > $1`, so the cursor IS a blob hash. S3
satisfies that with `StartAfter`. Azure has no equivalent on this SDK:
REST 2023-05-03 added `startFrom`, but `azure_storage_blobs` 0.21 never
sends it — `ListBlobs` exposes only prefix, delimiter, max_results and
an opaque marker that cannot be derived from a hash.
Resume rides on `prefix` instead. Names are `{hash[0..2]}/{hash}.blob`,
which partitions the container into 256 shards that are themselves in
hash order, so walking 00/…ff/ yields exactly the global order the
merge-join needs and a cursor names the shard to restart in.
Re-listing on resume is bounded by shard width rather than by the whole
container — the cost a client-side skip over a flat listing would pay on
every page. `marker` pages within one call and never escapes as the
cursor, the same treatment the S3 impl gives its continuation token.
One asymmetry against S3, deliberate: constraining to `{2-hex}/` means
foreign files outside that shape never reach `unknowns`. Safe in the
direction that matters — an orphan is a blob we wrote and stopped
referencing, so it always has the canonical name — and it buys O(N)
enumeration instead of O(N²/limit).
`hash_from_blob_name` mirrors S3's parser, shard-equals-prefix check
included: without it a mis-sharded name would round-trip to a
`blob_name` we never wrote, reporting a live blob no read path can find.
Tested for round-trip, for nine non-canonical shapes, and for the
ordering premise the merge-join rests on.
## Removing the fallback
With Azure enumerating, nothing shipped answers
`operation_not_supported`. The fallback's other stated justification —
mid-migration — never applied: it named a `MigrationBlobBackend` that
does not exist, and `SwappableBlobBackend::list_blob_hashes` forwards to
whatever is currently active, as do the Encrypted, Cached and Retry
wrappers.
What still reached it was a transient failure (auth blip, throttle,
network) relabelled as a capability limit, on a run that then read as
clean while having silently lost orphan coverage. So it was not merely
dead, it produced the wrong outcome — the only one it could. It also had
zero test coverage across 227 lines.
Now any `Err` from `list_blob_hashes` fails the run. That is louder than
an anomaly on a green run, which was the fallback's own goal. The trait
default still returns `operation_not_supported`, so a genuinely
unenumerable backend would fail every run — detectable, not silent, and
the point at which to bring the fallback back with tests.
## Also here
A doc note on `get_blob_range_stream` explaining why the Azurite
migration hang is not worked around: `azure_core` 0.21's
`Range::as_headers` attaches `x-ms-range-get-content-crc64` to any range
under 4 MiB with no opt-out, Azurite 500s on it, and `azure_core`
retries a deterministic error forever. The reachable path is
`backend_migration` → `EncryptedBlobBackend::head_check` →
`get_blob_range_stream(hash, 0, HEADER_SIZE)` — a pre-write probe on the
TARGET, so it fires on the first blob while `migration_readonly` refuses
writes app-wide. Working around it would trade production read
amplification for emulator support; the fix is the official SDK, where
`range_get_content_crc64` is an explicit field.
`RUSTSEC-2026-0275` is ignored on the same reasoning — `azure_core` 0.21
logs the `authorization` header at debug, the advisory's "upgrade to
>=0.22.0" names a version that does not exist, and the real remedy is
that same migration. Reachable only via an explicit
`RUST_LOG=…,azure_core=debug`; the entry says not to run that against a
real account.
`docs/plan/jobs-handling-recoverable-error.md` covers the other half —
a bounded retry should have turned that hang into a Paused run with a
reason, whatever the SDK does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 12. The two tables existed with COMMENT ON text, but nothing
explained the pair together — and the relationship is the part that
matters: they hold the same kind of artifact under two different keys,
and the keying difference is a security boundary.
Content keying shares one derivation across identical bytes, which is
free and correct for something the server derived. Apply it to
user-supplied bytes and uploading a file whose content matches someone
else's lets you replace the preview they see. A single table with a
`kind` column cannot express that: the key has to be one thing or the
other, and either choice is wrong for half the rows. The split is the
enforcement, which is why the two import jobs each refuse the other's
filenames rather than one job handling both trees.
Covers structure, negative rows and what may not become one, the NULL
trap (comparison against NULL silently excludes negative rows —
correct for refcounts, wrong for dangling checks, fatal for
enumeration; all three have been hit), why `variant` carries the format
on one table and not the other, lifecycle and which consistency job
covers which failure, worked examples, and a decision rule for adding a
third artifact type.
Records `content_type`-as-key as a rejected alternative: reasonable
until negative rows made the column nullable, and PostgreSQL does not
allow a nullable column in a primary key. Worth writing down because a
later feature retroactively eliminated an option that would have looked
sound at the time.
Named for the two things rather than "satellite tables" — that is
internal shorthand nobody would search for, while `derived` and
`attached` are the words the schema and jobs already use. Mentioned once
in the intro so the code's collective noun still resolves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The security boundary is a shape before it is a rule: DERIVED hangs off
content, ATTACHED hangs off the file, and two arrows starting from
different places carries the argument faster than the paragraph
explaining it.
Sketched inline rather than left as "add a diagram", so whoever writes
the page inherits the structure — including the parts a diagram is
uniquely good at showing: that both tables point into the same artifact
space (hence why one cannot be folded into the other with a kind
column), and that DERIVED.blob_hash is optional where ATTACHED.blob_hash
is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tables exist and carry COMMENT ON text, but nothing explains the
pair together, and the relationship is the part that matters: they hold
the same kind of artifact under two different keys, and the keying
difference IS the security boundary. Content keying shares one
derivation across identical content, which is exactly what must not
happen for user-supplied bytes — one user's uploaded preview would be
served for every file whose content matches. A single table with a kind
column would not prevent that; the split does.
Today that reasoning lives only in scattered prose across this plan and
two job doc-comments, so someone adding a third artifact type has
nothing to read.
Scopes the page: column-by-column structure including the parts that
mislead (nullable blob_hash meaning a negative verdict, uploaded_by
NOT NULL with no FK), worked examples that make the rule checkable,
lifecycle and which consistency job covers which failure, and the
NULL-handling trap that has already caused two bugs — comparison
against NULL silently excludes negative rows, which is correct for
refcounts and wrong for dangling checks.
Lands in docs/architecture/ beside backend-storage.md, which documents
the blob layer underneath.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan still said "Scoped 2026-08-27, not started" for work that is
complete and validated against S3 — the first thing a reviewer reads,
describing the PR as unwritten.
Records the two pieces that were not in the original scoping: the memory
cache moving to content keying (it was still file-keyed, so identical
content held two RAM entries), and the engine binding a run to the flags
it started with. Also that `.transcoded/` legitimately persists where
external mounts exist, since hash-less callers have no content identity
— absence is only expected elsewhere.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The durable tier has been content-keyed since it was introduced —
`content_derived_blobs(source_hash, kind, variant)` — but the moka cache
in front of it was still `{file_id}:{ext}`, so the layer closest to the
request used the wrong axis while the layer behind it used the right
one. That was legacy shape, and I had defended it in a comment as
"deliberate: per-request-path and short-lived", which was a
rationalisation rather than a reason. Ed asked why, and there is no why.
Transcoding is a pure function of the source bytes. Under file keying,
two files with identical content held two RAM entries for identical
bytes, and the second file was a guaranteed miss that fell through to a
DB lookup plus a blob read to fetch what was already in memory under
another key.
Now keyed by content hash when the caller has one, by file id only when
it does not — the same `content` / `external` split `ThumbnailCacheKey`
already makes, and for the same reason: hash-less callers (external
mounts) have no content identity to key on. Prefixed `c:` / `f:` so the
namespaces stay disjoint; a hash and a UUID cannot collide in practice,
but "in practice" is how a file ends up served another file's bytes.
`invalidate` now clears only the file-keyed entry. Dropping content
entries there would be wrong, not merely wasteful: one file's content
changing says nothing about the other files sharing the old bytes, and
evicting theirs would make one user's edit cost everyone else a
re-transcode. Content entries need no eviction — new content is a new
hash, so the old key is never consulted again.
transcode_cache.hurl updated to match, and its header corrected: the
second file is now a RAM hit rather than a derived-tier read, so that
scenario can no longer isolate the durable tier. It says so, and points
at satellites_consistency and a restart as what covers it instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`run_or_resume` now records `JobRunArgs` in `params` on a Fresh open and
restores them on Resume, passing the restored args to the handler rather
than whatever the resuming caller supplied.
Two problems, and the engine is the only place both are guaranteed.
**A resumed run must not change mode.** Handlers read `args` on every
call, so a paused `?repair=true` import resumed by a plain trigger
silently continued as import-only: the deletion half never finished and
nothing reported it. `?deep=true` had the same hole — a paused bit-rot
scan resumed shallow while still presenting as the run that started
deep. `blobs_consistency` and `manifests_consistency` had hand-rolled
this for their own two flags; the three import jobs had not, and fixing
it per-handler means every future job remembering.
**The run row should say what it did.** For a destructive job, "did this
run delete anything?" is answerable only from `params`, which is what an
operator reads afterwards. Before this, a repair run and a discovery run
were indistinguishable in the history.
Deliberately not overridable on resume: adding `?repair=true` to a
resume would apply it to the remaining entries only, producing a run
that half-deleted. Cancel and start fresh is the honest way to change
your mind.
A missing key reads as false/None, so a run paused before this existed
resumes under-acting rather than deleting under a flag nobody gave it.
Failure to record the flags fails the run instead of guessing — acting
under unrecorded flags is the one thing worth refusing for jobs that
delete.
The flag list is hardcoded here. Letting each job declare its own
parameters — name, type, default — is the better shape and is its own
change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was still registered on a 24h tick while the thumbnail imports moved
to on-demand. The same reasoning applies and I missed it: the boot run
in repair mode is the migration, nothing writes to that tree any more
so the tail cannot grow afterwards, and a tick could not finish the job
regardless because ticks never pass `repair`. Once drained it was a
`read_dir` returning nothing, daily, forever.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three import jobs only checkpointed on a full batch, so the
remainder after the last one was never counted. A run shorter than
BATCH_SIZE never checkpointed at all: `scanned_count` stayed 0 against
a known `total_rows`, and the admin progress bar sat at zero for the
whole run and finished there.
Seen on a transcode_import run over 20 entries — 13 imported, 5
negatives, 2 already present, progress 0/20 throughout. The thumbnail
imports had it too, just less visibly: a 105-file run reported
`scanned_count: 100`, losing the tail rather than all of it.
Cursor-wise the final checkpoint is a no-op — the walk is finished, so
nothing resumes from it — but the scanned delta is what the progress
display reads, and it has to include the last partial batch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fire-and-forget raced its own purpose. A second request for the SAME
content arriving before the spawned write landed found no row, re-ran
the full decode + encode, and stored the identical blob again. Keying
derivations by content exists so identical content is derived once — a
write that has not landed yet cannot deliver that, and the window is
milliseconds wide exactly when it matters most, a page loading many
images at once.
Caught by transcode_cache.hurl, which asserts a second distinct file
with identical bytes does not re-transcode: `transcodes: 2` where 1 was
expected, `disk_hits: 0` where the derived tier should have answered.
It had been passing on timing luck.
The cost of awaiting is bounded. This path has just spent a full decode
and re-encode, so one blob write beside it is marginal, and it only
runs on a genuine miss — every subsequent request for that content is
served from the row.
The negative verdict was already awaited, which is why only the
positive half of the scenario failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the job's surface: that it is registered, declares the metadata
the admin panel switches on (`mutates: always`, recoverable, a repair
description), and that a run against a drained tree completes cleanly
with zeroed counters.
It deliberately does NOT cover the re-keying, which is the part that
matters most. That needs `.transcoded/webp/` entries on disk before the
run, and nothing reachable over HTTP can create them — since the write
path moved to the derived tier, only hash-less callers still write
there, and hurl cannot place files in the server's storage directory.
The migration is validated by a snapshot restore instead, the way the
thumbnail one was; the file says so rather than implying coverage it
does not have.
The empty-tree assertions still earn their place. A drained tree is
what every run after the first sees, so it is the overwhelming
majority of this job's lifetime, and "does nothing, quietly" is a real
property: the thumbnail teardown warned `could not be removed / No
such file or directory` on every boot after its migration finished —
warning about success forever — and that was caught by eye, not by a
test. Counters are asserted as exact zeros, since finding work in a
directory the API cannot populate is the shape a re-keying bug would
take.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the pattern the thumbnail migration established, for
`.transcoded/`.
`initialize` no longer creates the tree. Creating it at boot is exactly
what kept `.thumbnails/` alive across restarts — the import removed it,
the next boot put it back, and the absence the read path gates on was
unreachable by construction. The write path already calls
`create_dir_all` on the parent before writing, so eager creation
achieved nothing except defeating the drain.
It now probes instead: one `stat`, cached for the process lifetime, and
the local-cache reads short-circuit on a relaxed atomic load when the
tree is gone. Fails open, so a service built without `initialize`
behaves as before.
One difference from the thumbnail tiers, and it is not a stalled
migration: callers with no content hash — external mounts — cannot use
the content-keyed tier at all, so they still read and write here. On an
install without such mounts the directory drains once and stays gone;
on one with them it persists, correctly.
`transcode_import?repair=true` joins the startup defaults on the same
terms as the thumbnail imports, and with the weakest safety argument
needed of the three: a transcode is a pure function of its source, so
anything deleted in error is recomputed on the next request. The
`default_startup_jobs` test failed on the change rather than being
updated silently, which is what it is for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The twin of thumb_derived_import, with the difference that shapes the
whole job: the legacy tree is keyed by FILE (`{file_id}.webp`) while the
destination is keyed by CONTENT. Thumbnail sidecars were already named
by blob hash, so importing them was a move; every entry here has to be
resolved through storage.files first.
That re-keying is the point rather than bookkeeping. A sandbox with five
.skip markers had three of them naming the same image, so the file-keyed
tree stored one verdict three times. After the import it is one row, and
any future upload of those bytes inherits it instead of paying for the
decision again.
Both artifact kinds are claimed by one walk: `{id}.webp` becomes a
derived Blob, `{id}.webp.skip` becomes a negative row. They share a
source file and a cursor, so splitting them into two passes would be two
chances for the pair to disagree about what had been handled. `.skip` is
matched BEFORE `.webp` — the shorter suffix matches a marker too, and
getting that backwards would read a zero-byte file and store it as the
transcode of its source, then serve it to clients. There is a test.
Entries whose file is gone cannot be re-keyed at all, so they are
reported and, under repair, deleted: unimportable by definition, and a
run that keeps rediscovering them never reports zero, so the gate for
removing the directory never opens.
Deletion reuses verify_and_unlink, so a cached transcode is removed only
after its stored replacement reads back byte-identical. Directory removal
follows the same rule as .thumbnails/ — delete, and only rename aside if
a non-cache file is in the way.
The batch checkpoint is a shared helper rather than inline at both exits
of the loop body. The first draft duplicated it and dropped the future on
the skip path without awaiting: the entry counted toward the batch, the
cursor never advanced, and a resumed run would have rewalked everything
it had already handled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writing the hurl scenario surfaced a gap: a transcode that comes out
larger than the original runs a full decode + encode and increments no
counter at all. `transcodes` is bumped only on the success path, beside
`bytes_saved`, so the most expensive failure mode was invisible — a
multi-megapixel image decoded and re-encoded on every request, for
every file sharing that content, producing nothing.
That is precisely the cost the persisted negative verdict exists to
stop paying, and it could not be measured before or after. `not_beneficial`
counts it, kept separate from `transcodes` because conflating "work
done" with "work that paid off" would hide exactly what an operator
needs to see.
It is also what lets the hurl scenario assert the negative half: the
first fetch increments it, the second — a distinct file with identical
content — leaves it untouched, which is the negative row being read
rather than the verdict recomputed.
Assertions are exact equality against captured values throughout, no
`>` or `<`. A "greater than" would pass if a counter moved for the
wrong reason; equality against the prior reading catches any transcode
from any source, including one this scenario did not intend to cause.
Also fixes two URLs the first runs caught: file download is
`GET /api/files/{id}`, not `/content`, and the trash listing is
`/api/trash/resources`. And the duplicate uploads go to a second
folder — re-uploading the same filename into the same folder returns
the EXISTING file id, which would have made both halves of every
"two files, one content" pair the same row and left the scenario
asserting nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `GET /api/admin/transcode/stats` and a hurl scenario that uses it
to assert both halves of the caching contract.
The endpoint exists because the property was previously unobservable.
`derived_blob_copy.hurl` records the same limitation for thumbnails:
stored blob, RAM cache and a fresh re-render return identical bytes
with identical status, so no HTTP-level assertion can tell them apart.
Counters can. `transcodes` is work done; `cache_hits` and `disk_hits`
are work avoided, and a rising `transcodes` against a flat `disk_hits`
is exactly what a broken derived tier looks like from outside.
Each case uploads the same bytes as TWO distinct files. Re-fetching one
file would only prove moka works — that cache is keyed `{file_id}:{ext}`.
A second file with identical content is a guaranteed memory miss but the
same content hash, so avoiding a transcode there can only be the
content-keyed tier answering. That is the whole point of keying
derivations by content rather than by file, and this is the first test
that can see it.
The negative half is the one the row exists for: without it the server
re-runs a full decode + encode of a half-megabyte screenshot for every
file sharing that content, on every request, to discard the result each
time.
Assertions capture-then-compare rather than computing deltas — hurl has
no arithmetic in predicates, and pinning the exact prior value is
stricter anyway, since a transcode triggered from anywhere shows up.
Absolute values are never asserted: other scenarios in the same run
transcode too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The transcode negative path — "the result came out larger, serve the
original and remember that" — had no test because no synthetic image
reaches it. Measured against the real encoder: flat colour goes
4780 → 186 bytes, a diagonal gradient 24852 → 102, and uniform RGBA
noise still loses by ~242 bytes at every size, a margin constant in
absolute terms and so one that never flips. Grayscale does not help
either; WebP's subtract-green transform handles R=G=B.
Two things have to be true at once and only real content does both.
The encoder is the `image` crate's own minimal VP8L writer, not
libwebp, so it wins only where redundancy is extreme enough for any
encoder to find it. And the original has to be near PNG-optimal, which
a screenshot from a real capture tool is: a 2x Retina UI is long
identical runs, flat panels and sharp edges — precisely what PNG's
scanline filters plus zlib were built for.
So the fixture is a real OxiCloud screenshot (emails masked by
overtyping rather than block-filling, which would have added back the
flat redundancy the property depends on; re-verified negative after
masking, 556180 -> 511124 bytes).
`fixture_premise` pins both halves of what tests/api/transcode_cache.hurl
will assume — this one negative, red-image.png positive. Without the
guard a future encoder bump would silently turn the negative half of
that scenario into a second positive test: still passing, no longer
checking what it was written to check.
Worth recording for whenever libwebp replaces this encoder: most of
these screenshots would likely flip to positive, which leaves every
stored negative row a stale verdict. An encoder change has to purge
them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 7 of docs/plan/derived-blobs.md, write path first — the plan is
explicit that fixing it before the import means transcode_import only
has to handle history, not a moving target.
ImageTranscodeService now reads and writes storage.content_derived_blobs
under kind='transcode', keyed by the BLAKE3 of the SOURCE content. The
hash is threaded in from file_retrieval_service, which already holds it
as dto.content_hash; hashing here would be a BLAKE3 over the whole file
on every request. Callers without one (external mounts) keep the local
cache untouched, which is what the service did before this tier existed.
Negative verdicts become rows rather than zero-byte .skip files. A
transcode that came out larger is deterministic in the content, so it is
worth remembering; the row survives moka eviction, a restart, and the
deletion of .transcoded/, none of which the marker does. Only that
verdict is persisted — a timeout or a read error returns Err and is
recorded nowhere, because a momentary failure written here would mark a
perfectly transcodable image hopeless with nothing to retry it.
Representation is a NULL blob_hash, per the plan: a sentinel hash would
stop blob_hash naming a real Blob and every consumer would need to learn
the exception. A CHECK keeps blob_hash and content_type NULL together —
a type without bytes describes nothing, bytes without a type cannot be
served.
Two consumers had to be corrected for NULLs first, both of which would
have broken on the first negative row ever written:
* satellites_consistency reported them as derived_dangling_blob at
data_loss severity. SQL comparison against NULL is NULL, so EXISTS was
false and a row correctly pointing at nothing read as an artifact that
had gone missing.
* blob_reference_sources::list_referenced_blobs decodes blob_hash into
String, so the first NULL would have failed the decode and taken the
whole enumeration down. It would also have been wrong if it decoded —
a negative row holds no reference, which is why the counting forms
(WHERE blob_hash = <hash>) already exclude it for free.
lookup_derived returns a three-way answer because Option collapses the
two cases a caller deciding whether to spend a decode most needs apart:
never attempted, versus attempted and known not worth it.
DedupService is attached after construction via a OnceLock. DI builds
the transcode service ~240 lines before DedupService exists, and the
retrieval path that needs it is wired earlier still, so a constructor
argument would mean reordering more than this is worth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every startup job logged two info lines saying the same thing: the
scheduler engine's `job.run` (outcome + timing, which every dispatch
has always produced) and my `job.startup_completed` right after it.
Reading the boot log, that looks like the job ran twice.
Demoted to debug. The engine's line is the one that matters — logging
uniformly is the reason startup jobs go through `registry.trigger`
rather than calling handlers directly — and the `job.startup_trigger`
audit line before it already records that the startup path was the
caller, along with the flags it used.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every boot after the migration completes logged
`WARN legacy sidecar directory could not be removed / No such file or
directory`. The directory being absent IS the end state — it is what
success looks like from the second boot onward — so this warned about
the migration having worked, forever, on every restart.
Returns early when the root is gone, which also skips walking three
directories that no longer exist. The remaining `Err` arms keep their
warning for the cases that are genuinely failures: a directory that
exists and cannot be removed or moved aside.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found on a sandbox restore. `thumb_derived_import` ran first, imported
and deleted its own hash-named sidecars, then found `remove_dir` refused
because the `ext-*.jpg` previews were still there — those belong to
`thumb_attached_import`. The rename fallback fired, moving the tree to
`.thumbnails.migrated`; the attached job then looked in `.thumbnails/`,
found nothing, and reported zeros.
That stranded the user-uploaded previews, which are the one class of
file here with no render path to rebuild them. The rename exists for
files NEITHER job claims — a `.DS_Store` blocking removal forever — and
it fired for the sibling's work in progress instead. Inverting the job
order does not help: once the tree is renamed, both jobs look at
`.thumbnails/` and find nothing, whatever order they run in.
Teardown is now shared and refuses to act while anything remains that
either job would claim. Both jobs call it, so whichever finishes last
removes the tree in the same boot rather than leaving an empty
directory until the next one. The rename survives for its original
purpose, and now only fires when the remaining files are genuinely
nobody's.
Also drops the daily tick on both imports — they are on-demand now. The
boot run in repair mode IS the migration: nothing has written a sidecar
since step 10d2, so the tail cannot grow afterwards, and a tick could
not finish the job anyway because ticks never pass `repair`. Once
drained it was a `read_dir` returning nothing, every day, forever.
UX: the "at boot" badge moves from beside the job name into the cadence
column. It answers WHEN a job runs, which is what that column is for —
next to the name it read as a property of the job, and the row could
show "on-demand" beside a badge saying otherwise.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A migration nobody triggers never finishes. Scheduled ticks deliberately
never pass `repair`, so a deployment whose operator never opens the
admin panel re-imported the same sidecars forever and never drained the
directory — and relying on operators to edit `.env` has the same failure
mode one level up.
`OXICLOUD_STARTUP_JOBS` dispatches named jobs once, in the background,
after the scheduler is ready. Entries use the syntax operators already
type at the trigger URL (`name?repair=true`), so the value is literally
the request they would otherwise make by hand. It defaults to both
migration jobs in repair mode, so an untouched deployment migrates and
drains itself.
That is a destructive default and a real exception to
no-silent-auto-repair, so the guard it rests on had to get stronger:
`verify_and_unlink` now compares CONTENT, not length. A blob of the
right size and the wrong bytes used to pass — a key-mapping bug handing
back another file's preview at the same length would have deleted the
original and kept the impostor, and thumbnails cluster tightly enough in
size for that to be a real coincidence. The readback streams from the
backend with no cache in front, so it proves durability rather than that
a write was acknowledged.
Deletion of `.thumbnails/` is attempted first and only falls back to
renaming it `.thumbnails.migrated` when `remove_dir` refuses because a
non-sidecar file is inside (Finder's `.DS_Store`). Either way the
directory stops existing, which lets the read-path probe go back to a
single `stat` on the root instead of walking the size directories.
Validation is fail-fast: an unknown job name or flag panics at boot. A
silently dropped `?repare=true` would leave the job in discovery-only
mode while the operator believed the tier was draining, surfacing months
later as "the migration never finished" with nothing pointing at the
config line.
Interrupted runs resume. Boot recovery flips abandoned rows to Paused
with their cursor, so `run_or_resume` continues rather than rescanning —
a long migration completes across however many restarts it takes. That
is a scoped exception to "we do not auto-resume": here somebody did ask,
in configuration, and not having to ask again is the point.
`StartupJob` holds a `JobRunArgs` rather than re-listing its four
fields, so a fifth flag cannot be added to the scheduler and silently
ignored in configuration.
Jobs named here are ordinary registered jobs — visible in the panel,
triggerable by hand, same runs and findings. Their rows now carry a
`startup` object so an operator can see that a job deletes on every boot
rather than only when someone clicks Run.
Adds docs/config/thumbnail-migration.md: what runs on first boot, how to
snapshot database and storage together beforehand, and how to verify
afterwards with satellites_consistency plus backend_consistency
?deep=true.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 10e was written as a removal release: delete the fallback read
path once the directories are empty. That has the same flaw as gating
deletion on an empty tail, one level up — sidecars are local disk, so
no release can know that every instance has drained.
The only removal that can actually be written is "if the tier is gone,
return". `initialize` now probes the size directories once at boot;
when absent, every fallback read short-circuits on a relaxed atomic
load and touches no filesystem. The code stays, costs nothing, and can
be deleted whenever — or never.
Two things had to change for absence to be reachable at all:
* `initialize` no longer creates the directories. It create_dir_all-ed
all three at every boot, so the import job removed them and the next
restart put them back — the absence this gates on was unreachable by
construction. Found on a sandbox where the job had drained the tier
and a restart left three empty directories behind. Nothing has
written a sidecar since step 10d2, so there was nothing to create
them for.
* The probe tests the size directories, not the root. On macOS Finder
leaves a .DS_Store in the root, which blocks remove_dir there
permanently; gating on the root would keep the fallback alive on
every developer machine for a reason unrelated to thumbnails. No size
directory means no sidecar.
Every sidecar read and existence check now goes through `read_sidecar`
/ `sidecar_exists`, so the guard exists once rather than at each of the
twelve sites that built a path and read it — the build-then-read pair
was duplicated six times over.
The import job's root removal reports its outcome instead of discarding
it. It is the one result an operator is waiting for, and "directory not
empty" with no sidecars left is a failure worth naming.
Falls open: the flag starts true, so a service constructed without
`initialize` behaves as before. A drain completing mid-process leaves
it stale-true until restart, which costs the same failed opens as
today; it never goes false while sidecars remain.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`blobs_consistency` probed `blob_exists` once per row and, under
`?deep=true`, read and re-hashed every blob. `backend_consistency`
already reports the same `blob_missing_from_backend` from its
merge-join — so the probe was duplicated work that found strictly less
(a DB walk cannot see backend-only orphans by construction) at N round
-trips instead of one enumeration. Every scheduled sweep paid for it.
All three physical checks move to `backend_consistency`:
* `blob_missing_from_backend` was already there; the duplicate is gone.
* `blob_corrupted` / `blob_unreadable` hook the matched arm of the
merge-join, which holds exactly the key pairs worth reading. Guarded
by `in_range` so a pair past the horizon is not read twice, and
`params.deep` is persisted on a fresh run and read back on resume so
a paused deep scan does not silently continue shallow.
Deep mode belongs there because it is backend work end to end: the
only DB input is the hash. Keeping it in `blobs_consistency` forced
that tenant to carry a backend for one flag.
What remains is the half that needs no backend: `refcount_mismatch`
and its repair. The constructor drops from five parameters to two —
no backend, no storage_entries, no storage_path_fallback — and
`?storage=<name>` / `?deep=true` are now inert there, which the
job description says outright.
`affected_files` is needed by both tenants, so it moves to a shared
`blob_diagnostics` module rather than being copied.
`PROBED_STORAGE_PARAM` moves to `backend_consistency`: it was defined
in `blobs_consistency` and re-exported, which is backwards once the
DB-only tenant has no entry to scope. The create-grace window goes
with the probe — it existed to avoid flagging a blob whose bytes had
landed before its row, and the refcount comparison reads one
consistent snapshot.
Known cost: `backend_consistency` returns `backend_unenumerable` on
Azure and mid-migration, so on those configs missing bytes now go
unreported where the per-row probe caught them. That argues for the
Azure enumeration impl, not for keeping the probe.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The admin panel had no repair toggle wired to anything but a hardcoded
name list naming the two refcount tenants, so `thumb_derived_import` and
`thumb_attached_import` could not be run in repair mode from the UI at
all despite supporting it. And nothing in the job list said what any
given job does or whether clicking Run on production writes anything.
Three defaulted methods on `JobHandler` and `RecoverableJobHandler`:
fn description(&self) -> &'static str
fn mutates(&self) -> Mutates // Never | Always | OnRepairOnly
fn repair_description(&self) -> Option<&'static str>
`RecoverableAdapter` forwards them — the registry only holds
`dyn JobHandler`, so a tenant's metadata is invisible otherwise, and
falling back to the defaults would report every recoverable job as
read-only, including the ones that delete files.
Three values rather than a boolean because a job can be read-only by
default and destructive under `?repair=true`; a boolean answers wrongly
for one of its two modes, and `false` on something that unlinks files is
the dangerous direction to be wrong in. `repair_description` returning
`Option` collapses "does it repair" and "what does repair do" into one
method: presence gates the toggle, content is the confirmation text —
which the frontend cannot invent, since correcting a counter and
deleting sidecars are not the same warning.
`OnRepairOnly` with no `repair_description` is rejected at registration:
it claims to mutate only under a flag it does not support.
All 17 registered jobs declare all three. The panel now renders the
description under each name, badges read-only jobs, confirms before a
plain run of a mutating one, and offers the repair variant off the
backend flag instead of the name list.
Descriptions are English in the trait, next to the behaviour: one in
`locales/*.json` rots invisibly the moment a job changes, and a
translator cannot know what `manifests_consistency` reconciles. i18n can
layer on later keyed by job name with these as the fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Extends the derived check to `file_attached_blobs` and renames it, since
the two tables are one concept — the content-keyed and file-keyed halves
of "things attached to a Blob" — and `storage.copy_file_satellites`
already established the vocabulary.
The attached half is the one that cannot be recovered.
`attached_dangling_blob` is data_loss with `recoverable: false`: those
bytes were user-supplied and have no server-side render path, so nothing
can regenerate them. Its derived twin carries `recoverable: true`,
because a derived artifact is a pure function of its source and
re-rendering restores it. Same finding shape, materially different
stakes, and the detail says which.
No orphan-mapping check on the attached side, deliberately: `file_id` is
ON DELETE CASCADE, so a row cannot outlive its file. The database
enforces what the derived table cannot, since a content hash has no row
to point a foreign key at — which is exactly why only that half could
rot.
One job walking two tables needs a phase in the cursor, or an attached
checkpoint would be replayed against the derived table and silently
re-scan or skip.
Two things the sweep was missing, found while checking whether every
consistency job is actually exercised:
drives_consistency and folders_consistency were registered but never
run by any test. Now included; the list is exhaustive by intent.
An unknown job was a warning-and-skip. That protected feature-gated
builds at the cost of something worse: this list said
`derived_consistency` for one commit after the rename and would have
dropped that coverage without a word, leaving the suite green over a
check that no longer ran. It fails now.
Finds derived mappings whose Blob is gone on either side. Nothing else
can, and that is the point rather than an oversight: every other job
reasons from a Blob outwards, so a row whose SOURCE was reaped breaks
none of their invariants — valid reference, exactly correct refcount,
bytes present on the backend. Every check agrees the system is healthy
while the artifact is pinned forever. A leak that looks like
correctness, which is why it took four suite runs to name.
Two findings:
derived_orphan_mapping (inconsistent) — source_hash has neither a
manifest nor a blob row, so purge_derived_blobs can never fire for it.
Storage that grows and never reclaims.
derived_dangling_blob (data_loss) — blob_hash has no Blob behind it.
The mapping promises an artifact that is gone, so a read finds the row
and then fails.
Existence means EITHER table on both sides, since source_hash and
blob_hash each name a Blob: a manifest for CDC content, a bare blob row
for legacy whole-file content. Checking one would report every legacy
blob as missing.
Paged on the full primary key with a row-value comparison rather than
source_hash alone — a source has several variants, so a page boundary
can fall inside one and advancing by source would skip the rest. Both
existence probes fold into the page query, so a page is one round-trip
rather than 2xN. Cursor round-trip is tested, including that a malformed
one fails loudly: silently restarting would make a paged audit
under-report, which is the worst failure available to a job whose
purpose is finding what is missing.
e4c78ae0 stops new orphans at the write side; this finds the ones
already on disk, which that fix cannot reach. Added to the end-of-suite
sweep so it runs against real state every time.
Permanent blob leak, three rows per image. Confirmed green after this.
The leftovers named their source, and it had no manifest, no blob row
and no files. Nothing will ever reap that hash again, so
purge_derived_blobs can never fire for it — meaning the rows were
written AFTER the source died, not left behind by a reap that skipped
them. Two earlier attempts assumed the latter and fixed the wrong thing.
Background thumbnail generation is spawned and unawaited, so an upload
deleted promptly — constant in a test suite, occasional for real users —
has its render finish after GC reaped the blob and then record three
mappings to a corpse. Each pins its own thumbnail blob at ref_count 1,
which GC is thereafter CORRECT to refuse: that is why three passes with
force=true reclaimed nothing and why the leak was invisible, a healthy
system declining to delete referenced data.
store_derived_blob now inserts only WHERE the source still exists,
checking both tables since source_hash names a Blob — a manifest for CDC
content, a bare blob row for legacy whole-file content. A refused insert
falls into the existing `inserted == 0` branch and releases the
reference, so the thumbnail blob becomes collectible rather than
stranded.
Closed in both directions: if the source dies before the statement's
snapshot the row is refused; if after, that reap's purge finds the row.
034f1050 stays — the bulk manifest reap genuinely lacked the purge that
reap_blob had, and two manifest reap paths with only one purging is its
own defect. It just was not this one.
Still missing, and now clearly worth building: the orphan-mapping check
the plan's coverage matrix already lists (content_derived_blobs.
source_hash with no Blob behind it). This stops new ones; nothing yet
finds the ones already on disk.
No test passed repair=true, so verify_and_unlink, the
sidecar_delete_unverified finding and the directory removal had never
executed. That left the one destructive part of the migration as its
least-tested code: everything else is additive and recoverable, this
unlinks files after a readback check, and a defect costs bytes.
Four assertions, because "the files are gone" cannot by itself tell a
correct drain from a destructive one:
sidecars gone — the drain happened
rows still present — it deleted the COPY, not the record. Removing
the row would strand the blob in exactly the way
the bulk-reap bug just did: a live reference
with nothing behind it, which GC is then correct
to refuse forever.
unverified == 0 — every unlink passed its readback rather than
being skipped, which is the property that makes
deleting safe at all
directory absent — the signal step 10e gates on, and why remove_dir
is used: it refuses a non-empty directory, so
success proves emptiness rather than asserting it
Preconditions assert the sidecars exist first, or a no-op run would pass
all four by doing nothing. The directory check logs rather than fails,
since a concurrent render could legitimately repopulate it.
Real leak, found by storage_cleanup_check.sh: three blobs surviving a
full teardown, all `derived=1`, all naming one `src` whose manifest,
blob row and files were already gone. The source had been reaped without
its derived rows being purged.
`reap_blob` purges correctly for the single-blob path. The BULK manifest
reap did not — it iterated the deleted batch only to invalidate the
manifest cache, so every manifest reaped that way left its
content_derived_blobs rows behind.
The predicate is not at fault. It protects a manifest that IS a derived
artifact (content_derived_blobs.blob_hash) and deliberately not one that
is the SOURCE of them, because counting source_hash as a reference would
pin every original for as long as a thumbnail existed. The source is
therefore reaped correctly and the purge simply has to follow it.
The consequence is permanent, not cosmetic: the orphaned row holds
chunk_manifests.ref_count at 1 on the thumbnail's own blob, so GC is
thereafter CORRECT to refuse it — which is why three passes with
force=true reclaimed nothing. Every deleted image left three behind, one
per size, growing forever.
Fixed at the reap rather than in any deletion path, which is where all
of them converge: folder cascade, drive deletion, user deletion and
single-file delete all reach it through the decrement trigger, so one
call covers every route.
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.
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.
The write paths now persist only to the blob tiers. Until this, dual-write
meant any render or upload recreated .thumbnails/ seconds after the import
job removed it, so step 10e's gate — "the directory no longer exists" —
could never hold.
Rendered thumbnails: the fs::write in persist_rendered is gone. Safe
because the read flip landed first, so nothing depended on that write to
be found, and a failed derived store now costs a re-render rather than
data — regenerable by definition. Existing sidecars are untouched and stay
readable through the fallback until the import drains them.
Uploaded previews needed a change first, and the order was not optional.
upload_thumbnail_impl logged and still returned 201 when store_attached_blob
failed — safe only while ext-{file_id}.jpg was a second copy. These bytes
have NO server-side render path, so removing the sidecar while the store
stayed best-effort would lose a user's upload behind a success response.
The PUT is now fatal, and drops the RAM entry too, or the cache would keep
serving a preview that was never persisted and vanishes on eviction,
contradicting the error the client just received. Only then does the ext-
write go.
thumb_import_check.sh had to change with it: its premise was "upload, then
delete the row, and what remains on disk is legacy state", which no longer
holds now that nothing writes sidecars. It lays them down itself, with the
bytes the API just served, at the exact paths the pre-10d2 code used. The
reconstruction stays faithful — same bytes, same paths — it just no longer
depends on current code to produce a shape current code has stopped
producing. Both sidecars get identical bytes, which is realistic rather
than a shortcut: they dedup to one blob while keeping separate mappings,
which is the property the keying split exists to preserve.
Step 10 went ...enable deletion, then remove the fallback "once the
directory no longer exists". That gate is unreachable as written: while
persist_rendered dual-writes and the PUT still writes ext-, any render
or upload recreates the tree seconds after the job removes it, so the
directory never stays absent and (e) can never fire.
Adds it as d2, between deletion and fallback removal, with the split
that only became visible while implementing 10d.
Rendered sidecars can stop immediately — the read flip has landed,
existing files are untouched so un-imported boxes keep their fallback,
and a failed derived store costs a re-render rather than data, since
that content is regenerable by definition.
Uploaded ones cannot, yet. upload_thumbnail_impl logs and still returns
201 when store_attached_blob fails, which is safe only because the ext-
sidecar catches it. Remove the sidecar while the store is best-effort
and a user's preview vanishes silently behind a success response — and
these are precisely the bytes with no server-side render path. So the
PUT must become fatal first.
Order recorded explicitly: make it fatal, then drop the sidecar.
Reversed, it trades a silent data-loss window for an empty directory.
Edouard Vanbelle