supervisor: retain lifetime restart history across operator resets (closes #70) - #77
Conversation
|
Version collision with #76 — flagging before you hit it, not after. Both open PRs bump Each is correct in isolation — both add an additive public item to They are otherwise independent — #76 touches I will rebase whichever loses and re-cascade the versions; say the word or just merge in whatever order you prefer and I will follow. I would rather do that than have you resolve a version cascade in a merge editor. |
2a2a8b0 to
3e7af9c
Compare
|
Rebased onto Head Collision with #76 unchanged — both still bump |
There was a problem hiding this comment.
Review by execution: the reset asymmetry is the whole feature and it holds — all three restart_count increment sites increment lifetime_restarts beside it, and the single operator reset site (set_enabled) touches restart_count alone, verified by reading every assignment site on the branch. The status split (spent-budget vs lifetime) closes #70's never-crashed vs crashed-then-reset ambiguity exactly as ruled. Ten fleet_lint failures during my local gate were proven ENVIRONMENTAL (identical on master in the same twin — fresh-stub first-exec assessment, the documented macOS class; clean rerun after warm-exec: 452/0). Full matrix green on feat/lifetime-restarts at the rebased head. Merging.
The budget display restarts 0/3 currently describes both a never-crashed module and one that crashed twice before an operator reset. Add a monotonic lifetime count so those states remain distinguishable, while deliberately leaving restart_count reset semantics unchanged because operator restart and re-enable are budget grants.\n\nCONSUMER-IMPACT: subc-control 0.7.0 adds optional lifetime_restarts; subc-client-rs 0.8.1 cascades the wire dependency. The field is serde-defaulted and omitted when unknown, so old payloads decode cleanly and existing absent fixtures remain byte-stable.
3e7af9c to
f8ce018
Compare
Closes #70.
ck module statusreportsrestarts N/M— the live restart budget. Three operator verbs zero that counter:So
restarts 0/3means either "this module has never crashed" or "this module crashed twice and an operator restarted it". Those are opposite operational situations — one is healthy, one has been failing and was papered over — and the status line renders them identically. An instrument that reports the same value for a healthy module and a repeatedly-failing one is not measuring what its label claims.Change
A second counter,
lifetime_restarts, incremented at every site whererestart_countis incremented and never reset.reset_restart_countdoes not touch it.Rendering shows it only when it diverges from the budget count, so the healthy case stays unchanged:
Budget semantics are deliberately untouched. The reset is correct — an operator restart legitimately grants a fresh budget, and
set_enabled(true)revival depends on it. This adds an honest second counter; it does not change when a module gives up.Wire representation
Option<u32>withserde(default, skip_serializing_if = "Option::is_none").The
Optionis load-bearing rather than stylistic. My earlier PR #8 usedskip_serializing_if = "is_zero"on a bareu32in this same struct, which collapsed "zero restarts" and "field absent" into one wire state — the absent-vs-empty collapse that issue #12 was filed about, and it was right to be rejected. HereSome(0)("known: never restarted") stays distinct fromNone("old peer, unknown"), so an old payload keeps its honest unknown instead of asserting zero. Existing golden fixtures are unchanged — they serializeNoneand skip the field.Tests
Four arms, each proved load-bearing by breaking the seam it guards:
restart_count0,lifetime_restarts2left 0 right 1; reload site →left 0 right 1; crash site → lifetime stuck at 0 whilerestart_countadvancedNonenullinstead of50/3instead of0/3 (2 lifetime)The per-site arm is the one worth having: a test that only exercises the common crash path stays green if an increment site is missed, and the counter then under-reports silently forever — which would reproduce the exact defect this fixes.
Gates
Baseline measured independently on
2a0fbb67rather than taken on trust: 765 passed there, 769 here, difference exactly the four new tests.Additive wire change —
subc-control0.7.0 with the dependent cascade.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes
ck module statusso the restart budget no longer hides repeated crashes behind an operator reset, distinguishing "never crashed" from "crashed twice, then reset" (closes #70).lifetime_restarts, incremented whereverrestart_countincrements but never reset, so reset history renders as0/3 (2 lifetime)while a fresh budget keeps the plain0/3.None), and both render branches so a missed site can't silently under-report.Wire change
SupervisorEntrygains optionallifetime_restarts;Some(0)stays distinct fromNoneon old peers, so unknown history stays honest.subc-controlto 0.7.0, cascading tosubc-client-rs0.8.1 andsubc-core0.8.2.Written for commit f8ce018. Summary will update on new commits.