Land Tracks A/B/C upgrade paths with live proof harnesses - #55
Open
electricapp wants to merge 12 commits into
Open
Land Tracks A/B/C upgrade paths with live proof harnesses#55electricapp wants to merge 12 commits into
electricapp wants to merge 12 commits into
Conversation
Rolling binary upgrade, pg_upgrade 17→18, and blue/green cutover with compose proofs, WAL/PITR archive path, and an upgrade-proof CI suite.
The Tracks A/B/C branch had every checklist box ticked. Auditing it found a
CI break, three ways for the Track C authority protocol to lose writes, a
refused-by-name marker file, and five FATAL contracts whose inversions could
not turn red. This addresses all of it.
CI was red on a clean checkout. `*.bin` in .gitignore swallowed the two Raft
frame goldens, so `supported_frame_fixture_decodes_and_round_trips` and
`unsupported_newer_frame_fixture_is_refused` panicked on a fresh clone. They
had only ever run against a working tree that still held the regenerated
files. The cosign fixtures already had a negation beside them; the frame
fixtures now do too.
SEAL was the wrong way round. It committed WRITE_AUTHORITY_SEALED and then
applied the role revoke, taking the final LSN from the caller — which had read
it before asking. Both orderings leave a window where blue acks a client write
whose LSN is past `final_lsn`, so the recipient's drain gate passes without it
(U2). The endpoint now fences first, reads `pg_current_wal_flush_lsn()` on the
server, and only then commits. If the fence fails, nothing commits.
A gateway fence was never enough on its own — `dual_writability_prober` writes
straight to the internal PG port, and it is right to. The revoke is now driven
by a reconciler reading committed state, so a node promoted between SEAL and
the revoke applies the same fence without being asked again, and it covers
sequences because `nextval` is a write. `lift` restores exactly the grants the
apply recorded, so aborting a cutover no longer hands the app role blanket
DML on every table it never had.
Green accepted writes from boot. It ran its own Raft group, elected its own
leader, and nothing said otherwise — the "second Raft group that elects itself
into write authority" the checklist lists under explicit refusals. Clusters now
carry `cluster_id`, `holds_client_write_authority(local)` is the single read
that answers whether this cluster may take client writes, and a recipient sets
`awaits_write_authority_from` + `client_fence_roles` to fence itself until its
own log records the handoff. Config speaks only until the log does.
Out-of-order cutover commands answered `success: true`. `ClusterState::apply`
returned `()`, so a phase-mismatched entry committed as a no-op and the API
reported it as progress — including applying the revoke on a cluster that was
never in a cutover. `apply` now returns `CommandOutcome`, which rides back on
`ClusterResponse` and becomes a 409.
Forced transfer left no trace. It reused DrainObserved+Transfer, so the log
could not distinguish a break-glass with accepted write loss from a clean
handoff. It is now `CutoverTransfer { forced: true }`, permanently. It still
needs donor quorum, which the chaos case now asserts rather than tolerating —
the previous harness printed the status and moved on.
Track B kept a marker file. `.pgbattery_pg_upgrade_anchor.json` recorded
from/to majors, both datadirs, both bindirs and the transfer mode — the thing
UPGRADE.md refuses by name ("never by a marker it wrote about its own
intent"). Every field is on disk: PG_VERSION names the majors, and
`pg_upgrade --link` renames `global/pg_control`, so its absence *is* link
mode. `UpgradeSite::probe` reads it. The anchor was also written last, after
the new postmaster started, so a crash mid-upgrade left no anchor, no
rollback, and a refusal to re-run — that path now discards an unstarted
target and resumes.
Also Track B: the two `pg_upgrade` invocations had a 10-minute budget and no
`kill_on_drop`, so a timeout orphaned a live pg_upgrade still writing into the
target while the orchestrator unfroze the gateway. Every Command in
process.rs sets it, with a comment explaining why. The budget now scales with
the data directory. `--confirm` re-runs the verification gate before deleting
anything. The freeze moved into the Raft log: it was a per-node latched bit
that an orchestrator dying mid-flow would leave set forever, and that a
restart would silently clear into a cluster mid-`pg_upgrade`. initdb params
are derived once, including the locale provider and ICU locale, and while
PostgreSQL is still up — a mismatch used to abort out of pg_upgrade after the
freeze. Post-upgrade scripts were looked for in the new datadir; pg_upgrade
emits them in its working directory, so they had never run. Rollback prints
the unfreeze watermark against the new cluster's final LSN and refuses
without --accept-write-loss only when writes actually landed past it, and its
proof write goes to a temp table instead of leaving `pgbattery_rollback_proof`
in the user's schema.
The FATAL inversions did not invert. `lint_matrix` checked that each name
resolved somewhere in the tree, so `docs/CONTRACTS.md` and a Rust source path
both satisfied it, and U1-U5 shipped with happy-path unit tests and a spec
that says in its own header it does not model them. The gate now requires a
matrix case, a test function, a testing/ harness, or an `inv-` model config,
and testing/test_bluegreen_oracles.py supplies 16 real ones — each driving an
oracle to its failing answer, including the row-count drain check that passes
on a history missing a key, and the `setval` on a column owning no sequence
that made U3 inert.
The TLA+ spec modelled the problem away: one global phase both clusters read
atomically, a handoff in a single step, and `EpochMonotonic` defined as
`epoch \in 0..MaxEpoch` — a type constraint that cannot express monotonicity.
It now gives the clusters separate logs with a delivery step between them, so
TLC explores the window where blue has relinquished and green has not yet
heard, and monotonicity is an action property. The counterexample is
AcceptAuthority with its attestation guard removed rather than an action
written to negate the assertion it trips.
Harnesses: the Track C e2e waited for catch-up by sleeping two seconds,
gated the drain on row counts, never asserted that a post-cutover write
through blue's gateway reached green, and accepted the U1 prober's
INCONCLUSIVE exit as a pass. All four chaos modes carried `expect_exit: null`,
were excluded from CI, and printed instead of asserting; one said "verify
separately" in its own message. They now assert, and they run. The Track B
proof gained a writer with an acked-key ledger, dropped `--allow-no-backup` so
the verified-restore preflight actually runs, and gained a live rollback drill.
Ports come from topology.py rather than hardcoded 5434 and 172.31.0.11.
Performance: the cutover projection cloned three Strings and parsed a
SocketAddr on every Raft metrics tick — tens per second, forever, for state
that changes twice in a cluster's lifetime — inside the watch closure, which
held the watch lock across a parking_lot acquisition. It is derived once,
outside, without allocating. The gateway's per-message path is a single Copy
read again.
Python: typer and a rich Console throughout, no bare print, no urllib.
Docs: STATE_MACHINE gains 8b-8e for the freeze, the probed upgrade position,
the authority protocol, and the preflight; UPGRADE.md stops calling itself a
proposal and names the attestation gap; the checklist drops step 9 to `[~]`
and lists what is not implemented instead of ticking it.
14 preflight gates pass. 5 TLA specs and all 5 counterexamples pass.
The two findings the audit remediation left open. Both were closable; neither
needed the months of work the "not implemented" note implied.
Provenance was taken on trust. Green recorded the handoff in its own log and
checked only that the epoch increased, because the two Raft groups share no
log and it could not read blue's `AUTHORITY_TRANSFERRED` entry. The endpoint
said so — `accept_unverified_provenance` was the only path — but a fence that
lifts on an unverified assertion is a fence with a hole in it, and U1 is the
contract it holds.
It can be verified, and the mechanism is the ordinary one: a commit is
confirmed from outside a Raft group by reading it from a majority. Green now
fetches `/api/v1/cutover/status` from every donor node it is given and
requires a strict majority to report TRANSFERRED, to green, at exactly this
epoch, with blue no longer holding authority. Silence counts against, so a
partitioned green talking to one stale blue node stays fenced — a minority
cannot hold a commit, so a minority cannot prove one.
`accept_unverified_provenance` narrows to the break-glass for a wholly
unreachable donor and is refused alongside a node list. Five unit inversions
cover minority, all-silent, contradicting, empty-set, and the 5-node case; the
live case asserts a single donor node cannot hand over.
`--blue-green` apply refused unconditionally. The protocol worked, the
endpoints existed, and the only thing driving them was the test harness — so
the audit was right that the harness was the implementation. It now runs:
pgbattery upgrade postgres --to 18 --blue-green --experimental \
--green-node <addr> --green-cluster-id green --app-role app
Arm, seal, drain, transfer, receive-authority, in that order, against the
donor's leader and the recipient's. It refuses a recipient that already claims
authority (which would mean it was never fenced), one whose `cluster_id` is
not the one named, and one with no logical subscriptions — draining against a
cluster that is not subscribed would burn the budget and then blame the wrong
thing. Drain waits on `pg_replication_origin_progress`, the minimum across the
recipient's subscriptions, against the donor's sealed LSN: the applied
position, not `received_lsn`. Postflight refuses unless exactly the
recipient's nodes claim authority.
What is still not implemented is now the honest boundary: green provisioning
and logical seeding. The command requires green to be streaming already and
says so, rather than the previous blanket refusal that made the whole track
look absent. The DDL freeze and schema digest remain unimplemented too, and
the checklist lists all three instead of ticking them.
14 preflight gates pass. 537 lib tests.
The recipient refused a handoff the donor had actually committed. It read the donor's nodes once, immediately after the donor's leader applied the transfer; the donor's followers had not applied it yet, so one node in three confirmed and receive-authority returned 409. Waiting is not a weaker check than refusing: a majority is still required before any fence lifts, so the poll now runs against a 30s budget and refuses only when it expires. Three ways CI was hiding why a case failed: - A non-zero exit raised the exit code and the command, never the output. Every failure cost an artifact download to read what the harness said. - A timeout killed only the direct child. Steps are process trees (uv -> python -> docker), so a grandchild kept the pipe open, the drain timed out, and the partial output died with it. Steps now run in their own process group and the group is killed. - Python block-buffers stdout to a pipe, so a killed harness had written nothing at all. Steps run unbuffered. The upgrade-proof suite sets manage_cluster: false, so the cases own their compose file and ports. The runner was collecting `docker compose logs` for its own project (empty) and probing node1-3 for a cluster running as blue1-3/green1-3, producing a directory of connection errors that read as a dead cluster. Those suites now collect container state and logs directly.
The upgrade-proof job never built the runtime image, so the first `docker compose up` inside each harness built it. That build ran against the case's own step budget: the Track A proof gets 600s, spent all of it building, and was killed having asserted nothing. The other jobs already build in a separate step; this one now does too. All three compose files build the same runtime target, so only the first pays for it. The docker calls in the three upgrade harnesses had no timeout, so a wedged daemon hung until the step budget killed the harness, naming no command. They are bounded now and say which call hung.
The donor-quorum poll ran for exactly the management API's own request timeout, so its 409 — the one that names which donors did not confirm — raced the timeout layer's bare 408. An operator debugging a stuck cutover got whichever won. The budget now sits under that timeout, with a compile-time assertion tying the two constants together so they cannot drift back into a tie.
/api/v1/postgres/status derived active_major from PG_VERSION on disk but returned active_bin_dir straight from the configured pg_bin_dir, so the two fields could describe different majors. That is the state a Track B upgrade creates, and a caller that runs the reported binaries against the reported data directory gets a server that will not start. It now resolves through pg_bin_dirs, falling back to the configured dir only when the major has no mapping. has_upgrade_anchor named the marker file that no longer exists; it reports whether rollback is available, so it says that now. The PITR proof started its restored postmaster with `pg_ctl -l pg.log` and, when that failed, raised the bare CalledProcessError without ever reading the log it had just written. The reason the server refused to start was in that file every time.
Running each step in its own session let a timeout kill the whole tree, but it also means Ctrl-C no longer reaches the step: the signal goes to the runner's group, and docker compose and the harness under it survive. A local run interrupted halfway would leave a cluster up and a harness writing to it. The runner now kills the group before re-raising.
Aborting a cutover restored blue's write authority in the log and left its application locked out of PostgreSQL. The reconciler decided what to do by comparing the wanted state against a bool remembering what it had applied — but the seal handler applies the fence itself, so that bool was still false while PG was fenced. On abort it compared false against false, concluded there was nothing to do, and the REVOKEs stayed. A cluster that aborted a cutover never accepted a client write again. It now reads the fence from the snapshot table and reconciles against that, which is the rule STATE_MACHINE.md already states: no in-process cache of truth that can be re-derived, probe PostgreSQL across the process boundary. Two Writer threads bound self._stop, which is a threading.Thread internal on CPython 3.12. join() calls it, so every Track B proof crashed at teardown after its assertions had passed. The interpreter drops _stop in 3.13, which is why preflight could not see it: uv picked 3.13 locally against CI's 3.12.3. testing/.python-version pins them together, and the writers use _stop_event. psql prints the `INSERT 0 1` command tag next to the value of an INSERT ... RETURNING, so the U3 sequence probe parsed "1051\nINSERT 0 1" as an int. Wrapped in a CTE, the result is a row set and the tag goes away. The blue-quorum chaos case asserted force-transfer fails closed immediately after stopping two containers, without waiting for openraft to notice they were gone — and it stopped them with check=False, so a stop that never happened would have read the same. It now requires the stop to succeed and waits for blue to actually lose its leader before asserting anything.
A recipient that took write authority was stuck in TRANSFERRED, and arming refused every phase but IDLE and ABORTED. So the reverse cutover — the rollback the whole track is supposed to offer — could not be started at all: green answered the drill's first arm with 409. Phase alone cannot say who may arm, because donor and recipient are both TRANSFERRED after a cutover. The guard is the authority read: the cluster that holds client write authority may hand it on, and the one that gave it away may not take it back without a handoff. Letting both arm would put two clusters in ARMED at the same epoch with no transfer between them. The restored PITR datadir was left at the shell's umask by mkdir and tar, and the postmaster refuses anything looser than 0750 — which the pg.log dump added in the previous commit said on the first run that had it.
A failed reconcile went back to waiting on the next fence change. The change that needed acting on had already been observed, so one transient SQL error left the application fenced out of an unsealed cluster for good — the same outcome the cached flag used to produce, by a different route. It retries. Three harness assertions raced the thing they were asserting about: - The Track B proof read server_version_num from a node the upgrade had just restarted, and got "the database system is starting up". It waits for pg_isready first. - The hold case counted acked writes from the moment the freeze was requested, so a write already in flight while the fence propagated counted against the hold. It now measures from where the writers actually stall, and a writer that never stalls is the failure. - The blue-quorum case waited for blue to stop reporting a leader, which is openraft's demotion timing rather than the contract. It confirms the containers are down: with two of three gone no majority can ack, so a commit that still succeeds is a real violation with no timing caveat. The U1 prober ran as the app role and could not create its probe table -- permission denied for schema public. Granting CREATE would have put a hole in the very fence the prober checks, since a role that creates a table owns it and writes to it whatever was revoked. The table is created by the superuser during setup and granted DML, so the fence covers it. Rollback failures reported only their outermost context: e.to_string() on an anyhow error drops the chain, so "rolled-back cluster did not accept a write" arrived with the reason removed.
The previous attempt decided the hold had taken effect once the writer had not acked for 1.5s. That is a guess about propagation wearing the clothes of an effect check: a writer that stalled for its own reasons and resumed while frozen would fail the case, and a slow enough gateway would pass it. pgbattery_connections_held_during_upgrade increments at the moment the gateway parks a connection, and counters only go up. Waiting for it to move is the effect itself, and the writer's one connection cannot ack while it is held, so the baseline taken straight after is stable. A failed scrape reads as None rather than zero. Treating an unavailable reading as "not holding" would have ended the wait early and put the flake back in a different place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.