Follow-up from #293 P2 (PR #360), which PINNED the current behavior of the backup waiters with unit tests. Two inconsistencies were flagged and intentionally NOT changed under that test PR; this issue tracks aligning them. Both are behavior changes and need the full engineering cycle (PLAN + adversarial review + human GO); the tests added in #360 assert the CURRENT behavior and must be updated as part of the fix.
F1 — backup inventory waiters treat ANY read error as fatal
BackupVirtualDiskClient.WaitForInventory / BackupVirtualMachineClient.WaitForInventory
(waitForBackupVirtualDiskInventory / waitForBackupVirtualMachineInventory) call options.error(err)
on any read error, with no transient-vs-permanent distinction. A transient 5xx / transport blip while
polling the inventory therefore fails the wait — the #245 class already fixed for activity and
backup_job (which retry transient reads with a bounded budget).
Proposed: retry transient reads (isTransientAPIError) with a bounded consecutive budget, like
waitForActivityCompletion; keep permanent / configured-timeout / context errors fatal.
F2 — backup-job not-found tolerance is attempt-1-only
waitForBackupJobCompletion tolerates a nil job only when count == 1 (the counter is incremented at
the top of the loop). A prior attempt — a transient read error or a RUNNING status — consumes the
tolerance, so a later legitimate eventual-consistency not-found becomes immediately fatal. This is the
FF-3 class already fixed for activity (which tracks notFoundTolerated / activitySeen independently
of the attempt counter).
Proposed: track the one-time not-found tolerance independently of the attempt counter (mirror
waitForActivityCompletion).
Refs #293
Follow-up from #293 P2 (PR #360), which PINNED the current behavior of the backup waiters with unit tests. Two inconsistencies were flagged and intentionally NOT changed under that test PR; this issue tracks aligning them. Both are behavior changes and need the full engineering cycle (PLAN + adversarial review + human GO); the tests added in #360 assert the CURRENT behavior and must be updated as part of the fix.
F1 — backup inventory waiters treat ANY read error as fatal
BackupVirtualDiskClient.WaitForInventory/BackupVirtualMachineClient.WaitForInventory(
waitForBackupVirtualDiskInventory/waitForBackupVirtualMachineInventory) calloptions.error(err)on any read error, with no transient-vs-permanent distinction. A transient 5xx / transport blip while
polling the inventory therefore fails the wait — the #245 class already fixed for
activityandbackup_job(which retry transient reads with a bounded budget).Proposed: retry transient reads (
isTransientAPIError) with a bounded consecutive budget, likewaitForActivityCompletion; keep permanent / configured-timeout / context errors fatal.F2 — backup-job not-found tolerance is attempt-1-only
waitForBackupJobCompletiontolerates a nil job only whencount == 1(the counter is incremented atthe top of the loop). A prior attempt — a transient read error or a
RUNNINGstatus — consumes thetolerance, so a later legitimate eventual-consistency not-found becomes immediately fatal. This is the
FF-3 class already fixed for
activity(which tracksnotFoundTolerated/activitySeenindependentlyof the attempt counter).
Proposed: track the one-time not-found tolerance independently of the attempt counter (mirror
waitForActivityCompletion).Refs #293