fix(migration): a transient copy failure pauses instead of skipping the blob
`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) <noreply@anthropic.com>
This commit is contained in:
@@ -776,6 +776,58 @@ impl RecoverableJobHandler for BackendMigrationService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(e) => {
|
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;
|
failed_count += 1;
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
target: "oxicloud::migration",
|
target: "oxicloud::migration",
|
||||||
|
|||||||
Reference in New Issue
Block a user