079265ad8e
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.
147 lines
6.8 KiB
Go
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)
|
|
}
|