From e054987c65f785498ffaa42030c56c66a4465454 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 7 Sep 2026 21:30:51 +0200 Subject: [PATCH] fix(migration): a transient copy failure pauses instead of skipping the blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `backend_migration` tolerated a failed copy by recording a `migration_failed` finding and moving to the next blob. Correct for one corrupt object — a single bad blob must not abort a migration of millions — but wrong when the backend has simply gone away: every remaining blob then fails, each records a `data_loss` finding, and the run walks the whole space to reach a conclusion available in seconds. **The cursor is what makes skipping unsafe.** It advances to the batch's LAST hash, after the inner loop. So continuing past a transient failure lets the batch finish and the cursor move BEYOND the blob that failed, and nothing revisits it — the run ends carrying a `data_loss` finding for a blob that was never damaged, only briefly unreachable. Ed caught this reviewing a first version that tolerated N consecutive transient failures before pausing: that variant skipped up to N blobs per batch for exactly this reason. The threshold is gone. A transient failure now pauses on the FIRST occurrence. The cursor is still at the previous batch's end, so a resume re-walks the batch and retries the blob; re-copying already-present blobs is free because the walk short-circuits on them. Permanent failures keep the old tolerate-and-continue, which is what it was built for — retrying them would fail identically. `migration_readonly` stays engaged across the pause, so Cancel remains the way to release it. Cost of pausing eagerly is small: `RetryBlobBackend` has already made 4 attempts (0 / 100 / 200 / 400 ms) before the error arrives here, so a pause means the backend was unreachable for ~700 ms of trying, and Resume is one click that continues from the cursor. Co-Authored-By: Claude Opus 5 (1M context) --- .../services/backend_migration_service.rs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/infrastructure/services/backend_migration_service.rs b/src/infrastructure/services/backend_migration_service.rs index 49722f29..6edd1950 100644 --- a/src/infrastructure/services/backend_migration_service.rs +++ b/src/infrastructure/services/backend_migration_service.rs @@ -776,6 +776,58 @@ impl RecoverableJobHandler for BackendMigrationService { } } Err(e) => { + // A transient failure pauses IMMEDIATELY. Not + // after a threshold — on the first one. + // + // The cursor advances to the batch's LAST hash, + // after this loop. So continuing past a transient + // failure lets the batch finish and the cursor + // move BEYOND the blob that failed, and nothing + // revisits it: the run would carry a `data_loss` + // finding for a blob that was never damaged, only + // briefly unreachable. Ed caught this in review of + // a "tolerate N consecutive" version — that + // version skipped up to N blobs per batch for + // exactly this reason. + // + // Pausing here keeps the cursor at the PREVIOUS + // batch's end, so a resume re-walks this batch + // and retries the blob. Re-copying a few + // already-present blobs is free — the walk + // short-circuits on them. + // + // Tolerate-and-continue still applies to + // PERMANENT failures, which is what it was built + // for: one corrupt or unreadable blob must not + // abort a migration of millions, and retrying it + // would fail identically. + // + // The copy has already been retried beneath this + // (RetryBlobBackend: 3 attempts with backoff), so + // arriving here means 4 attempts failed. + if e.is_transient() { + tracing::warn!( + target: "oxicloud::migration", + event = "backend_migration.backend_unreachable", + run_id = %store.run_id(), + hash = %hash, + copied = copied_count, + error = %e, + "backend unreachable; pausing at the last checkpoint so this \ + blob is retried on resume" + ); + // `migration_readonly` stays engaged — only + // Cancel releases it. See + // `release_readonly_on_terminal_cancel`. + return RunOutcome::from_domain_error( + cursor.as_ref().map(|s| s.as_bytes()), + &format!( + "backend unreachable while copying ({copied_count} blob(s) \ + copied so far)" + ), + &e, + ); + } failed_count += 1; tracing::warn!( target: "oxicloud::migration",