controller v0.178.0 — R-88 Part 2: only a positive 'never' fires the valve
MinAgent: 0.105.0. scheduledRunAllowed fired on any nil age; it now requires a licence from valveLicensed, which grants it for AgeStateAbsent and for a LEGACY agent, and refuses it for AgeStateUnknown. An unreadable storage no longer masquerades as a first-ever backup and no longer quiesces apps outside the window. A missing wire field means legacy, not unknown — deliberately. Treating it as unknown would stop the valve firing on un-upgraded boxes and starve genuinely new ones. Degrade logged once; unrecognised future values also map to legacy. Caught in passing: TieredBackend is satisfied by a RUNTIME assertion, so the signature change compiled and vetted clean while quiesceBackend silently stopped satisfying it — which would have degraded every box to the single-tier path with no error. Added a compile-time witness. Also corrects the notifier comment that claimed operator-only came from a missing customerMessages entry; enforcement is hub-side operatorOnlyEvents (hub 0.79.0).
This commit is contained in:
@@ -100,6 +100,8 @@ type Loop struct {
|
||||
mu sync.Mutex
|
||||
// degradeOnce reports the pre-R-82 agent fallback exactly once per process (see tiers.go).
|
||||
degradeOnce sync.Once
|
||||
// ageStateDegradeOnce reports a pre-v0.105.0 agent (no age_state) exactly once (R-88 Part 2).
|
||||
ageStateDegradeOnce sync.Once
|
||||
// breaker (R-88) defers the QUIESCE for a tier whose backups keep failing, so a broken target
|
||||
// cannot stop the customer's apps every 5 minutes forever. Scheduled path only — see breaker.go.
|
||||
breaker *failureBreaker
|
||||
@@ -224,7 +226,7 @@ func (l *Loop) runOnce(ctx context.Context) error {
|
||||
// cadence+24h" — cannot be suppressed by a fresher sibling tier.
|
||||
if l.windowStartFn != nil {
|
||||
window := l.windowStartFn()
|
||||
if !scheduledRunAllowed(l.now().In(budapestLocation()), window, oldestAge(dueTiers), l.cadence) {
|
||||
if !scheduledRunAllowed(l.now().In(budapestLocation()), window, oldestAge(dueTiers), valveLicensed(dueTiers), l.cadence) {
|
||||
from, to := gateBounds(window)
|
||||
l.logger.Printf("[DEBUG] [quiesce] scheduled backup due but outside the backup window [%s–%s) — deferring to the next poll inside it", from, to)
|
||||
return nil
|
||||
@@ -288,6 +290,31 @@ func (l *Loop) dropBackedOffTiers(tiers []dueTier) []dueTier {
|
||||
return kept
|
||||
}
|
||||
|
||||
// valveLicensed reports whether ANY due tier holds a POSITIVE claim of "never backed up" — the only
|
||||
// thing that may fire the window-gate safety valve on a nil age (R-88 Part 2).
|
||||
//
|
||||
// Two states license it, and the second is the important one:
|
||||
// - AgeStateAbsent — the agent looked and there is genuinely nothing there;
|
||||
// - AgeStateLegacy — a pre-v0.105.0 agent that cannot tell us either way. Preserving the OLD
|
||||
// behaviour is correct here: reading its silence as "unknown" would stop the valve firing on
|
||||
// every un-upgraded box, so a genuinely new box would never take its first backup outside its
|
||||
// window and nobody would notice for weeks. The MinAgent floor drives the upgrade; the valve is
|
||||
// not the place to force it.
|
||||
//
|
||||
// AgeStateUnknown does NOT license it. That is the entire fix: an unreadable storage no longer
|
||||
// masquerades as a first-ever backup.
|
||||
func valveLicensed(tiers []dueTier) bool {
|
||||
for _, t := range tiers {
|
||||
if t.ageSecs != nil {
|
||||
continue // a real age needs no licence; the age comparison decides
|
||||
}
|
||||
if t.state == AgeStateAbsent || t.state == AgeStateLegacy {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// oldestAge returns the largest (most overdue) age among the due tiers; nil when any tier has never
|
||||
// backed up (nil age = "never", which is maximally overdue and must win).
|
||||
func oldestAge(tiers []dueTier) *int64 {
|
||||
@@ -569,26 +596,28 @@ const (
|
||||
// absence of a signal was read as a specific value, and each time the fix was the same shape:
|
||||
// give "unknown" its own representation instead of letting it collapse into a real answer.
|
||||
//
|
||||
// ── WHAT IS AND IS NOT FIXED HERE ────────────────────────────────────────────────────────────
|
||||
// ── CLOSED BY R-88 PART 2 (agent v0.105.0 + controller v0.178.0) ─────────────────────────────
|
||||
//
|
||||
// The nil branch below STILL fires the valve, and that is currently correct-by-necessity, not by
|
||||
// design: the controller cannot yet tell the two apart. The agent's `/backup/due` returns
|
||||
// BYTE-IDENTICAL responses for "the storage read errored" and "there has genuinely never been a
|
||||
// backup" — same `Due: true`, same `Reason: "no successful backup recorded yet"`, same nil
|
||||
// `AgeSecs`. The root cause is agent-side: `newestArchiveOn` (localapi/server.go) documents that
|
||||
// errors "degrade to unknown, never to no-backup", but its `(time.Time, bool)` signature cannot
|
||||
// represent unknown, so the error collapses into a positive claim of "never".
|
||||
// The value is now THREE-STATE, not merely "nil or not". The agent reports `age_state` on
|
||||
// /backup/due — `known` / `absent` / `unknown` — and a nil age fires the valve only when
|
||||
// `valveLicensed` finds a tier holding a POSITIVE claim of "never backed up".
|
||||
//
|
||||
// Distinguishing them needs a new field on `/backup/due` plus a compat rule in both directions →
|
||||
// tracked as its own task (R-88 Part 2, agent-side). Until then the R-88 BREAKER is what bounds the
|
||||
// damage: an unknown-driven cycle may still run once outside the window, but it can no longer repeat
|
||||
// every 5 minutes.
|
||||
// It used to be that the agent returned BYTE-IDENTICAL responses for "the storage read errored" and
|
||||
// "there has genuinely never been a backup" (same Due, same Reason, same nil AgeSecs), because
|
||||
// `newestArchiveOn`'s `(time.Time, bool)` signature could not represent "unknown" — while its own
|
||||
// doc comment promised exactly that. An unreadable storage therefore masqueraded as a first-ever
|
||||
// backup and quiesced customer apps outside the window. Fixed at the source.
|
||||
//
|
||||
// DO NOT "fix" this by deleting the nil branch. Scenario D — a genuinely never-backed-up box that is
|
||||
// only ever powered on outside its window — depends on it, and TestContract_NeverBackedUp_RunsOutside
|
||||
// -TheWindow will fail if you do. Silencing the valve would trade a loud bug for a silent one: a box
|
||||
// that never backs up at all, with nobody noticing for weeks.
|
||||
func scheduledRunAllowed(now time.Time, windowStart string, lastAgeSecs *int64, cadence time.Duration) bool {
|
||||
// STILL LICENSED, DELIBERATELY: `AgeStateLegacy` — a pre-v0.105.0 agent that omits the field. Its
|
||||
// silence must NOT be read as "unknown", or the valve stops firing on every un-upgraded box and a
|
||||
// genuinely new box never takes its first backup. The MinAgent floor drives the upgrade instead.
|
||||
//
|
||||
// DO NOT "fix" this by deleting the nil branch, or by dropping the legacy case from valveLicensed.
|
||||
// Scenario D — a genuinely never-backed-up box that is only ever powered on outside its window —
|
||||
// depends on BOTH, and TestContract_NeverBackedUp_RunsOutsideTheWindow will fail if you do.
|
||||
// Silencing the valve trades a loud bug for a silent one: a box that never backs up at all, with
|
||||
// nobody noticing for weeks.
|
||||
func scheduledRunAllowed(now time.Time, windowStart string, lastAgeSecs *int64, valveOK bool, cadence time.Duration) bool {
|
||||
startMin, err := backupwindow.ParseHHMM(windowStart)
|
||||
if err != nil {
|
||||
return true
|
||||
@@ -599,7 +628,10 @@ func scheduledRunAllowed(now time.Time, windowStart string, lastAgeSecs *int64,
|
||||
}
|
||||
// Outside the window: only the safety valve may run it.
|
||||
if lastAgeSecs == nil {
|
||||
return true // no recorded backup yet — never withhold the first one
|
||||
// R-88 Part 2: a nil age is no longer self-licensing. It fires the valve ONLY on a positive
|
||||
// "never backed up" (or a legacy agent that cannot say). An UNKNOWN age — an unreadable
|
||||
// storage — now defers, which is the whole point of this arc.
|
||||
return valveOK
|
||||
}
|
||||
return time.Duration(*lastAgeSecs)*time.Second > cadence+24*time.Hour
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user