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

IHostConnectivityProbe / DriverHostStatus: dead surface — build the publisher or delete it #521

New Issue
Closed
opened 2026-07-27 18:55:13 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-07-27 18:55:13 -04:00
Owner
Copy Link
Copy Source

Split out of deferment.md §3.2 / Gitea #518. The IRediscoverable half of #518 is fixed — DriverInstanceActor now consumes it and surfaces a re-browse prompt on /hosts. This tracks the other half, which is a keep-or-delete decision, not a bug.

What is true today

  • IHostConnectivityProbe is implemented by 11 drivers (Modbus, FOCAS, TwinCAT, OpcUaClient, AbLegacy, S7, Sql, Galaxy, MTConnect, Mqtt, AbCip).
  • GetHostStatuses() has zero production call sites. The only non-driver references are the interface declaration and a comment in UnwrappedCapabilityCallAnalyzer explaining why the analyzer exempts it.
  • OnHostStatusChanged has exactly one += in src/ — GalaxyDriver.cs:221, which is intra-driver (its aggregator re-raising onto its own interface). Every other subscription is in tests/.
  • The DriverHostStatus entity + DbSet + table exist and nothing ever writes a row. Every DriverHostStatuses reference in the repo is the DbSet declaration plus DriverHostStatusTests.
  • The entity's own doc-comment describes a component that does not exist: "The publisher hosted service on the Server side subscribes to every registered driver's OnHostStatusChanged and upserts rows on transitions + periodic liveness heartbeats."
  • DriverInstanceResilienceStatus.cs:12 claims "The Admin UI left-joins this table on DriverHostStatus for display" — that join exists in no razor file.

There is no permanently-empty UI table. /hosts reads Akka membership, IDriverStatusSnapshotStore and ConfigDB ClusterNodes/DriverInstances — never DriverHostStatus. So the dead surface is invisible: an empty table, an unused DbSet, and 11 driver implementations of a method nobody calls.

The decision

Do not delete on inference. The table was re-created in the v3 initial migration (20260715230637_V3Initial.cs:108-121), so someone carried it forward deliberately. Confirm intent first.

Either:

(a) Build the missing publisher. A hosted service subscribing to every registered driver's OnHostStatusChanged, upserting DriverHostStatus rows on transition plus a liveness heartbeat, and a /hosts section that reads them. This is what the entity doc already describes. Note the two halves are separable — OnHostStatusChanged (the event) is the useful half; GetHostStatuses() (the snapshot pull) is only needed to prime state at startup.

(b) Delete it. Drop GetHostStatuses() from the interface, the 11 driver implementations, the DbSet, the table (migration), and DriverHostStatusTests. Fix the two doc-comments that describe the non-existent publisher and the non-existent Admin UI join.

Per-host connectivity is genuinely useful for a multi-device driver (a FOCAS or AbLegacy instance owning several PLCs), where today one unreachable device is invisible behind an otherwise-Healthy driver row. That is the argument for (a).

Split out of `deferment.md` §3.2 / Gitea #518. The `IRediscoverable` half of #518 is **fixed** — `DriverInstanceActor` now consumes it and surfaces a re-browse prompt on `/hosts`. This tracks the other half, which is a **keep-or-delete decision, not a bug**. ## What is true today - `IHostConnectivityProbe` is implemented by **11 drivers** (Modbus, FOCAS, TwinCAT, OpcUaClient, AbLegacy, S7, Sql, Galaxy, MTConnect, Mqtt, AbCip). - `GetHostStatuses()` has **zero production call sites**. The only non-driver references are the interface declaration and a comment in `UnwrappedCapabilityCallAnalyzer` explaining why the analyzer exempts it. - `OnHostStatusChanged` has exactly one `+=` in `src/` — `GalaxyDriver.cs:221`, which is intra-driver (its aggregator re-raising onto its own interface). Every other subscription is in `tests/`. - The `DriverHostStatus` entity + DbSet + table exist and **nothing ever writes a row**. Every `DriverHostStatuses` reference in the repo is the DbSet declaration plus `DriverHostStatusTests`. - The entity's own doc-comment describes a component that does not exist: *"The publisher hosted service on the Server side subscribes to every registered driver's `OnHostStatusChanged` and upserts rows on transitions + periodic liveness heartbeats."* - `DriverInstanceResilienceStatus.cs:12` claims *"The Admin UI left-joins this table on DriverHostStatus for display"* — that join exists in no razor file. **There is no permanently-empty UI table.** `/hosts` reads Akka membership, `IDriverStatusSnapshotStore` and ConfigDB `ClusterNodes`/`DriverInstances` — never `DriverHostStatus`. So the dead surface is invisible: an empty table, an unused DbSet, and 11 driver implementations of a method nobody calls. ## The decision **Do not delete on inference.** The table was *re-created* in the v3 initial migration (`20260715230637_V3Initial.cs:108-121`), so someone carried it forward deliberately. Confirm intent first. Either: **(a) Build the missing publisher.** A hosted service subscribing to every registered driver's `OnHostStatusChanged`, upserting `DriverHostStatus` rows on transition plus a liveness heartbeat, and a `/hosts` section that reads them. This is what the entity doc already describes. Note the two halves are separable — **`OnHostStatusChanged` (the event) is the useful half**; `GetHostStatuses()` (the snapshot pull) is only needed to prime state at startup. **(b) Delete it.** Drop `GetHostStatuses()` from the interface, the 11 driver implementations, the DbSet, the table (migration), and `DriverHostStatusTests`. Fix the two doc-comments that describe the non-existent publisher and the non-existent Admin UI join. Per-host connectivity is genuinely useful for a multi-device driver (a FOCAS or AbLegacy instance owning several PLCs), where today one unreachable device is invisible behind an otherwise-Healthy driver row. That is the argument for (a).
dohertj2 commented 2026-07-30 04:55:41 -04:00
Author
Owner
Copy Link
Copy Source

Resolved on master abda6f9e — but not the way this issue framed the choice, so the difference is worth recording.

Option (a) as written was not buildable

The publisher this issue (and the entity's own doc-comment) describes — driver nodes upserting DriverHostStatus rows — cannot exist. Per-cluster mesh Phase 4 gates AddOtOpcUaConfigDb on the admin role in Program.cs, so a driver-only node has no ConfigDb connection to write rows with. The design predates that change and was never revisited.

So the capability was kept and the transport changed instead.

What shipped

DriverHealthChanged.HostStatuses carries the probe result to /hosts as a Hosts column. That channel already reached the page, already survives the mesh split via the Phase 5 gRPC telemetry stream, and already replays a last-value snapshot on re-subscribe — so per-host state re-primes after a reconnect without a durable store. Both halves of the interface now do what this issue says they are for: OnHostStatusChanged triggers a prompt publish, GetHostStatuses() is the source of truth.

The argument for (a) in the issue is exactly what the column delivers: a multi-device driver stays aggregate-Healthy while one of its devices is unreachable, which the driver-level state chip structurally cannot express.

Dropped: the DriverHostStatus entity, enum, DbSet, model config, tests, and table (migration DropDriverHostStatusTable). Empty on every deployment, so the scaffolder's data-loss warning is moot; Down() recreates it exactly.

Two traps, both falsified against the prod code

  • The host digest must be in the publish fingerprint. On a single-host-down transition every other fingerprint component is unchanged, so the dedup would swallow precisely the publish carrying the news — the same trap that bit the rediscovery signal in #518. Removing it turns the guard test red; verified by doing it.
  • null (no probe) must stay distinct from empty (probe, no hosts). proto3 cannot tell an absent repeated field from an empty one, hence an explicit has_host_statuses flag. Collapsing them would render every probe-less driver as one whose devices are all fine.

Live-gated on docker-dev

All three states render against real drivers: — (Calculation, no probe), 1 host (healthy Modbus/Sql), 1 / 1 DOWN for the faulted OpcUaClient with tooltip opc.tcp://10.100.0.35:50000: Unknown since 2026-07-30 08:51:23Z.

That tooltip vindicates counting Unknown as degraded — the faulted driver's probe never completed a tick, so treating Unknown as healthy would have shown the one genuinely broken driver as fine.

The cross-mesh gRPC path was verified, not assumed. site-a is a separate Akka mesh, so its telemetry crosses the Phase 5 stream the proto change targets. It first showed —, which is also what a broken mapper produces; rebuilding site-a turned it into 1 host, proving the presence flag round-trips over the real wire and that the — was version skew. Mixed-version fleets therefore degrade to —, not to a false 0 hosts.

Found en route — filed, not folded in

DriverInstanceResilienceStatus is the identical defect: no writer, no reader, only a DbSet declaration, while the live data rides the driver-resilience-status telemetry channel. Its doc-comment now states that instead of describing the sampler and AdminUI join that were never built. Tracked as #524 rather than widening this schema change beyond what was asked.

Resolved on master `abda6f9e` — but **not the way this issue framed the choice**, so the difference is worth recording. ## Option (a) as written was not buildable The publisher this issue (and the entity's own doc-comment) describes — driver nodes upserting `DriverHostStatus` rows — **cannot exist**. Per-cluster mesh Phase 4 gates `AddOtOpcUaConfigDb` on the `admin` role in `Program.cs`, so a **driver-only node has no ConfigDb connection to write rows with**. The design predates that change and was never revisited. So the capability was kept and the *transport* changed instead. ## What shipped `DriverHealthChanged.HostStatuses` carries the probe result to `/hosts` as a **Hosts** column. That channel already reached the page, already survives the mesh split via the Phase 5 gRPC telemetry stream, and already replays a last-value snapshot on re-subscribe — so per-host state re-primes after a reconnect without a durable store. Both halves of the interface now do what this issue says they are for: `OnHostStatusChanged` triggers a prompt publish, `GetHostStatuses()` is the source of truth. The argument for (a) in the issue is exactly what the column delivers: a multi-device driver stays aggregate-`Healthy` while one of its devices is unreachable, which the driver-level state chip structurally cannot express. Dropped: the `DriverHostStatus` entity, enum, DbSet, model config, tests, and table (migration `DropDriverHostStatusTable`). Empty on every deployment, so the scaffolder's data-loss warning is moot; `Down()` recreates it exactly. ## Two traps, both falsified against the prod code - **The host digest must be in the publish fingerprint.** On a single-host-down transition every *other* fingerprint component is unchanged, so the dedup would swallow precisely the publish carrying the news — the same trap that bit the rediscovery signal in #518. Removing it turns the guard test red; verified by doing it. - **null (no probe) must stay distinct from empty (probe, no hosts).** proto3 cannot tell an absent repeated field from an empty one, hence an explicit `has_host_statuses` flag. Collapsing them would render every probe-less driver as one whose devices are all fine. ## Live-gated on docker-dev All three states render against real drivers: `—` (Calculation, no probe), `1 host` (healthy Modbus/Sql), `1 / 1 DOWN` for the faulted OpcUaClient with tooltip `opc.tcp://10.100.0.35:50000: Unknown since 2026-07-30 08:51:23Z`. That tooltip vindicates counting `Unknown` as degraded — the faulted driver's probe never completed a tick, so treating Unknown as healthy would have shown the one genuinely broken driver as fine. **The cross-mesh gRPC path was verified, not assumed.** site-a is a separate Akka mesh, so its telemetry crosses the Phase 5 stream the proto change targets. It first showed `—`, which is *also* what a broken mapper produces; rebuilding site-a turned it into `1 host`, proving the presence flag round-trips over the real wire and that the `—` was version skew. Mixed-version fleets therefore degrade to `—`, not to a false `0 hosts`. ## Found en route — filed, not folded in `DriverInstanceResilienceStatus` is the **identical defect**: no writer, no reader, only a DbSet declaration, while the live data rides the `driver-resilience-status` telemetry channel. Its doc-comment now states that instead of describing the sampler and AdminUI join that were never built. Tracked as #524 rather than widening this schema change beyond what was asked.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#521
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?