diff --git a/CHANGELOG.md b/CHANGELOG.md index d6b368d..0813de8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ ## Changelog +### v0.175.0 — R-82: a tier that overruns the quiesce bound defers the rest (2026-07-26) + +Operator ruling 2026-07-26: *"let the first backup run as long as needed; other backups shouldn't +start until finished."* + +A first FULL offsite snapshot legitimately runs for **hours** — far past `max_quiesce`. When that +bound elapses the app resumes (correct, and unchanged), but `quiesceAndPollTiers` then moved on and +started the NEXT tier while the first was still uploading. That is now a `break`: the remaining tiers +are deferred to a later poll. + +Why it matters: vzdump still holds the guest lock, so the second start would be **refused by the +agent (409, v0.99.0)** or fail on the lock — and a failed backup never satisfies a cadence, so the +tier would stay permanently due and retry into the same wall every poll. + +`pollTier` now returns `(phase, stillRunning, err)`; `stillRunning` means the bound elapsed with the +backup still going. Nothing else changed — the app still resumes exactly once, on the same guard. + +**Tests:** `TestTierOverrunsQuiesceBound_RemainingTiersDeferred`. Red-proof observed: dropping the +`break` starts the second tier and the test fails with +`the second tier MUST NOT start while the first is still running; started=[local felhom-pbs]`. +Restored; full suite green. + + ### v0.174.0 — R-82 Slice B: one quiesce window, two backup tiers (2026-07-26) **MinAgent UNCHANGED — deliberately.** This release degrades gracefully against ANY older agent; it diff --git a/controller/internal/quiesce/quiesce.go b/controller/internal/quiesce/quiesce.go index 9b520b3..9e9e22a 100644 --- a/controller/internal/quiesce/quiesce.go +++ b/controller/internal/quiesce/quiesce.go @@ -354,16 +354,24 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { _ = l.writeMarker(marker) // best-effort: record the CURRENT tier's job id for diagnosis l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s started — polling", label, jobID) - phase, perr := l.pollTier(ctx, t.target, jobID, label, deadline, last, &unquiesced, unquiesce) + phase, stillRunning, perr := l.pollTier(ctx, t.target, jobID, label, deadline, last, &unquiesced, unquiesce) if perr != nil && firstErr == nil { firstErr = perr } if phase == phaseFailed { l.logger.Printf("[WARN] [quiesce] tier %s: backup job %s failed", label, jobID) } - // The max-quiesce guard already unquiesced; keep going so the remaining tiers still run - // (the app is up — the backups simply continue without the quiesce guarantee, which is - // strictly better than skipping the DR tier entirely). + if stillRunning { + // The max-quiesce guard fired while THIS tier's backup is still going (a first full + // offsite snapshot legitimately runs for hours). The app is already back up. We must + // NOT start the next tier: ONE BACKUP AT A TIME PER GUEST (operator ruling + // 2026-07-26) — vzdump still holds the guest lock, so a second start would be refused + // by the agent (409) or fail on the lock and record a spurious failure. The remaining + // tiers simply run on a later poll, once this one has finished. + l.logger.Printf("[INFO] [quiesce] tier %s still running past the quiesce bound — deferring %d remaining tier(s) to a later cycle", + label, len(tiers)-i-1) + break + } } // Belt: if every tier failed to start, nothing above unquiesced. The deferred call covers it, @@ -378,19 +386,21 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { // app-consistent while still costing exactly one stop/start pair. For a non-last tier the loop // waits for a TERMINAL phase (done/failed), because vzdump holds the guest lock and the next tier // cannot start until this one truly finishes. +// Returns (phase, stillRunning, err). stillRunning=true means the quiesce bound elapsed while the +// backup is STILL going — the caller must not start another tier (one backup at a time per guest). func (l *Loop) pollTier(ctx context.Context, target, jobID, label string, deadline time.Time, - last bool, unquiesced *bool, unquiesce func(string)) (string, error) { + last bool, unquiesced *bool, unquiesce func(string)) (string, bool, error) { for { if !l.now().Before(deadline) { l.logger.Printf("[WARN] [quiesce] max-quiesce-duration (%s) exceeded on tier %s (job %s) — unquiescing while the backup continues on the agent", l.maxQuiesce, label, jobID) unquiesce("max-quiesce guard") - return "", nil + return "", true, nil } phase, err := l.backupStatusOn(ctx, target) if err != nil { unquiesce("status poll failed") - return "", fmt.Errorf("poll backup status on %s: %w", label, err) + return "", false, fmt.Errorf("poll backup status on %s: %w", label, err) } switch phase { case phaseSnapshotted: @@ -408,17 +418,17 @@ func (l *Loop) pollTier(ctx context.Context, target, jobID, label string, deadli } else { l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s done — next tier may start (app still quiesced)", label, jobID) } - return phaseDone, nil + return phaseDone, false, nil case phaseFailed: if last { unquiesce("backup failed") } - return phaseFailed, nil + return phaseFailed, false, nil } select { case <-ctx.Done(): unquiesce("controller shutting down") - return "", ctx.Err() + return "", false, ctx.Err() case <-time.After(l.statusPoll): } } diff --git a/controller/internal/quiesce/tiers_test.go b/controller/internal/quiesce/tiers_test.go index ad07d65..6f67cea 100644 --- a/controller/internal/quiesce/tiers_test.go +++ b/controller/internal/quiesce/tiers_test.go @@ -397,3 +397,45 @@ func TestOldestAge(t *testing.T) { t.Fatalf("empty → nil, got %v", *got) } } + +// Operator ruling 2026-07-26: "let the first backup run as long as needed; other backups shouldn't +// start until finished." A first FULL offsite snapshot legitimately runs for hours — far past the +// quiesce bound. When that bound elapses the app resumes (correct), but the remaining tiers must +// NOT start: vzdump still holds the guest lock, so a second start would be refused by the agent +// (409) or fail on the lock and record a spurious failure that leaves the tier permanently due. +// +// COMPANION RED-PROOF (observed): drop the `break` on stillRunning → the second tier starts while +// the first is still going and this fails with +// "the second tier MUST NOT start while the first is still running; started=[local felhom-pbs]". +// Restored. +func TestTierOverrunsQuiesceBound_RemainingTiersDeferred(t *testing.T) { + be := newTierBackend() + be.tiers = []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}} + be.dueSet["local"] = true + be.dueSet["felhom-pbs"] = true + // The local tier never reaches a terminal phase — it just keeps running, like a long upload. + be.phases["local"] = []string{phaseRunningForever} + + st := &fakeStacks{running: []string{"immich"}} + l := New(Options{ + Backend: be, Stacks: st, + MarkerPath: filepath.Join(t.TempDir(), "q.json"), + StatusPoll: time.Millisecond, + MaxQuiesce: 20 * time.Millisecond, // bound elapses almost immediately + Logger: log.New(io.Discard, "", 0), + }) + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("an overrun is not an error: %v", err) + } + got := be.startedTargets() + if len(got) != 1 || got[0] != "local" { + t.Fatalf("the second tier MUST NOT start while the first is still running; started=%v", got) + } + // The app must still have come back exactly once (the max-quiesce guard). + if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 1 || starts != 1 { + t.Fatalf("the app must resume exactly once on the quiesce bound; got %d/%d", stops, starts) + } +} + +// phaseRunningForever is a non-terminal phase the fake returns indefinitely. +const phaseRunningForever = "running"