Ed pulled the network mid-migration and got nothing: no log, no pause,
after more than two minutes. The cause is not the classification work
that preceded this — it is that there was no error to classify.
Pull a network on an ESTABLISHED TCP connection and there is no RST and
no ICMP. The peer simply stops answering and the socket read blocks
until the OS abandons retransmission, on the order of fifteen minutes.
For that whole window the job is neither running nor failed. Nothing
retries, because nothing failed. It looks exactly like a slow migration.
A refused connection is instant and does surface, which is what made
the earlier `127.0.0.1` test look reassuring. It exercised the one
network failure that cannot hang.
## Two layers, because one does not fit
`TimeoutBlobBackend` is innermost, below retry — a hang has to become an
error before any layer above can react to it. Bounds are per operation
class, because one number cannot fit both a HEAD and a 5 GB upload:
metadata 30s exists / size / delete / init / health / list
open 60s time to FIRST BYTE, not transfer duration
write off the whole transfer is inside the future, so any
bound here is also a maximum upload duration
Write is unbounded by default deliberately: guessing it wrong truncates
legitimate uploads, which is worse than the hang it would prevent. All
three are configurable (`OXICLOUD_STORAGE_TIMEOUT_*_MS`, 0 = unbounded).
The S3 client also gets what it could always have had. It was built from
a bare `config::Builder::new()`, which carries NO `TimeoutConfig` at
all — so `SdkError::TimeoutError`, an arm `s3_domain_error` already
handles, was unreachable. It now sets connect/read timeouts plus
stalled-stream protection, which measures throughput rather than
elapsed time and is therefore the correct instrument for a stream: it
bounds a stalled upload without capping how long a large one may take.
## Local is not the justification
Ed's correction, and it is right: a local path is reached through the
kernel, and the kernel owns that timeout. iSCSI gives up after
`replacement_timeout` (120s default) and returns an I/O error; NVMe-oF
and soft-mounted NFS behave the same. Those arrive as `io::Error` and
`local_io_error` already classifies them. Local passes through the
decorator only because a uniform chain beats a conditional one, and a
bound that never fires costs nothing.
The real asymmetry is Azure: its 0.21 client has no timeout knob short
of a custom transport, and the SDK migration is deferred. That is why
this lives in the chain rather than being configured per SDK.
Also fixes the log gap: the timeout warns with the wrapper, backend,
operation and bound, so a stalled layer is visible before the pause
rather than only afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit fixed get / get-range / stat but left `blob_exists`
returning `internal_error` on S3 and Azure, which undoes the point of
the exercise: `blob_exists` is the FIRST call `backend_migration` makes
against the source for every blob.
match self.source.blob_exists(hash).await { // migration, per blob
An unclassified error there is permanent, so a refused connection during
a migration takes the permanent branch — record a finding and move on —
which is the skip-and-advance behaviour the pause was added to prevent.
The classification has to hold at the probe, not only at the read that
follows it.
Both now classify before deciding: only a genuine 404 / `is_not_found`
answers "absent", everything else keeps its transient class. On S3 that
means classifying the `SdkError` by reference first, since
`into_service_error()` consumes it.
Local was already routed through `local_io_error` at its stat site.
Audited the rest of the S3 surface: initialize, put ×3, get, get-range,
delete, stat, list and exists all classify. The two remaining
`internal_error`s in `put_blob` read a *local* source file, so there is
no network class to preserve.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ed's point, and the most dangerous bug in the batch: NotFound is a
conclusion callers ACT on. Every read path in all three backends
returned it unconditionally.
// s3, azure, local — all of them
.map_err(|e| DomainError::new(ErrorKind::NotFound, …))
So a refused connection, a 503, an expired credential, a stale NFS
handle and an unmounted iSCSI target all reported "blob missing". Nine
sites: get / get-range / stat on each backend.
## Why it is disastrous rather than untidy
`backend_migration` probes its source before copying. A transient probe
error used to `continue` — skip the row, record NOTHING, and let the
cursor advance past it at the end of the batch. With `failed` still 0
the run reached `finish_completed` and FLIPPED THE POINTER to a target
missing every blob the outage covered. A migration reporting success
having silently dropped whatever was unreachable at the time.
That path now pauses when the probe error is transient, and records a
finding when it is permanent, so a run can no longer report clean while
having skipped rows.
## Local storage is not exempt
Ed again: a local backend is a PATH, and that path may be an iSCSI or
NVMe-oF LUN, an NFS mount, or a disk with a failing sector. It matters
MORE there than for a remote backend, because `RetryBlobBackend` is only
applied when the active backend is not Local — nothing below retries, so
the classification is the only thing between a flaky mount and a run
concluding the data is gone.
`local_io_error` maps the network-mount family (TimedOut,
HostUnreachable, NetworkDown, ConnectionReset, StaleNetworkFileHandle)
plus Interrupted and ResourceBusy to transient. PermissionDenied,
ReadOnlyFilesystem and StorageFull stay permanent because retrying
changes nothing without an operator, and InvalidData stays permanent
because corruption is a finding worth keeping. A bad sector arrives as
an uncategorised EIO and lands there too, which is right: the useful
outcome is a finding naming the blob, not a run that waits for a disk to
heal.
## Shape of the fix
Only a genuine absence is NotFound — `NoSuchKey` on S3 GET,
`is_not_found` on S3 HEAD, HTTP 404 on Azure, `ErrorKind::NotFound` on
local. Everything else goes through the classifier, so a 403 stays
permanent rather than being retried forever.
Tested at the local layer, which is where the mapping table is dense
enough to get wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ed proposed the obvious end-to-end test — point an S3 entry at
127.0.0.1 with nothing listening, get a refused connection, expect a
transient error — and it would have failed, because `initialize()` was
the one SDK call still wrapped as a plain `internal_error`.
That is the FIRST call both jobs make, so it is what a wrong-endpoint
test actually hits: `backend_consistency` and `backend_migration` each
return `Failed` on init, and every classification added in the previous
commits sits downstream of a path the test never reaches.
Now `head_bucket` goes through `s3_domain_error` like the rest, and both
call sites route through `RunOutcome::from_domain_error`. A refused
connection or a 5xx pauses and can be resumed once the endpoint returns;
a wrong bucket or bad credentials is 4xx and stays terminal, which is
the distinction that makes pausing safe to offer at all.
No cursor at init — nothing has been scanned — so the pause resumes from
the start, which is correct rather than lossy.
Worth noting for `backend_migration`: target init runs BEFORE
`migration_readonly` is engaged, so pausing there holds no write freeze.
An operator can leave it paused indefinitely and resume when the target
comes back, with no read-only window.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 1 of docs/plan/jobs-handling-recoverable-error.md, and the blocker
for the rest of it: the engine cannot retry-then-pause until it can tell
"the provider is down" from "this data is wrong". Both arrived as
`ErrorKind::InternalError`, so the distinction survived only inside a
formatted message.
`RetryBlobBackend` was reading that message. Literally:
let msg = err.to_string().to_lowercase();
msg.contains("timeout") || msg.contains("503") || msg.contains("reset by peer")
Fragile in a specific way — an SDK reformatting its `Display` turns
retrying off with nothing failing to say so — and blind to any status
code that never made it into the text.
Adds `ErrorKind::TransientBackend` and `DomainError::is_transient()`.
One predicate, so the retry decorator and the job engine cannot classify
the same failure differently. `Timeout` counts (transient by
construction); everything else must say so explicitly. The default is
"not retryable" because that fails visibly, whereas retrying a permanent
fault burns attempts and — once the engine wires this up — holds
`migration_readonly` while it does.
A kind rather than a `transient: bool` field: 21 struct-literal sites
construct `DomainError` directly and would all have needed touching for
a change that is conceptually about classification. The plan allowed
either.
`s3_domain_error` does the classification where the status is still in
hand. Transient: 5xx, 429, and the SlowDown / RequestTimeout /
ThrottlingException codes that arrive as 400 (status alone is not
enough), plus dispatch-level I/O and timeouts. Permanent: other 4xx —
credentials, missing bucket, malformed request — and construction
failures. `ResponseError` counts as transient since truncation on the
wire is the usual cause and the attempt cap bounds being wrong.
Applied at the five S3 sites that wrap an SDK error, including
`ListObjectsV2` — `backend_consistency` fails the whole run on an
enumeration error, so a throttle midway through a large bucket should be
retryable rather than discarding the sweep.
The exhaustive `ErrorKind` match in `interfaces/errors.rs` forced the
HTTP decision, which is the right friction: 503, not 500. The request
was fine and may succeed shortly, which is what a caller needs to decide
whether to retry and what a proxy keys off to avoid caching the failure.
The substring matcher stays for now, behind the typed check, with the
deletion condition written down: it goes when every backend wrapping a
remote SDK error classifies at the point of wrapping. Removing it before
then would silently reduce retrying on the unconverted backends, which
is the worse direction. Azure is the one left, and it is queued for the
official-SDK migration anyway.
Not yet wired: `RunOutcome::PausedRetryable` (step 2) and the engine's
bounded backoff (step 3). This commit only makes the distinction
representable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5343fdda switched S3 blob enumeration from an opaque continuation token
to a hash cursor (StartAfter), per the port contract. Its fallback for a
page containing no canonical blob was wrong: it stored the full key
(`0a/junk.tmp`), stripped it to a basename (`junk.tmp`), and the next
call fed that to `object_key()` — producing `ju/junk.tmp.blob`. Wrong
shard and a doubled extension, so the resume jumped to an arbitrary
position: skipped objects, or backwards into a loop.
The cursor can only ever be a real hash, because `object_key()` is
applied to it. So instead of synthesising one, keep listing internally
until the page holds at least one blob or the bucket is exhausted. The
continuation token is used only inside the call and never escapes.
Two pathological cases cannot produce a cursor at all — `is_truncated`
with no token (protocol violation), and a run of foreign keys long
enough to buffer the bucket. Both now fail loudly. A visible job failure
beats a sweep reporting "no missing blobs" having read a fraction of
them.
Extract `hash_from_object_key` as the paired inverse of `object_key`,
with the round-trip and the rejection set under test. It also now
requires the shard to match the hash's own prefix, which the inline
filter did not check.
Precondition for the merge-join in backend_consistency (step 6 /
option A of docs/plan/derived-blobs.md), landed separately because it
is independently useful and carries the risk.
Two contract changes on BlobStorageBackend::list_blob_hashes:
1. Entries MUST be in ascending hash order. Every shipped backend
already did this — local sorts within each shard and walks 00..ff,
and since the shard IS the hash prefix that is globally sorted; S3
and Azure list lexicographically by key and blobs/<xx>/<hash> sorts
identically to <hash>. It was accidental, and a future backend
enumerating in any other order would have silently made the
merge-join emit bogus blob_missing_from_backend findings at
data_loss severity.
2. The cursor is the last hash returned, not an opaque backend token.
This is what lets a caller resume from a checkpoint it already
holds — the merge-join keeps one cursor for both the DB walk and
the backend walk instead of a compound one, which in turn means
blobs_consistency's existing cursor format survives and no paused
run is stranded.
Local already derived its position from a hash; it now emits the bare
hash instead of "<shard>/<hash>", and still accepts both legacy forms
so a run paused across this deploy resumes. The bare-shard form works
through the same path unchanged, since "3f" sorts before every 64-char
hash beginning "3f".
S3 moves from continuation_token to StartAfter, which supports this
natively. One non-obvious case handled: a page can contain only
non-canonical keys (.tmp spool files, .corrupt sidecars), which are
filtered into `unknowns`, leaving `blobs` empty — a naive
blobs.last() would return no cursor and silently end enumeration while
is_truncated said otherwise, making an audit job under-report. It now
falls back to the last key seen; StartAfter is a string comparison, so
a non-hash resume point is fine. "Cursor is a hash" constrains what
callers may synthesise, not what backends may return.
Azure is unaffected — it does not implement list_blob_hashes (TODO,
inherits the NotSupported default).
Adds the first test for enumeration at all: ordering across shards with
deliberately out-of-order inserts, complete paged traversal, and
resume from a caller-synthesised cursor.
NOT verified against real S3 — no bucket available here. The local path
is covered by the new test; the StartAfter change is reasoned from the
API contract and needs exercising against a real bucket before it is
relied on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
if target blob already exists, migration will check the blob
if header is already with the targeted key (or no key if not ciphered) no need write
otherwise write the blob (that will convert any blob with no header into the correct version)
Round 3 of benchmark-gated optimizations (benches/ROUND3.md; every change
gated by a before/after benchmark — an AFTER that did not beat its BEFORE
was to be rolled back; none needed it. Equivalence gates assert identical
row sequences / byte-identical output on every behavior-preserving rewrite):
DB hot paths (local PG16, EXPLAIN-verified):
- Web-UI listing (list_resources_paged): cursor pushed INSIDE the
folders/files UNION-ALL branches as sargable row-value comparisons with
per-branch ORDER/LIMIT + two partial expression indexes
(folder_id, LOWER(name), id). 20k-entry folder: 26.6 -> 1.3 ms/page
(19.5x); other sort modes at parity or better. New migration
20260918000000. [benches/LISTING-KEYSET.md section in ROUND3]
- Photos timeline (list_media_files): per-drive CROSS JOIN LATERAL top-N
on the timeline index, joins moved above the top-N. 50k-photo library:
97.4 -> 1.6 ms/page (55.7x). The old "LIMIT stops the scan early"
comment was refuted by EXPLAIN.
- PROPFIND sub-folders (both DAV surfaces): keyset list_folders_batch off
idx_folders_unique_name replaces COUNT(*) OVER() + LIMIT/OFFSET
(5k dirs: 79.7 -> 17.9 ms full walk, 4.5x).
Concurrency:
- Basic-auth cache single-flight (moka try_get_with): 8 concurrent DAV
connections at TTL expiry paid 8 Argon2id runs (2.6 s CPU + 8x64 MiB);
now 1 (300 ms). Failed verifications remain uncached.
- CachedBlobBackend per-hash single-flight + unique tmp names: 16
concurrent cold readers = 16 full remote downloads racing truncating
writes on ONE deterministic .tmp (corruptible cache); now 1 download
(16x less egress, 2.8x wall on a shared link) and torn files can never
be renamed into the cache.
I/O and allocations:
- Chunk-assembly reads 64K -> 512K buffers (2.3x, 8x fewer syscalls);
chunk-spool writes via BufWriter 512K (5.6x, 32x fewer syscalls).
- S3/Azure put_blob_from_bytes_unsynced overrides: dedup settle no longer
pays a HEAD probe per new chunk (2 RTT -> 1, 1.8x); Azure stops copying
every chunk (Bytes -> Body, -0.44 ms - 4 MiB alloc per 4 MiB chunk).
- Entity->DTO mapping: Arc<str> interning of closed-set display fields +
common MIMEs, 1-alloc etag/size formatting, FolderDto moves instead of
clones. File row: 11 -> 4 allocs; folder row: 11.8 -> 1 (2.1x faster).
- CardDAV REPORT: deleted dead per-contact vCard pre-generation and the
O(N^2) uid scan whose result was discarded (5k contacts: 55.7 -> 5.7 ms,
9.8x); byte-identical XML asserted.
- Search-results cache: byte weigher + 32 MiB budget
(OXICLOUD_SEARCH_CACHE_MAX_BYTES) replaces the 1000-ENTRY cap that let
~300 MiB of enriched rows sit in RSS; read latency parity.
- Dropped aws-config + aws-smithy-types (zero references; -82 dep-graph
nodes, three SDK stacks gone from every build). tokio "process" is now
an explicit feature (was enabled transitively by aws-config).
Frontend:
- Cached Intl.DateTimeFormat keyed by (locale, options) in formatDate and
4 sibling callsites: 20k dates 2612 -> 51 ms (51.6x); vitest gate
asserts output identity across locales and a 3x floor.
Validation: cargo fmt + clippy --all-features --all-targets -D warnings
clean; 518 unit + 548 integration-cfg tests green; new-shape endpoints
smoke-tested end-to-end over HTTP (all 5 listing sort modes with cursor
walks, WebDAV PROPFIND Depth-1, photos timeline, Basic-auth DAV login);
frontend npm run check clean, new vitest gates green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBsU2qEzny3A8WQUEuMNCr
read_blob_stream / read_blob_range_stream reassembled a CDC file by fetching
its chunks with `buffered(1)` — strictly sequential, so the next chunk's
backend fetch (a file `open` locally; a full request round-trip on S3/Azure)
only started after the current chunk was fully drained.
A benchmark of the exact pipeline (stream::iter(chunks).map(get).buffered(K)
.try_flatten()) showed a blind `buffered(4)` is the WRONG fix: on a local
disk it is neutral on a warm page cache and ~37% SLOWER cold, because
concurrent opens turn one sequential read into several competing random-I/O
streams over content-addressed (scattered) chunk files. The win is entirely
on remote backends, where per-chunk request latency dominates and overlapping
fetches hide it (≈ linear in K).
So the read-ahead depth is now a backend hint, not a constant:
- BlobStorageBackend::read_prefetch() default 1 (sequential; safe for local).
- S3 / Azure override to 8 (overlap GETs to hide TTFB).
- cached / encrypted / retry / migration delegate to the backend that serves
the bytes.
- Both CDC read paths use `self.backend.read_prefetch().max(1)`.
Net: local backend unchanged (no regression); remote reassembly ~4-8x faster.
Ordered `buffered` (not buffer_unordered) keeps chunks in sequence.
Bench (per-chunk fetch-latency model): buffered(1)->(4)/(8) = x3.9 / x7.8
@1ms, x4.0 / x8.1 @5ms, x4.0 / x8.0 @20ms. Local warm: 230ms@1 vs 227ms@4
(noise); local cold: 425ms@1 vs 585ms@4 (why local stays at 1).
https://claude.ai/code/session_01DCszkkU11LYxMEUWr4setK
Edouard Vanbelle