Files
felhom-controller/controller/internal/quiesce/contention.go
T
admin 079265ad8e F-CRIT-1 + F-A1: one alarm that never fired, one that fired wrongly (v0.179.0)
F-CRIT-1 — an app that failed to restart after a quiesce never alarmed, for two
independent reasons, either of which alone kept it dead:
  1. restartAll returned nothing, so the failure was logged and dropped and no
     caller could learn a customer's app had not come back. It now returns the
     stacks that failed; both call sites record the outcome.
  2. classifyRunStates whitelists StateStopped on invariant I1 ('StateStopped
     means the user stopped it'). The quiesce loop stops stacks by the same
     compose-down path, so a failed restart is also StateStopped and was
     whitelisted into silence. Loop.FailedRestarts() is now the only thing that
     lifts the whitelist, so genuine user stops stay silent (v0.164.0 pinned).

F-A1 — HTTP 409 is the agent's single-flight gate refusing while a restore-test
holds it, not a failure. agentapi now returns a typed *StatusError on POST, the
adapter maps 409 -> quiesce.ErrTierBusy, and the loop defers: no breaker, no
event, no operator email, tier stays DUE.

Two traps avoided. Silence: contention outliving contentionAlarmAfter (3h, set
by the agent's own 120m PBS restore-test ceiling) raises its own BLOCKED signal.
App thrash: removing the failure treatment also removes the breaker's deferral,
so a contended tier is dropped BEFORE anything stops (contentionRetryAfter 15m,
against a 12m01s longest observed restore-test).

Three comments corrected; the invariant rule added to both CLAUDE.md copies.
Six red-proofs, all observed failing.
2026-07-28 08:50:11 +02:00

147 lines
6.8 KiB
Go

package quiesce
import (
"errors"
"sync"
"time"
)
// F-A1 — contention is not failure, but unending contention is.
//
// THE BUG THIS EXISTS TO KILL. The agent refuses a backup with HTTP 409 while a restore-test holds
// its R-85 single-flight gate. That refusal is the gate working exactly as designed — nothing is
// broken, the backup simply cannot run this minute. The controller's start path had no 409 branch,
// so it called noteTierFailure: the R-88 breaker armed and `whole_guest_backup_failed` was emailed
// to the operator. Campaign 8 saw it on BOTH boxes in the same minute.
//
// Two comments in the tree asserted the opposite and were wrong:
// - inflight.go (agent): "A caller that cannot acquire DEFERS to its next cadence" — the backup
// caller did not defer, it recorded a failure.
// - quiesce.go: framed the agent's 409 as the thing that PREVENTS "a spurious failure" — on the
// start path it produced one.
//
// FREQUENCY IS WHY IT MATTERS. Campaign 8's rate was an artifact of compressed cadences, but at real
// cadences a ~12-minute restore-test against a daily backup collides on the order of once per 420
// guest-days — roughly every 4 days on a 100-guest fleet, forever. An alarm that cries wolf on a
// schedule trains the operator to ignore it, which quietly undoes R-97a.
//
// ── THE TRAP THIS DELIBERATELY AVOIDS ────────────────────────────────────────────────────────
//
// "Just ignore 409" would trade a false alarm for a SILENCE PATH: a wedged restore-test that never
// releases the gate would mean the backup never runs and nobody is ever told. That is the exact
// class of fault this whole arc has been closing. So contention DEFERS, and persistent contention
// ALARMS — see contentionAlarmAfter.
//
// ── AND THE SECOND TRAP: APP THRASH ──────────────────────────────────────────────────────────
//
// Removing the failure treatment also removes the R-88 breaker's deferral, which is what had been
// (accidentally) stopping the loop from re-quiescing every 5 minutes. Without a replacement, the
// customer's apps would be stopped and restarted on EVERY poll for the whole duration of a
// restore-test — strictly worse than the bug being fixed. So a contended tier is dropped from the
// due set BEFORE anything is stopped, exactly as R-88 does for failures.
const (
// contentionRetryAfter is how long a contended tier is skipped before trying again.
//
// Measured against the real thing, not picked round: restore-test durations observed on the
// fleet during Campaign 8 were 1m29s, 6m09s, 7m20s and 12m01s, and the agent's own wait on a
// LOCAL restore-test task is 10m. 15m therefore clears the realistic case in a single hop while
// capping app-stop churn during a long restore-test at 4/hour instead of 12/hour.
//
// It is deliberately NOT longer: the tier stays DUE throughout, and a needlessly long skip just
// delays a backup that could have run.
contentionRetryAfter = 15 * time.Minute
// contentionAlarmAfter is the point at which contention stops being normal and becomes a fault
// worth an operator's attention.
//
// Bounded by the agent's OWN ceiling rather than by taste: the PBS restore-test restore task is
// capped at 120 minutes (`config.RestoreTestPBSRestoreTimeout`, default 120m), after which the
// agent times out and releases the gate itself. Contention lasting longer than that cannot be a
// legitimate restore-test — something has leaked the gate. 3h = that 120m ceiling plus an hour
// of margin for teardown and the 5-minute poll granularity, and 15x the longest contention
// actually observed (12m01s).
contentionAlarmAfter = 3 * time.Hour
)
// ErrTierBusy is the loop's vocabulary for "the agent refused because a heavy operation is already
// in flight" (HTTP 409). The adapter translates the transport-layer status into this, the same way
// it translates a 404 on /backup/tiers into ErrTiersUnsupported — `internal/quiesce` keeps no
// dependency on `internal/agentapi`.
var ErrTierBusy = errors.New("quiesce: tier busy — a concurrent heavy operation holds the agent")
// contentionState is one tier's current run of refusals.
type contentionState struct {
since time.Time // first 409 of this run
until time.Time // skip the tier until here
alarmed bool // the contentionAlarmAfter signal has already fired for this run
}
// contentionTracker records per-target 409 runs. Like the R-88 breaker it is deliberately
// IN-MEMORY: a controller restart re-attempts immediately, which is the cheap direction to fail —
// forgetting a contention window costs one extra attempt, whereas persisting it could carry a stale
// "busy" verdict across a restart that actually cleared the gate.
type contentionTracker struct {
mu sync.Mutex
states map[string]contentionState
}
func newContentionTracker() *contentionTracker {
return &contentionTracker{states: map[string]contentionState{}}
}
// note records a refusal for target. It returns how long this run of contention has lasted, and
// whether THIS call is the one that crosses contentionAlarmAfter (true exactly once per run, so the
// operator is told once rather than every retry — the same edge-triggering rule as R-97a).
func (c *contentionTracker) note(target string, now time.Time) (elapsed time.Duration, alarmNow bool) {
c.mu.Lock()
defer c.mu.Unlock()
st, ok := c.states[target]
if !ok || st.since.IsZero() {
st.since = now
}
st.until = now.Add(contentionRetryAfter)
elapsed = now.Sub(st.since)
if elapsed >= contentionAlarmAfter && !st.alarmed {
st.alarmed = true
alarmNow = true
}
c.states[target] = st
return elapsed, alarmNow
}
// blocked reports whether target is inside its contention skip window, and until when.
func (c *contentionTracker) blocked(target string, now time.Time) (time.Time, bool) {
c.mu.Lock()
defer c.mu.Unlock()
st, ok := c.states[target]
if !ok || st.until.IsZero() || !now.Before(st.until) {
return time.Time{}, false
}
return st.until, true
}
// clear ends a run of contention. Returns true if there was one, so the caller can log the recovery
// without narrating every healthy cycle. Called on ANY non-409 outcome — success or a real failure —
// because either proves the gate is no longer holding us.
func (c *contentionTracker) clear(target string) bool {
c.mu.Lock()
defer c.mu.Unlock()
if _, ok := c.states[target]; !ok {
return false
}
delete(c.states, target)
return true
}
// contendedFor exposes the current run length (tests + diagnosis).
func (c *contentionTracker) contendedFor(target string, now time.Time) time.Duration {
c.mu.Lock()
defer c.mu.Unlock()
st, ok := c.states[target]
if !ok || st.since.IsZero() {
return 0
}
return now.Sub(st.since)
}