Skip to content

fix(database): five HA defects an adversarial audit of #1143 confirmed live - #1159

Merged
paulocsanz merged 1 commit into
masterfrom
pcs/database-ha-audit-fixes
Aug 31, 2026
Merged

fix(database): five HA defects an adversarial audit of #1143 confirmed live#1159
paulocsanz merged 1 commit into
masterfrom
pcs/database-ha-audit-fixes

Conversation

@paulocsanz

Copy link
Copy Markdown
Collaborator

Follow-up to #1143, which is merged and released as v5.46.0. An adversarial audit of that PR's content found five defects that are live in the released CLI. Each is independently reachable; they are grouped here because they were found in one pass over the same feature.

Redis/MySQL scale restamps consensus quorum on the whole fleet

restamp_replica_wiring wrote quorumVariable to the root and every replica, while the peer-list block directly above it stamped joining nodes only and carried a comment explaining why.

A running node reads that env exactly once, on the first boot that writes its coordinator config, and never again — so the edit changed nothing functionally, while still marking every node stale. That is a simultaneous fleet-wide restart, racing the coordinator into a spurious failover mid-scale. And on a real cluster the survivors' copy is a reference to the root's (${{Redis-1.SENTINEL_QUORUM}}), so overwriting it with a literal was not even a no-op edit — it detached them from the root's value.

Now stamped on joining nodes only, matching useScaleHACluster.tsx. A scale-down stamps nothing at all: survivors converge through the image's own quorum-sync watcher as removed nodes drop out. The per-replica identity variable moves to joining-nodes-only for the same reason — a replica's identity is its own name, which a scale never changes.

The routing edge's data-node list stays fleet-wide. It is read per connection, not once at boot.

ha revert's debris sweep was environment-wide and engine-blind

The sweep deleted any parentless, role-stamped service in the environment. In a mixed environment, reverting a Redis cluster deleted the Postgres cluster's live-scaled replicas — a clusterRole stamp was the only thing standing between an unrelated service and deletion.

The orphan scan is now scoped to services running an image the engine's own companion publishes, read off the companion record. Each companion publishes under its own prefix (postgres-ha/*, redis-ha/*, mysql-ha/*), which is the surviving evidence of which companion built a service once the parent link is gone — read from the record, so a template that adds a sidecar the CLI has never heard of stays covered.

An unreadable record skips the debris scan and warns, rather than widening it to everything: the snapshot members are the part that is still provable. The resume path also now names the services it is about to delete instead of only counting them — it deletes services the user never listed, on evidence they cannot see.

The pooler exclusion is unchanged and still needed on the snapshot path, where the pooler is a genuine child of the root.

CLI-scaled replica volumes bypassed primary-size matching

The volume instance was provisioned by volumeCreate directly, ahead of the patch that stamps the node's clusterRole/parentServiceId — the exact pair backboard's resolveNewVolumeInstanceSizeMB keys on to size a replica volume to hold a full base backup of its primary. Created before either existed, it read as an ordinary volume and landed on the flat plan default, reintroducing the undersized-replica-volume class for CLI scaling.

The record is now created bare (environmentId: null) and the instance is staged in the same patch as the role and parent — the split useScaleHACluster already uses. It also drops a patch-system redeploy per volume.

Tag parsing diverged from the server, producing false refusals

The CLI required a tag component to be numeric end to end; the server's extractImageTagVersion reads the leading major[.minor] and ignores everything past it.

tag server CLI (before)
redis:8.2-alpine 8.2 major 8, no minor
redis:7-bookworm major 7 no version at all

Both were refused by a CLI pre-flight for conversions the server would have accepted. Now mirrors the server. The test that codified the old parse is corrected rather than kept.

--service pointed at a replica stood in for the root

resolve_root_service_id hopped only edge to its parent, so a replica resolved to itself. ha revert then prechecked the replica's primariness, handed templateRevert the replica's id (whose resolver does no root validation), and took a sweep snapshot in which the true root was just another deletable member. Every member role now hops. Preexisting, and inherited by all three engines when the tree was generalized.

Not fixed here

The mono-side root cause that feeds the orphan population above: the patch path drops parentServiceId. packages/backboard/src/temporal/workflows/environments/activities/service.ts applies clusterRole for an existing instance on its update branch but has no equivalent block for the parent link, and serviceCreate(environmentId) pre-creates the instance so the create branch (which does set it) never runs for live scale-ups. CLI and dashboard are both affected; the dashboard compensates through canvas groups, which the public API cannot stamp. Separate repo, separate PR.

Verification

  • cargo test — 1310 pass. New coverage: joining-nodes-only restamp on scale-up and the no-op scale-down, a Redis revert leaving a Postgres cluster untouched, the unreadable-companion degrade, every member role resolving to its root, the companion-repository reader (exact match, not prefix), and an end-to-end scale against a stub backboard asserting the volume is staged rather than pre-provisioned and that survivors go unstamped.
  • Each fix was re-reverted individually to confirm its own test fails without it — none of these pass against the old behavior.
  • cargo clippy --all-targets and cargo fmt --check clean; no new warnings in any touched file.
  • The three companion records (postgres-ha, redis-ha, mysql-ha) were read from the live API to confirm the per-companion repository prefixes the sweep scope relies on, rather than assumed.

…d live

Each is independently reachable in released 5.46.0; grouped here because
they were found in one pass over the same feature.

**Redis/MySQL scale restamped consensus quorum on the whole fleet.**
`restamp_replica_wiring` wrote `quorumVariable` to the root and every
replica, while the peer-list block directly above it stamped joining nodes
only and carried a comment explaining why. A running node reads that env
once, on the first boot that writes its coordinator config, and never
again, so the edit changed nothing functionally -- but it marked every
node stale and restarted the fleet at once, racing the coordinator into a
spurious failover mid-scale. On a real cluster the survivors' copy is a
reference to the root's (`${{Redis-1.SENTINEL_QUORUM}}`), so the literal
also detached them from it. Now stamped on joining nodes only, matching
`useScaleHACluster.tsx`; a scale-down stamps nothing, as there the quorum
converges through the image's own quorum-sync watcher. The per-replica
identity variable moves to joining-nodes-only for the same reason.

**`ha revert`'s debris sweep was environment-wide and engine-blind.**
Any parentless, role-stamped service in the environment was a delete
target, so reverting a Redis cluster in a mixed environment deleted the
Postgres cluster's live-scaled replicas -- a `clusterRole` stamp was the
only thing standing between an unrelated service and deletion. The scan is
now scoped to services running an image the engine's own companion
publishes, read off the companion record (each publishes under its own
prefix: `postgres-ha/*`, `redis-ha/*`, `mysql-ha/*`), which is the
surviving evidence of which companion built a service once the parent link
is gone. An unreadable record skips the debris scan and warns rather than
widening it, and the resume path now names what it is about to delete.

**CLI-scaled replica volumes bypassed primary-size matching.** The volume
instance was provisioned by `volumeCreate` directly, before the patch
stamped the node's `clusterRole`/`parentServiceId` -- the exact pair
backboard's `resolveNewVolumeInstanceSizeMB` keys on to size a replica
volume to hold a full base backup of its primary. It read as an ordinary
volume and got the flat plan default, reintroducing the undersized-replica
class for CLI scaling. The record is now created bare (`environmentId:
null`) and the instance staged in the same patch as the role and parent,
the split the dashboard already uses; it also drops a patch-system
redeploy per volume.

**Tag parsing diverged from the server and produced false refusals.** The
CLI required a tag component to be numeric end to end; the server reads the
leading `major[.minor]` and ignores the rest. `redis:8.2-alpine` was 8.2 to
the server and "no minor" to the CLI, `redis:7-bookworm` was major 7 and
"unversioned" -- both refused conversions the server would have accepted.
Now mirrors `extractImageTagVersion`. The test that codified the old parse
is corrected.

**`--service` pointed at a replica stood in for the root.**
`resolve_root_service_id` hopped only `edge` to its parent, so a replica
resolved to itself: revert then prechecked the replica's own primariness,
handed `templateRevert` the replica's id (whose resolver validates no
root), and took a sweep snapshot listing the TRUE root as deletable. Every
member role now hops. Preexisting, and inherited by all three engines.

Not fixed here, different repo: the patch path drops `parentServiceId` on
an existing instance (`environments/activities/service.ts` applies
`clusterRole` on update with no equivalent block for the parent link),
which is what strands the members the sweep above has to recognize.

Verification: `cargo test` -- 1310 pass, including an end-to-end scale
against a stub backboard asserting the volume is staged rather than
pre-provisioned and that survivors are left unstamped. Each fix was
re-reverted individually to confirm its test fails without it. `cargo
clippy --all-targets`/`cargo fmt` clean.
@paulocsanz paulocsanz added the release/patch Author patch release label Aug 31, 2026
@paulocsanz
paulocsanz merged commit 46546c9 into master Aug 31, 2026
8 of 9 checks passed
@paulocsanz
paulocsanz deleted the pcs/database-ha-audit-fixes branch August 31, 2026 21:54
paulocsanz added a commit that referenced this pull request Aug 31, 2026
…eletions; never delete the acting primary

Rebased onto v5.47.0: #1159 already landed the joining-nodes-only restamp
scope, the staged volume instance, the scoped revert sweep and the tag-parse
parity this branch originally carried. What remains here is the rest of the
scale contract:

1. New member SERVICE records are now created bare (environmentId: null,
   no source) and their instances created by the staged patch (isCreated),
   the way the dashboard's hook does -- because the patch-apply workflow's
   instance-create path is the ONLY one that persists parentServiceId. An
   instance pre-created by serviceCreate takes the update path at commit,
   which applies clusterRole but silently drops the parent link, so every
   CLI-scaled member landed role-stamped and orphaned from parent-chain
   membership walks (ha status, scale rosters, the platform's cluster-primacy
   guard, the admin monitors). The volume record goes fully detached too (no
   serviceId; the mount is declared by the patch). With the cause gone, the
   post-commit "platform dropped the parent link" warning goes with it.
   The image, the cluster's canvas group, and the sibling's whole deploy
   config (healthcheck, region placement -- a node cloned into a different
   region than its cluster replicates cross-region forever) ride the patch.

2. Scale-down deletions are STAGED (volume first, then the service) instead
   of issued as direct ServiceDelete/VolumeDelete calls, so the whole scale
   commits atomically and the platform's cluster-primacy commit guard can
   inspect the patch it exists to inspect.

3. Replica scale-down never deletes the node currently ACTING as primary.
   Deletion order is by node number, and after a failover the primary can be
   any replica: ha scale now probes the live role first, removes a
   lower-numbered replica instead where the count allows it, and refuses
   with the switchover remedy where it does not; when no member answers it
   degrades to a warning (same posture as revert's primacy precheck).

Also: the internal (coordinator) identity variable moves to joining-nodes-
only, matching the replica identity scope #1159 set (coordinators restarting
together is quorum loss); the routing-edge lookup excludes the pooler
through the engine's declared pooling spec instead of an image name compiled
into the filter.

cargo test: 1312 pass. The stub-backboard scale-up test now also pins the
bare service record (environmentId/source null) and the staged
isCreated/image; new stub tests pin the staged scale-down tombstones, the
primary being skipped in favor of a lower-numbered replica, and the refusal
with the switchover remedy when the count cannot be honored.
paulocsanz added a commit that referenced this pull request Aug 31, 2026
…s config shape

The CLI's ServiceInstance config struct parsed haConversionConfig (and its
role selectors) but nothing ever read it -- by design: conversion bounds are
read from the COMPANION template record, the same record the server-side
gate reads, because a standalone service's own copy can be partial, stale,
or absent entirely (the redis template carries none, and the postgres slot
shipped a stale one that offered even-voter cluster sizes). Keeping the
field parsed was an invitation for a future call site to trust the service's
copy and silently skip the real bounds for exactly the engines that need
them. Remove it, with a note on the struct saying why it is absent.

The rest of this branch's original content -- the image-lineage-scoped
revert sweep and any-member-to-root resolution -- landed via #1159
(v5.47.0).
paulocsanz added a commit that referenced this pull request Aug 31, 2026
…s config shape (#1161)

The CLI's ServiceInstance config struct parsed haConversionConfig (and its
role selectors) but nothing ever read it -- by design: conversion bounds are
read from the COMPANION template record, the same record the server-side
gate reads, because a standalone service's own copy can be partial, stale,
or absent entirely (the redis template carries none, and the postgres slot
shipped a stale one that offered even-voter cluster sizes). Keeping the
field parsed was an invitation for a future call site to trust the service's
copy and silently skip the real bounds for exactly the engines that need
them. Remove it, with a note on the struct saying why it is absent.

The rest of this branch's original content -- the image-lineage-scoped
revert sweep and any-member-to-root resolution -- landed via #1159
(v5.47.0).
paulocsanz added a commit that referenced this pull request Aug 31, 2026
…eletions; never delete the acting primary (#1160)

Rebased onto v5.47.0: #1159 already landed the joining-nodes-only restamp
scope, the staged volume instance, the scoped revert sweep and the tag-parse
parity this branch originally carried. What remains here is the rest of the
scale contract:

1. New member SERVICE records are now created bare (environmentId: null,
   no source) and their instances created by the staged patch (isCreated),
   the way the dashboard's hook does -- because the patch-apply workflow's
   instance-create path is the ONLY one that persists parentServiceId. An
   instance pre-created by serviceCreate takes the update path at commit,
   which applies clusterRole but silently drops the parent link, so every
   CLI-scaled member landed role-stamped and orphaned from parent-chain
   membership walks (ha status, scale rosters, the platform's cluster-primacy
   guard, the admin monitors). The volume record goes fully detached too (no
   serviceId; the mount is declared by the patch). With the cause gone, the
   post-commit "platform dropped the parent link" warning goes with it.
   The image, the cluster's canvas group, and the sibling's whole deploy
   config (healthcheck, region placement -- a node cloned into a different
   region than its cluster replicates cross-region forever) ride the patch.

2. Scale-down deletions are STAGED (volume first, then the service) instead
   of issued as direct ServiceDelete/VolumeDelete calls, so the whole scale
   commits atomically and the platform's cluster-primacy commit guard can
   inspect the patch it exists to inspect.

3. Replica scale-down never deletes the node currently ACTING as primary.
   Deletion order is by node number, and after a failover the primary can be
   any replica: ha scale now probes the live role first, removes a
   lower-numbered replica instead where the count allows it, and refuses
   with the switchover remedy where it does not; when no member answers it
   degrades to a warning (same posture as revert's primacy precheck).

Also: the internal (coordinator) identity variable moves to joining-nodes-
only, matching the replica identity scope #1159 set (coordinators restarting
together is quorum loss); the routing-edge lookup excludes the pooler
through the engine's declared pooling spec instead of an image name compiled
into the filter.

cargo test: 1312 pass. The stub-backboard scale-up test now also pins the
bare service record (environmentId/source null) and the staged
isCreated/image; new stub tests pin the staged scale-down tombstones, the
primary being skipped in favor of a lower-numbered replica, and the refusal
with the switchover remedy when the count cannot be honored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/patch Author patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant