cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat(azure): enumerate blobs, and fail instead of degrading when that breaks

Browse Source 
`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>
This commit is contained in:
Edouard Vanbelle
2026-09-02 19:24:32 +02:00
parent 1800fa9a47
commit 2b52d233f0
4 changed files with 665 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.cargo/audit.toml
+27
View File
@@ -51,6 +51,33 @@ ignore = [
# No direct security impact; no upgrade path exists.
"RUSTSEC-2024-0384",
# azure_core 0.21.0 writes the `authorization` header value to logs —
# `policies/transport.rs` does `debug!("…{request:#?}")` over the whole
# request. For a SharedKey entry that value is the per-request HMAC
# signature; for a SAS entry it is the token. Severity 6.5 (medium).
#
# The advisory says "upgrade to >=0.22.0". That version does not exist:
# `azure_core` jumped 0.21 → 1.x, and `azure_storage_blobs` never left
# 0.21.0 before being archived. So the stated remedy IS the official-SDK
# migration, tracked separately alongside the quick-xml pair above.
#
# Not reachable at our log levels: the line is `debug!` on the
# `azure_core::policies::transport` target, and the default filter is
# `info`. It fires only if an operator explicitly asks for
# `RUST_LOG=…,azure_core=debug`, which is not hypothetical — that is the
# invocation used to diagnose the Azurite migration hang. **Do not run
# `azure_core=debug` against a real Azure account**; it prints request
# signatures to the terminal. Against Azurite it only exposes the
# published dev key's signatures.
#
# A subscriber-level directive pinning that target off was prototyped
# and rejected 2026-09-02 — not worth carrying a filter hack for a
# dependency being replaced.
#
# Un-ignore trigger: the azure_storage_blob 1.x migration lands
# (`cargo tree -i azure_core@0.21` returns no rows).
"RUSTSEC-2026-0275",
# quick-xml 0.31.0 — transitive via azure_core 0.21.0 (unofficial SDK,
# now archived). Our direct dep is already on 0.41.0; the 0.31 copy is
# only reachable through the azure_storage_blobs chain, which parses
docs/plan/jobs-handling-recoverable-error.md
+265
View File
@@ -0,0 +1,265 @@
# Recoverable errors in jobs — retry, then pause
**Status: not started.** Design settled 2026-08-31, from a live
diagnosis (see [Motivating incident](#motivating-incident)).
A job that hits a failing backend today has two possible endings, and
neither is right for an outage: it fails the run (throwing away a
partially-complete migration, since `Failed` is terminal and only
`Paused` resumes), or it hangs forever inside an SDK retry loop with no
log line and no way to act on it.
This plan adds the third: **retry a bounded number of times, then pause
with the reason recorded**, so an operator resumes when the provider
recovers and the job continues from its cursor.
---
## Motivating incident
`backend_migration ?storage=azurite` hung indefinitely. Diagnosis, after
several wrong theories:
- The job issued a ranged GET carrying `x-ms-range-get-content-crc64`.
- Azurite answered **500** (real Azure supports CRC64 range validation;
the emulator does not).
- `azure_core`'s retry policy classifies 500 as retryable and loops.
- The response was deterministic, so every retry failed identically.
- The job never advanced, never failed, and emitted no per-blob line —
while holding `migration_readonly`, refusing writes **across the whole
application**.
The exact chain was pinned down later (2026-09-02) and is worth having,
because it is not where you would look — the copy itself is innocent:
```
backend_migration_service.rs target.head_check(hash) ← pre-write probe
→ EncryptedBlobBackend::head_check
→ get_blob_range_stream(hash, 0, HEADER_SIZE) ← ~40 bytes
→ azure_core Range::as_headers ← adds the CRC64
(src/request_options/range.rs: any range < 4 MiB) header, no opt-out
```
`copy_blob` reads from the SOURCE, which is local in a local→Azure
migration, so it never touches an Azure range. What hangs is the format
probe against the TARGET, on the first blob, before a byte is copied.
A workaround exists — unranged `get()` for small requests, truncate
client-side — and was **rejected 2026-09-02**: it pays for an emulator
with production read amplification and puts new offset arithmetic on the
read path. See the note on `AzureBlobBackend::get_blob_range_stream`.
The fix is the official SDK, where `range_get_content_crc64` is an
explicit field. **This plan is unaffected either way** — a bounded retry
would have turned the hang into a Paused run with a reason, which is the
point.
Two properties made it invisible: the 500 was only visible at
`azure_core=debug`, and nothing bounded the retry. The same shape would
occur against real Azure or S3 on any persistent 5xx; it is not an
emulator quirk. `AzureBlobBackend` configures no retry policy and no
timeout at all — `grep "retry\|timeout\|ClientOptions"` on
`azure_blob_backend.rs` returns nothing.
Full evidence chain, including the theories ruled out and what each
cost, is in the memory note `bug-azure-put-blob-hangs-no-timeout`.
---
## Why this belongs at the top level
Every recoverable job goes through `run_or_resume`, which already owns
the run lifecycle: it opens the row, persists and restores
[`JobRunArgs`](./job-registry.md) (`63820c3c`), dispatches the handler,
and writes the terminal state. Retry-and-pause is the same kind of
concern — policy about *how a run behaves*, not about what any one job
does.
Implemented there, `backend_rotate`, `transcode_import`, the thumbnail
imports and anything added later inherit it. Implemented per-job, it
gets written once per job and drifts.
It also survives the pending official-Azure-SDK migration untouched,
where per-SDK retry tuning would have to be redone.
**The blocker is that the engine cannot currently act on what it is
told.** A handler returns `RunOutcome::{Completed, Paused, Failed}`, so
a transient backend error is already flattened into `Failed` before the
engine sees it — "the provider is down" and "this data is wrong" are
indistinguishable. Closing that is what makes a top-level
implementation possible, and it is step 1.
---
## Step 1 — errors say whether they are retryable
Today both backends wrap SDK errors into
`DomainError::internal_error("Azure", format!("…{e}"))`, so the status
code survives only inside a formatted string. Recovering it means
string-matching, which is exactly the kind of fragility that turns into
a silent behaviour change when an SDK reformats its `Display`.
Carry the distinction on the type instead — a `retryable` flag, or a
kind the storage ports set deliberately.
**Retryable** (environment, may clear on its own):
- HTTP 5xx, 429 / `SlowDown` / throttling
- connect timeouts, connection resets, DNS failure
**Permanent** (will fail identically forever):
- 4xx other than 429 — 401/403 (credentials), 404 (missing container)
- decode failures, checksum mismatch
- `operation_not_supported`
> **The Azurite 500 is a permanent error wearing a retryable status
> code.** No classification by status alone gets this right, which is
> the case for a bounded cap rather than "retry until it works". The cap
> is the safety net for exactly the errors the taxonomy misjudges.
**Do not double-retry.** The AWS SDK already retries internally with its
own backoff, so a second layer above it multiplies. Check what the S3
backend inherits before adding anything, and consider making the
engine's cap the *outer* bound with SDK retries reduced or disabled.
## Step 2 — an outcome the engine can act on
`RunOutcome` grows a variant meaning "the environment failed, this is
worth trying again later":
```rust
RunOutcome::PausedRetryable { cursor: Vec<u8>, reason: String }
```
Distinct from all three existing outcomes, and the distinction is the
point:
| outcome | meaning | resumes? |
|---|---|---|
| `Failed` | the data or the request is wrong | no — terminal |
| `Paused` | an operator asked it to stop | yes |
| `PausedRetryable` | the environment failed | yes, and says why |
The row lands as `Paused` either way, so resume works unchanged. What
differs is `error_message`, which must let the panel — and an operator —
tell "I paused this" from "the provider went down". Without that
distinction a paused run is an unexplained one.
## Step 3 — the engine implements the policy
In `run_or_resume`:
- bounded exponential backoff, ~5 attempts
- **log each failed attempt at `warn` on our side.** The incident took
several runs to diagnose because the 500 was visible only at
`azure_core=debug`. One line per exhausted operation, naming status,
target and attempt count.
- on exhaustion, write the row as `Paused` with the reason in
`error_message`
Handlers then return the retryable outcome and get the policy for free.
---
## Step 4 — `migration_readonly`, the sharp edge
`backend_migration` holds a gate that refuses writes **application-wide**
until cutover. What happens to it on pause is a correctness question,
not a cosmetic one.
### Start conservative: keep the gate held while paused
Correct, because no writes during the pause means the cursor stays valid
and resume-from-cursor is sound.
It is also **strictly better than today**, which is the thing to
remember when the read-only window looks unattractive: the application
is *already* read-only while the job hangs — there is simply no way to
see why or act. Same lock, now with a reason and two operator actions.
**Cancel must clear the gate.** It ends the run with no swap, so the
source stays active and writes must return. The `failed > 0` path
already clears readonly on the reasoning that "users shouldn't be locked
out because of a partial run" — cancel is the escape hatch operators
will reach for during an outage, and it must work.
**Make the state loud.** Writes refused app-wide should be obvious in
the admin panel, not discovered by reading a run row. The progress
snapshot already feeds the header middleware; a paused migration wants
the same visibility, saying why and offering resume/cancel.
### Why NOT to release the gate on pause (yet)
Tempting — ops should not be locked out during a provider outage — and
unsafe as the job stands:
1. Release readonly; users write again. New blobs land on the source,
which is still active, and are absent from the target.
2. Resume continues **from the cursor**, a position in a hash-ordered
walk.
3. A blob written during the pause whose hash sorts *before* that cursor
is never visited.
4. The run completes, flips the pointer, and reads for that hash 404
against a target that never received it.
This is why the existing `failed > 0` path can safely clear readonly: it
**ends** the run, and an operator retrying starts a new run with a fresh
cursor, so everything is rescanned. Pause-and-resume is what
reintroduces the gap.
Releasing on pause becomes safe only alongside one of:
- **resume rescans from the beginning** rather than trusting the cursor
— cheap, because the walk short-circuits on blobs already present in
the target, so a second pass costs a lookup per blob, not a copy; or
- **a final catch-up pass under readonly before the swap**, with the
pointer flipping only when a complete pass finds nothing new.
### Where this should end up
The second option is the standard online-migration shape, and it is
where this job wants to go regardless: copy the bulk **without** holding
the gate, engage it only for a short final catch-up plus the swap.
That removes what made the incident damaging — writes refused app-wide
for the entire duration of a long copy — and turns releasing-on-pause
into a free consequence rather than a correctness fix. Worth doing as a
follow-up, from the safer base this plan establishes.
---
## Scope: Azure and S3 both
The only reason S3 looks healthy is that the endpoint behaves. A
persistent 5xx from S3 hangs identically — the gap is the absence of a
bound, not anything Azure-specific.
Putting the policy above the `BlobStorageBackend` trait covers Azure,
S3, local and anything added later with one implementation. Per-backend
SDK retry tuning stays a separate, optional refinement — and for Azure
specifically it should wait for the official SDK, since `azure_core`
0.21 is archived and queued for replacement.
## Testing
The `backend_consistency_azure.hurl` scenario and its Azurite service
are already wired (`tests/common/docker-compose.test.yml`,
`spawn-db.sh`), which gives a backend that reliably produces the
failure: Azurite 500s on the CRC64 ranged GET every time. That makes it
a genuine fixture for this work rather than a flake —
**deterministically unretryable-but-retryable-looking**, which is the
hard case.
Note the scenario no longer triggers a migration — it audits Azurite
without cutting over, for the reason in its header. Reaching the fixture
means triggering `backend_migration ?storage=azurite` explicitly, which
is exactly the hang this plan is meant to convert into a Paused run.
Doing that inside the shared suite is what ordering it last was for; it
can go back once the outcome is bounded.
Assert the run reaches `Paused`, that `error_message` names the cause,
and that it does so in bounded time rather than hanging.
`POST /api/admin/settings/storage/test` with `entry_name` is the
pre-flight worth keeping in mind: synchronous, does a real write/read
round-trip, and isolates "the backend is misconfigured" from "the job
is broken". It passing while the migration hung is what ruled out
credentials, container and the write path during the incident.
src/infrastructure/services/azure_blob_backend.rs
+318 -11
View File
@@ -13,7 +13,8 @@ use futures::{StreamExt, TryStreamExt};
use tokio::fs;
use crate::application::ports::blob_storage_ports::{
BlobStorageBackend, BlobStream, StorageHealthStatus,
BackendBlobEntry, BackendUnknownEntry, BlobListPage, BlobStorageBackend, BlobStream,
StorageHealthStatus,
};
use crate::common::config::AzureStorageConfig;
use crate::domain::errors::{DomainError, ErrorKind};
@@ -55,6 +56,29 @@ impl AzureBlobBackend {
}
}
/// Inverse of [`Self::blob_name`] — the hash a listing entry names,
/// or `None` when the entry is not one of ours.
///
/// Mirrors `S3BlobBackend::hash_from_object_key`, including the check
/// that the shard equals the hash's own first two characters: without
/// it, `blob_name(hash)` would not reproduce the name we just parsed,
/// and a mis-sharded object would be reported as a live blob that no
/// read path can find.
fn hash_from_blob_name(name: &str) -> Option<String> {
let (prefix, rest) = name.split_once('/')?;
if prefix.len() != 2 || !prefix.chars().all(|c| c.is_ascii_hexdigit()) {
return None;
}
let stem = rest.strip_suffix(".blob")?;
if stem.len() != 64 || !stem.chars().all(|c| c.is_ascii_hexdigit()) {
return None;
}
if !stem.starts_with(prefix) {
return None;
}
Some(stem.to_string())
}
/// Compute the blob name for a given hash.
fn blob_name(hash: &str) -> String {
let prefix = &hash[0..2];
@@ -243,6 +267,37 @@ impl BlobStorageBackend for AzureBlobBackend {
})
}
/// # Known incompatibility: sub-4 MiB ranges break on Azurite
///
/// `azure_core` 0.21's `Range::as_headers`
/// (`src/request_options/range.rs`) attaches
/// `x-ms-range-get-content-crc64: true` to **any range shorter than
/// 4 MiB**, unconditionally and with no opt-out. Real Azure honours
/// it; Azurite answers 500. `azure_core` then classifies 500 as
/// retryable and loops on a deterministic error, forever.
///
/// The reachable path is `backend_migration` →
/// `EncryptedBlobBackend::head_check` →
/// `get_blob_range_stream(hash, 0, HEADER_SIZE)`. `HEADER_SIZE` is a
/// few dozen bytes, and it runs against the TARGET before each write,
/// so a local→Azurite migration hangs on its first blob while holding
/// `migration_readonly` — writes refused application-wide.
///
/// **Deliberately not worked around here.** The available workaround
/// is to issue an unranged `get()` for small requests (its 16 MiB
/// `initial_range` clears the threshold, so the header is never sent)
/// and truncate client-side. That is correct against real Azure but
/// pays for an emulator with production cost: a ~40-byte format probe
/// becomes a whole-blob transfer, and it puts new offset arithmetic
/// on the read path, where a mistake serves wrong bytes silently
/// rather than failing.
///
/// The real fix is the official `azure_storage_blob` 1.x, where
/// `range_get_content_crc64` is an explicit field on
/// `BlobClientDownloadOptions` — leave it unset and the request is
/// never made. Until then, the Azurite suite exercises enumeration
/// and round-trips but not migration; see
/// `tests/api/backend_consistency_azure.hurl`.
fn get_blob_range_stream(
&self,
hash: &str,
@@ -394,6 +449,197 @@ impl BlobStorageBackend for AzureBlobBackend {
})
}
/// Enumerate blob hashes in lexicographic order, so
/// `backend_consistency` can merge-join against `storage.blobs`
/// instead of degrading to a per-row probe that structurally cannot
/// see orphans.
///
/// ## Why this is a shard walk and not one flat listing
///
/// **The cursor IS a blob hash**, not a provider token. The caller
/// forces that: it advances ONE cursor across both sides of the join,
/// feeding the same value here and to `WHERE hash > $1` in SQL. S3
/// satisfies it with `start_after(object_key(cursor))`.
///
/// Azure has no `StartAfter`. REST API 2023-05-03 added `startFrom`,
/// which would be the direct equivalent — but this SDK
/// (`azure_storage_blobs` 0.21, archived) never sends it: `ListBlobs`
/// exposes only `prefix`, `delimiter`, `max_results` and `marker`,
/// and `marker` is an opaque continuation token that cannot be
/// derived from a hash.
///
/// So 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. Walking
/// `00/` … `ff/` therefore yields exactly the global hash order, and
/// a cursor names the shard to restart in. Re-listing on resume is
/// bounded by shard width — 1/256th of the container — rather than
/// by the whole container, which is what a client-side skip over a
/// flat listing would cost on every single page.
///
/// `marker` is used only INSIDE one call, to page within a shard, and
/// never escapes as the cursor — the same treatment the S3 impl gives
/// its continuation token.
///
/// ## What this does NOT see, unlike S3
///
/// S3 lists the bucket with no prefix, so any foreign object lands in
/// `unknowns`. Constraining to `{2-hex}/` means foreign names outside
/// that shape are invisible here.
///
/// That asymmetry is deliberate and safe in the direction that
/// matters: an orphan is a blob **we** wrote and later stopped
/// referencing, so it always has the canonical name and is always
/// enumerated. Only genuinely foreign files — another workload
/// sharing the container — can be missed, and they are informational
/// notices, never findings. Trading them for O(N) enumeration instead
/// of O(N²/limit) is worth it.
fn list_blob_hashes(
&self,
cursor: Option<String>,
limit: usize,
) -> Pin<Box<dyn std::future::Future<Output = Result<BlobListPage, DomainError>> + Send + '_>>
{
Box::pin(async move {
// A run of foreign entries can't produce a resume cursor, and
// buffering the container to find one blob is worse than
// failing. Mirrors the S3 impl's bound, and like it is on
// entries accumulated rather than requests made: request
// count scales with the caller's `limit`, so a request cap
// would fire on a healthy container merely because the caller
// paged finely.
const MAX_UNKNOWNS: usize = 10_000;
// A shard is `{2-hex}/`, so the space is 0x00..=0xff.
const LAST_SHARD: u16 = 0xff;
let mut blobs: Vec<BackendBlobEntry> = Vec::new();
let mut unknowns: Vec<BackendUnknownEntry> = Vec::new();
// Resume in the cursor's own shard — its remaining entries
// still sort after it, and the client-side skip below drops
// the ones that don't. A malformed cursor is a bug in the
// caller's checkpoint, and silently restarting from `00`
// would re-report every blob as new, so refuse it.
let mut shard: u16 = match cursor.as_deref() {
Some(c) => u16::from(u8::from_str_radix(c.get(0..2).unwrap_or(""), 16).map_err(
|_| {
DomainError::internal_error(
"Blob",
format!(
"Azure enumeration cursor '{c}' is not a blob hash — it must \
start with the two hex characters naming its shard"
),
)
},
)?),
None => 0,
};
// Azure caps a page at 5000; asking for the caller's `limit`
// keeps a small page cheap. `MaxResults` rejects zero, and a
// caller asking for nothing still needs a well-formed
// request — and, more importantly, must not be answered with
// an empty page and a `None` cursor, which would read as
// "container fully enumerated, nothing here".
let want = limit.max(1);
let page_size = want.min(5000) as u32;
'shards: while shard <= LAST_SHARD {
let prefix = format!("{shard:02x}/");
// `Pageable` follows `next_marker` itself, so one stream
// covers the whole shard however many round-trips it takes.
let mut pages = self
.container_client
.list_blobs()
.prefix(prefix)
.max_results(std::num::NonZeroU32::new(page_size).expect("clamped above 0"))
.into_stream();
while let Some(page) = pages.next().await {
let page = page.map_err(|e| {
DomainError::internal_error(
"Blob",
format!(
"Azure ListBlobs failed on shard {shard:02x} of container '{}': {e}",
self.container_name
),
)
})?;
for blob in page.blobs.blobs() {
let name = blob.name.clone();
// `OffsetDateTime` → chrono, for the caller's
// grace window. A value outside chrono's range
// degrades to `None`, which the port documents as
// "treat as old enough" — the conservative side,
// since it only ever suppresses a finding on a
// freshly-written blob.
let mtime = chrono::DateTime::<chrono::Utc>::from_timestamp(
blob.properties.last_modified.unix_timestamp(),
blob.properties.last_modified.nanosecond(),
);
match Self::hash_from_blob_name(&name) {
Some(hash) => {
// `prefix` is inclusive of the cursor's own
// entry and of everything before it in the
// shard. Without this skip the caller sees
// a hash it already consumed and the
// merge-join never advances past it.
if cursor.as_deref().is_some_and(|c| hash.as_str() <= c) {
continue;
}
blobs.push(BackendBlobEntry { hash, mtime });
}
// Not ours — a foreign workload sharing the
// container. Surfaced rather than dropped so
// an operator can see it.
None => unknowns.push(BackendUnknownEntry { path: name, mtime }),
}
}
if blobs.len() >= want {
break 'shards;
}
if unknowns.len() >= MAX_UNKNOWNS {
return Err(DomainError::internal_error(
"Blob",
format!(
"Azure enumeration accumulated {} non-blob entrie(s) without \
filling a page, so no resume cursor can be produced. Container \
'{}' likely holds a large foreign namespace — give OxiCloud a \
dedicated container.",
unknowns.len(),
self.container_name,
),
));
}
}
shard += 1;
}
// Exhausting every shard is the ONLY end of enumeration.
// Stopping early because one shard was empty would truncate
// the sweep and report the rest of the container as absent,
// so `shard > LAST_SHARD` — not "this page was empty" — is
// what produces `None`.
let next_cursor = if shard > LAST_SHARD {
None
} else {
blobs.last().map(|entry| entry.hash.clone())
};
Ok(BlobListPage {
blobs,
unknowns,
next_cursor,
})
})
}
fn backend_type(&self) -> &'static str {
"azure"
}
@@ -406,14 +652,75 @@ impl BlobStorageBackend for AzureBlobBackend {
fn local_blob_path(&self, _hash: &str) -> Option<PathBuf> {
None
}
// TODO: implement `list_blob_hashes` via
// `container_client.list_blobs()` (`azure_storage_blobs`
// paginator). Same filter as local + S3 impls:
// `<xx>/<64-hex>.blob` naming. Currently inherits the trait
// default which returns `operation_not_supported` — the
// `backend_consistency` tenant handles that by emitting a
// single run-level `backend_unenumerable` finding and
// completing without per-blob probes. Ship as a follow-up once
// there's an Azure test environment to validate against.
}
#[cfg(test)]
mod tests {
use super::*;
const H: &str = "0a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f9";
/// The enumeration cursor is fed straight back in as a shard prefix,
/// so a name that does not round-trip would resume in the wrong shard
/// and silently skip everything between.
#[test]
fn blob_name_round_trips_through_hash_from_blob_name() {
let name = AzureBlobBackend::blob_name(H);
assert_eq!(name, format!("0a/{H}.blob"));
assert_eq!(
AzureBlobBackend::hash_from_blob_name(&name).as_deref(),
Some(H)
);
}
/// Each of these would otherwise be treated as a hash — and the
/// resume path slices `[0..2]` off it to pick the next shard.
#[test]
fn non_canonical_names_are_rejected() {
let cases = [
"0a/junk.tmp".to_string(), // spool file
"junk.tmp".to_string(), // no shard
"0a/junk".to_string(), // no suffix
"thumbnails/abc.jpg".to_string(), // foreign namespace
format!("0a/{H}.blob.corrupt"), // sidecar
format!("0a/{H}"), // suffix missing
format!("zz/{H}.blob"), // non-hex shard
format!("ff/{H}.blob"), // shard != hash prefix
format!("0a/{}.blob", &H[..63]), // wrong length
];
for name in &cases {
assert_eq!(
AzureBlobBackend::hash_from_blob_name(name),
None,
"must not be read as a blob: {name}"
);
}
}
/// The shard walk relies on `{hash[0..2]}/…` ordering lexicographic
/// names into exactly the order `ORDER BY hash` produces. If the
/// shard were not the hash's own prefix the two sequences would
/// interleave differently and the merge-join would emit phantom
/// findings in BOTH directions.
#[test]
fn shard_order_matches_hash_order() {
let hashes = ["00aa", "0a1b", "0aff", "b0cd", "ffff"]
.map(|p| format!("{p}{}", "0".repeat(60)))
.to_vec();
let mut names: Vec<String> = hashes
.iter()
.map(|h| AzureBlobBackend::blob_name(h))
.collect();
names.sort();
let recovered: Vec<String> = names
.iter()
.filter_map(|n| AzureBlobBackend::hash_from_blob_name(n))
.collect();
let mut sorted_hashes = hashes.clone();
sorted_hashes.sort();
assert_eq!(recovered, sorted_hashes);
}
}
src/infrastructure/services/backend_consistency_service.rs
+55 -54
View File
@@ -49,15 +49,38 @@
//! that ever admitted uppercase or variable length would break this
//! silently and in both directions at once.
//!
//! ### Run-level check
//! ### When enumeration fails
//!
//! * `backend_unenumerable` (severity `anomaly`) — the backend
//! returned `operation_not_supported` on the first
//! `list_blob_hashes` call. Currently this fires when a
//! `MigrationBlobBackend` is active (refuses enumeration
//! mid-migration by design) or on an Azure backend (Azure impl
//! deferred). Informational — operators know they can't rely on
//! this scan under that config.
//! **The run fails.** There is no degraded mode.
//!
//! There used to be: an error on the first `list_blob_hashes` call
//! emitted a `backend_unenumerable` anomaly and fell back to
//! `probe_each_row`, one `blob_exists` per `storage.blobs` row. That
//! recovered `blob_missing_from_backend` (the direction that loses
//! FILES) but never `orphan_blob`, since bytes no row claims are
//! invisible to anything starting from the database.
//!
//! It was written for two cases, and neither exists:
//!
//! * **Azure** — enumerates since the 256-way shard walk (see
//! `AzureBlobBackend::list_blob_hashes` for why it needs one to do
//! what S3 gets from `StartAfter`).
//! * **Mid-migration** — never applied. That justification named a
//! `MigrationBlobBackend` that does not exist;
//! `SwappableBlobBackend::list_blob_hashes` forwards to whatever is
//! currently active, as do the Encrypted, Cached and Retry wrappers.
//! Do not reintroduce the claim without grepping for the impl.
//!
//! So the only thing still reaching it was a *transient* failure — auth
//! blip, throttle, network — being relabelled as a capability limit and
//! silently costing orphan coverage. A failed run is louder than an
//! anomaly on an otherwise-clean-looking scan, which was the fallback's
//! own stated goal.
//!
//! The trait default still returns `operation_not_supported`, so a
//! future write-only or read-only-mirror backend would fail every run
//! here. **That is when the fallback should come back — with tests.**
//! It had none, which is the other half of why it went.
//!
//! ### Grace window
//!
@@ -116,10 +139,6 @@ const BATCH_SIZE: usize = 500;
/// `blobs_consistency` + `dedup_gc`.
const CREATE_GRACE: Duration = Duration::hours(1);
/// Cap on affected-blob examples surfaced in the run-level
/// `backend_unenumerable` finding. Keeps the finding detail bounded.
const _MAX_EXAMPLES: usize = 5;
pub struct BackendConsistencyCheck {
pool: Arc<PgPool>,
/// Default backend to enumerate when `args.storage` is `None` —
@@ -172,10 +191,12 @@ impl RecoverableJobHandler for BackendConsistencyCheck {
"Merge-joins the storage backend's blob enumeration against \
storage.blobs, both ordered by hash, so one pass yields the delta \
in both directions: bytes on the backend no DB row claims, and \
rows whose bytes are gone. Add ?deep=true to also read every \
matched blob back and re-hash it, catching silent bit-rot — that \
is a full read of storage and can take hours. Read-only in both \
modes: nothing is uploaded or deleted."
rows whose bytes are gone. If the backend cannot be enumerated \
the run fails rather than reporting partial coverage. \
Add ?deep=true to also read every matched blob back and re-hash \
it, catching silent bit-rot — that is a full read of storage and \
can take hours. Read-only in every mode: nothing is uploaded or \
deleted."
}
/// Approximate total: on a healthy install every backend blob
@@ -377,45 +398,25 @@ impl RecoverableJobHandler for BackendConsistencyCheck {
let page = match backend.list_blob_hashes(cursor.clone(), BATCH_SIZE).await {
Ok(v) => v,
Err(e) => {
// Backend refuses / can't enumerate. First-batch
// failure = we emit ONE run-level anomaly and
// complete cleanly (the run stays useful — the
// operator learns why nothing was checked
// instead of getting a red error). Mid-scan
// failure = we fail the run.
let is_first_batch = cursor.is_none() && finding_count == 0;
if is_first_batch {
// No local increment — the local
// `finding_count` is only used for the
// completion log below, but this branch
// returns immediately. The finding IS
// persisted + counted in `stats.finding_count`
// by `record_or_log` → `store.record_finding`.
record_or_log(
store,
BACKEND_CONSISTENCY_JOB_NAME,
"backend_unenumerable",
"anomaly",
None,
serde_json::json!({
"backend": backend.backend_type(),
"error": format!("{e}"),
"note": "backend refused enumeration; no per-blob orphan probes attempted",
}),
)
.await;
tracing::info!(
target: "oxicloud::consistency",
event = "backend_consistency.unenumerable",
run_id = %store.run_id(),
backend = backend.backend_type(),
"backend refused enumeration (typical during migration or on backends without list support)"
);
return RunOutcome::completed();
}
// Fail loudly, first batch or not.
//
// A first-batch failure used to degrade to
// `probe_each_row` instead. That was written for
// backends which genuinely cannot enumerate, and none
// ship today — see the module docs for why the two it
// named do not apply. What was left reaching it was a
// transient error relabelled as a capability limit, on
// a run that then looked clean while having lost orphan
// coverage entirely.
//
// Whether the enumeration died on page 1 or page 900,
// the audit did not complete, and the operator needs to
// know that rather than read a green run.
return RunOutcome::Failed {
message: format!("backend list failed mid-scan: {e}"),
message: format!(
"backend enumeration failed on {}: {e}",
backend.backend_type()
),
};
}
};