fix(health): a pre-epoch newborn VM is not an owner-epoch violation - #145
Conversation
|
Heads up — I'm still working this one through (rebasing/verifying against the freshly-merged |
de21096 to
65faf45
Compare
|
Rebased onto Verified green on the rebased tip: Drafted and posted by Claude Code. |
Adversarial review remediation —
|
|
@colonelpanik — ready for review whenever you have time. This branch is done from our side for now: the adversarial-review remediation above is the last change we plan to make, CI is green on Happy to take it further if anything in the remediation — particularly the two-sided grace bound, or the decision to accept the negative-skew case rather than widen the window — reads differently to you. Drafted and posted by Claude Code. |
|
I agree that A High-severity suppression path remains under the stated replication threat model. If a peer rewrites This invalidates the remediation claim that worst-case suppression is capped and that a forged row always surfaces. The tests assign The grace needs detector-owned durable state, such as a finite complete-scan budget that peer row edits cannot reset. That preserves the newborn allowance without trusting remote wall time. The stronger end state assigns a positive epoch and writes both markers before publishing the VM as running. I would not merge this head until the renewable-grace path is addressed. |
A fresh create is born at vm_owner_epoch 0 and graduates on the reconciler's next sweep, which also writes its first runtime marker. In that window the VM is RUNNING on its owner with no marker, and the dual-run detector's epoch check — which requires a VALID marker equal to the DB epoch — paged it as a critical owner_epoch_mismatch. Every fresh `lv run` flashed a critical condition for up to a sweep interval (and, via the host-involvement admission clause, transiently refused unrelated creates on that host) before resolving on its own. Observed live on the lab, 2026-08-05 and again 2026-08-06 after a migration test. The exception is scoped as tightly as the regime allows: only a row at epoch 0 with a MISSING marker is skipped — a state that names exactly 'awaiting backfill', since epochs are never reset. A PRESENT marker against an epoch-0 row still pages (the runtime claims a generation the DB does not know), and a missing marker on a graduated row remains the violation it always was; both boundaries are pinned by tests. The newborn case was observed red before the fix; full repo gate (build, vet, go test ./..., ci-guards) green.
withinNewbornGrace tested only `time.Since(t) < newbornEpochGrace`. Since is NEGATIVE for a created_at in the future and every negative duration is less than the grace, so a row stamped ahead of the evaluating host sat inside the exception indefinitely: the owner-epoch mismatch finding was skipped on every pass, and that VM lost owner-epoch protection permanently without ever paging. That is reachable without an attacker — a creator whose clock is wrong — and with one: created_at is a replicated column and corrosion is last-writer-wins, so any peer can write it. A detector must not have an input that switches it off for good, which is exactly what an unbounded future timestamp was. The window is now bounded in both directions by the same constant. A modest future stamp is honest clock skew between the creator and whichever host holds the detector lease, so it still earns the newborn exception and the fix this branch landed is preserved; past one grace ahead it does not. Empty and malformed timestamps keep failing toward paging. Be precise about the bound: it caps what any ONE stamp buys at two grace windows. It does NOT bound a peer that REWRITES created_at before each pass, since the predicate is re-evaluated against freshly-read DB state every sweep. That is the detector's general trust of replicated DB state rather than anything specific to this predicate — the same writer suppresses the whole epoch check more cheaply by setting state to a migrationState, setting deleted_at (ListVMs filters it), or pointing host_name at an unprobed host, none of which need renewing. Closing that class means detector-owned durable state the peers cannot reset, or assigning a positive epoch and writing markers before a VM is published as running, which removes the newborn window instead of bounding it. Both are follow-ups, not this diff; the predicate now documents the limit rather than overstating it. Tests drive the predicate directly across both bounds (including year 9999, empty, and malformed) and drive the detector end to end with a post-dated row, which fails before this change.
124dacc to
66f5801
Compare
|
Rebased onto current On the renewable-grace finding: it is correct, and my earlier claim above was wrong. I wrote that worst-case suppression is capped at two grace windows and that a forged row always surfaces. Both hold only for a stamp that is wrong once. What changed here: the same overclaim was in the code comment, which is worse than in a PR thread — a comment asserting a safety property the code lacks discourages exactly the audit that would find the gap. The predicate now documents the real limit instead. What did not change, and why. Renewal is not a weakness of the newborn grace specifically; it restates that the epoch check trusts replicated DB state. The same writer suppresses it more cheaply, with a single write and no renewal at all:
The renewal path is strictly more expensive and achieves strictly less. Closing it alone would need new durable detector state — a schema bump, ledger entry and stmtshapecheck registration — and would still leave those three open, so it does not belong in a 20-line detector fix. Worth adding: Follow-ups, filed rather than folded in: detector-owned durable scan budget that peer row edits cannot reset; and the stronger end state you identified — assign a positive epoch and write both markers before publishing a VM as running, which removes the newborn window rather than bounding it. The second is the real fix and lives in the create path. Gate on Drafted and posted by Claude Code. |
A fresh VM create is born at
vm_owner_epoch = 0and graduates on the reconciler's next sweep, which also writes its first owner-epoch marker. In that window the VM is running on its owner with no marker — the expected newborn state — but the dual-run detector's epoch check (which requires a valid marker equal to the DB epoch) paged it as a criticalowner_epoch_mismatch. Every freshlv runflashed a critical condition for up to a sweep interval, and via the host-involvement admission clause it transiently refused unrelated creates on that host, before resolving on its own.The exception is scoped as tightly as the regime allows: only a row at epoch 0 with a missing marker is skipped — a state that names exactly "awaiting backfill", since epochs are never reset. A present marker against an epoch-0 row still pages (the runtime claims a generation the DB does not know), and a missing marker on a graduated row remains the violation it always was. Both boundaries are pinned by tests; the newborn case was observed red before the fix.
Verified live on a 4-node lab: a fresh
lv runpolled for two minutes produces zeroowner_epoch_mismatchflashes (the old build paged within one sweep), while epoch graduation and marker convergence are unaffected.