dohertj2/lmxopcua
1
0
Fork 0
Code Issues 8 Pull Requests Actions Packages Projects Releases Wiki Activity

docs(mesh): record phases 0a/0b as done and plan phase 1

Browse Source 
The design doc still described the downing gap and the RoleLeader derivation as
open. Both shipped earlier today, so 6.2 and 4 now carry the fix and its pin
instead of the diagnosis, and 7 marks 0a/0b done. 0a's live gate stays open and
is explicitly deferred to phase 7: the failure only appears in a 1-vs-1 split and
docker-dev is a single six-node mesh, so there is no rig on which to drill it
until the meshes are actually split.

The phase 1 plan lands with one finding that changes its shape. The design pairs
"ClusterNode gains address columns" with "coordinator sources its expected-ack set
from the DB" as though they were one change; they are independent, and the second
is far smaller than it reads. ClusterNode.NodeId is ALREADY in the coordinator's
identifier space — the rig seeds central-1:4053 and so on, because
NodeDeploymentState.NodeId is FK-bound to it and the coordinator's
membership-derived ids already satisfy that FK. That the deploy path works today
is standing proof the two sets agree.

What is not small is the semantics. Membership-derived and DB-derived sets differ
on a configured-but-stopped node: today the deployment seals green without it —
telling the operator the fleet is deployed when it is not — and afterwards it
would fail at the apply deadline. Failing is the better behaviour and is required
once central cannot see cluster membership, but it needs an escape hatch or a node
down for maintenance blocks every deploy. ClusterNode.Enabled already is that
hatch, so the plan filters on it and gates the phase on a live check that proves
both halves: a stopped node fails the deploy legibly, and disabling it lets the
deploy seal.

The plan also adds a task the design does not have: AkkaPort duplicates the node's
own Cluster:Port with nothing making them agree, and an unenforced duplicated
address is the same shape as the Modbus/ModbusTcp and TwinCat/Focas drifts already
recorded here.

Phases 2-7 remain unstarted. They change the running data path and rewrite the
rig, and each needs its own plan and live gate.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-21 18:46:10 -04:00
parent b2d19a1730
commit 84c6524edc
2 changed files with 217 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/2026-07-21-per-cluster-mesh-design.md
+26 -10
View File
@@ -1,9 +1,10 @@
# Per-Cluster Akka Mesh — design
> **Status:** design for review. No implementation planned yet.
> **Status:** design for review. **Phases 0a and 0b are DONE and merged** (2026-07-21); Phases 1–7
> are not started and each needs its own plan.
> **Decisions settled 2026-07-21** — see §6: one central DB with driver nodes disconnected from it,
> the two-node keep-oldest gap CLOSED upstream and still live here (§6.2 — most urgent item), and
> their auth posture matched for now.
> the two-node keep-oldest gap closed upstream and now closed here too (§6.2), and their auth posture
> matched for now.
## 1. What this changes
@@ -142,6 +143,12 @@ the dual-primary shape archreview 03/S4 exists to prevent.
and should not wait for this design.** It also happens to be exactly what the separate-mesh model
needs, so it is not throwaway work.
**FIXED 2026-07-21 (Phase 0b).** `RedundancyStateActor.BuildSnapshot` selects the oldest Up member
carrying the `driver` role, matching singleton placement; `NodeRedundancyState.IsRoleLeaderForDriver`
was renamed `IsDriverPrimary` so the contract stops asserting the old derivation. Pinned by
`RedundancyPrimaryElectionTests`, which builds a real two-node cluster in which the oldest node holds
the *higher* address, so the two orderings genuinely disagree.
## 5. Target architecture
Mirror ScadaBridge's shape, adapted to OtOpcUa's existing substrate:
@@ -231,11 +238,20 @@ Its commit message carries the analysis, live-proven on their rig:
`keep-oldest { down-if-alone = on }`, `stable-after = 15s`, with
`BuildClusterOptions` setting the matching typed `KeepOldestOption { DownIfAlone = true }`.
So a two-node OtOpcUa pair **cannot fail over when the oldest node crashes**: the survivor downs
itself. That is a total-outage defect in the redundancy feature itself, it is independent of this
design, and the sister project has already done the analysis and shipped the fix. **It should be
Phase 0 alongside the oldest-Up role derivation**, and it needs its own live gate — a
crash-the-oldest drill, since the failure only appears in the 1-vs-1 case.
So a two-node OtOpcUa pair **could not fail over when the oldest node crashed**: the survivor
downed itself. That was a total-outage defect in the redundancy feature itself, independent of this
design.
**FIXED 2026-07-21 (Phase 0a).** `Cluster:SplitBrainResolverStrategy` now selects the downing
provider and defaults to `auto-down` — Akka's `AutoDowning` with a 15s window — so the leader among
the reachable members downs the unreachable peer and a crash of either node fails over. `keep-oldest`
remains selectable for deployments that prefer partition-safety. Pinned by
`SplitBrainResolverActivationTests`, which asserts the provider read back off a running ActorSystem
rather than the configured one. See `docs/Redundancy.md`.
**Its live gate is still outstanding**, and cannot run on today's rig: the failure only appears in a
1-vs-1 split, and `docker-dev` is a single six-node mesh. The crash-the-oldest drill therefore lands
with Phase 6/7, which makes every mesh exactly two nodes.
### 6.3 DECIDED — match ScadaBridge's auth posture for now
@@ -250,8 +266,8 @@ Deliberately not a task plan — per-phase plans follow, one at a time.
| Phase | Content | Independent of the split? |
|---|---|---|
| 0a | Downing strategy: keep-oldest cannot survive an oldest-node crash in a 2-node cluster (§6.2) | **Yes** — most urgent |
| 0b | Oldest-Up role derivation (§4) | **Yes** |
| 0a | Downing strategy: keep-oldest cannot survive an oldest-node crash in a 2-node cluster (§6.2) | **Yes** — **DONE 2026-07-21**, live gate deferred to Phase 7 |
| 0b | Oldest-Up role derivation (§4) | **Yes** — **DONE 2026-07-21** |
| 1 | `ClusterNode` gains Akka + gRPC address columns; coordinator sources its expected-ack set from the DB | Yes |
| 2 | Comm actors + receptionist registration; ClusterClient transport; deploy notify + acks across the boundary | No |
| 3 | Config fetch-and-cache: artifact served by central, driver nodes read LocalDb (§6.1) | No |
docs/plans/2026-07-21-per-cluster-mesh-phase1.md
+191
View File
@@ -0,0 +1,191 @@
# Per-Cluster Mesh — Phase 1 plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
**Parent design:** [`2026-07-21-per-cluster-mesh-design.md`](2026-07-21-per-cluster-mesh-design.md).
Phases 0a and 0b are done and merged; this is the next phase.
**Goal:** Give `ClusterNode` the transport addresses central will need to reach each cluster
directly, and cut `ConfigPublishCoordinator`'s last dependency on shared cluster membership by
sourcing its expected-ack set from those rows.
**Architecture:** Additive schema change plus one substitution inside the coordinator. Nothing else
moves; the deploy channel stays on DistributedPubSub until Phase 2.
**Status:** NOT STARTED. Read §"The decision this phase forces" before executing — it changes
deploy-seal semantics and should be confirmed, not assumed.
---
## Why this is smaller than the design implies
The design says "`ClusterNode` gains Akka + gRPC address columns; coordinator sources its
expected-ack set from the DB", which reads as though the two are one change. They are not, and the
second does not depend on the first.
`ConfigPublishCoordinator.DiscoverDriverNodes` builds `NodeId.Parse($"{host}:{port}")` from
`Cluster.State.Members`. `ClusterNode.NodeId` is **already in that identifier space** — the rig seeds
`central-1:4053`, `site-a-1:4053` and so on (`docker-dev/seed/seed-clusters.sql:65`), and its comment
says so explicitly, because `NodeDeploymentState.NodeId` is FK-bound to `ClusterNode.NodeId` and a
mismatch throws FK 547 on deploy. The coordinator already writes `NodeDeploymentState` rows keyed by
the membership-derived id and they already satisfy that FK — which is standing proof the two sets
agree today.
So the coordinator switch is a substitution inside one private method, and the address columns are
purely forward-looking groundwork for Phase 2's ClusterClient and Phase 5's gRPC dial.
## The decision this phase forces
**Membership-derived and DB-derived expected-ack sets are not the same set, and the difference is a
behaviour change on the deploy path.**
| Situation | Today (membership) | After (DB rows) |
|---|---|---|
| Configured node is running | expected, acks, seals | same |
| Configured node is **stopped** | not expected — deploy seals **successfully** without it | expected — deploy waits, then **fails at the apply deadline** |
| Node running but not in `ClusterNode` | expected (and then FK 547 on the state row) | not expected; its ack is discarded |
The middle row is the one that matters. Today a deployment can seal green while a node that should
have received it was down and did not — the operator is told the fleet is deployed when it is not.
Failing instead is arguably the correct behaviour and is the reason the design moves this to the DB
(central cannot see cluster membership once the meshes split).
But it is a behaviour change, and it needs an operator escape hatch or a node taken down for
maintenance blocks every deployment. `ClusterNode.Enabled` already exists and is exactly that hatch,
so the query filters on it: **`Enabled = 1` rows are expected to ack; disabled rows are not.**
**Confirm this before executing Task 3.** If the preference is to keep sealing green past a down
node, the alternative is to intersect the DB set with current membership, which preserves today's
behaviour but does not survive the mesh split — it would have to be revisited in Phase 2 anyway.
---
## Task 1: Address columns on `ClusterNode`
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** none (Task 2 migrates what this declares)
**Files:**
- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs`
- Modify: the `ClusterNode` entity configuration in `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/` (find the `IEntityTypeConfiguration<ClusterNode>` or the `OnModelCreating` block)
- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/` (mirror an existing entity-mapping test)
Add two nullable columns beside the existing `OpcUaPort` / `DashboardPort`:
- `int AkkaPort` — default `4053`. The remoting port central will use to build the ClusterClient
contact point. Non-nullable with a default, because every node has one.
- `int? GrpcPort` — nullable, no default. The Phase 5 telemetry-stream port. Nullable is deliberate:
it does not exist yet, and a non-null default would assert a port nothing listens on.
Document on each property that these are **central's dial targets**, not the node's own binding
configuration — the node binds from `Cluster:Port` in its own appsettings, and these columns
duplicate that value for a reader that cannot see the node's config. That duplication is the reason
Task 4 exists.
**Step 1:** Write a mapping test asserting a round-tripped `ClusterNode` preserves both values and
that `AkkaPort` defaults to 4053.
**Step 2:** Run it; expect failure to compile.
**Step 3:** Add the properties + mapping.
**Step 4:** Run; expect pass.
**Step 5:** Commit.
## Task 2: EF migration
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** none
**Files:**
- Create: a new migration under the Configuration project's `Migrations/` folder
```bash
dotnet ef migrations add AddClusterNodeTransportPorts \
--project src/Core/ZB.MOM.WW.OtOpcUa.Configuration
```
Verify the generated `Up` adds `AkkaPort` with `defaultValue: 4053` and `GrpcPort` as nullable, and
that `Down` drops both. **Check the migration is additive only** — an `AlterColumn` on anything else
means the model had drifted from the last migration and that drift must be understood before this
ships, not carried along by it.
## Task 3: Coordinator sources the expected-ack set from `ClusterNode`
**Classification:** high-risk — this is the deploy path
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:**
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs` (`DiscoverDriverNodes`, ~line 262, and the class doc comment at line 21 which states the membership rule)
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorTests.cs`
Replace the `Cluster.State.Members` scan with a query over enabled `ClusterNode` rows. Keep
`NodeIdComparer` — the case-insensitivity rationale in its comment applies unchanged, and SQL
collation makes it more relevant, not less.
Points to get right:
- **Filter on `Enabled`.** See "The decision this phase forces".
- **Scope to the deployment's cluster if the deployment is cluster-scoped.** `DeploymentArtifact.ResolveClusterScope`
already defines that scoping; a fleet-wide deployment expects every enabled node, a cluster-scoped
one expects only that cluster's. Getting this wrong makes every cluster-scoped deploy hang on nodes
that were never sent it. **Read `ResolveClusterScope` before writing the query.**
- **The empty-set branch still matters.** `HandleDispatch` seals immediately when the set is empty,
and its log message says "no driver-role members in cluster" — reword it, because the reason is now
"no enabled ClusterNode rows", which is a configuration error rather than a topology observation.
- The DB is read inside `HandleDispatch`, which already opens a context; reuse it rather than opening
a second.
**Tests to add:** enabled-only filtering; cluster-scope filtering; empty set seals with the new
reason; an ack from a node absent from `ClusterNode` is discarded. **Positive control on each:**
delete the filter and confirm the test goes red — the existing suite passed throughout the
membership era, so a test that also passes against the old derivation is testing nothing.
## Task 4: Reconcile the duplicated port with the node's own config
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** none
`ClusterNode.AkkaPort` and the node's own `Cluster:Port` are the same fact in two places, and nothing
makes them agree. A node that binds 4054 while its row says 4053 is unreachable from central in Phase
2, and the symptom will be a silent absence of acks rather than an error.
Options, in preference order:
1. **Have the node assert its row on startup** — on join, compare `Cluster:Port` /
`Cluster:PublicHostname` to its own `ClusterNode` row and log an Error (not a Warning) on
mismatch. Cheap, no schema change, no write path from driver nodes to the ConfigDb (which Phase 4
removes anyway — so put this check on an **admin** node, reading the membership it can already see).
2. Deploy-time validation in `DraftValidator`, alongside the existing gates.
Pick one and implement it. Do not skip this task: an unenforced duplicated address is the exact shape
of the `Modbus`/`ModbusTcp` drift and the `TwinCat`/`Focas` drift already recorded in this repo.
## Task 5: Rig seed + docs
**Classification:** small
**Estimated implement time:** ~3 min
**Files:**
- Modify: `docker-dev/seed/seed-clusters.sql` — add `AkkaPort` to the six INSERTs (all 4053) so a
freshly-seeded rig matches a migrated one. Leave `GrpcPort` null.
- Modify: `docs/Configuration.md` — document both columns.
- Modify: `docs/plans/2026-07-21-per-cluster-mesh-design.md` — mark Phase 1 done in §7.
## Task 6: Live gate
**Classification:** high-risk
**Estimated implement time:** n/a — operator-driven
Offline tests cannot cover the substitution's real risk, which is that the DB set and the live set
disagree on the rig. Run on `docker-dev`:
1. Deploy with all six nodes up → seals green, six `NodeDeploymentState` rows, no FK 547.
2. Deploy a **cluster-scoped** config → expected set is that cluster's nodes only.
3. `docker stop` one node, deploy → **fails at the apply deadline** naming the missing node. This is
the new behaviour; confirm the failure is legible, because an operator will meet it.
4. Set that node's `Enabled = 0`, deploy again → seals green without it. This proves the escape
hatch, and is the check that makes step 3 acceptable to ship.
Record results in a gate doc beside this plan, per the Phase 1/Phase 2 precedent.