From 2e4ccf7fe9c58c9ebda87efade88aca452672edf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Jul 2026 05:03:57 -0400 Subject: [PATCH] chore(localdb): phase-2 DoD sweep Build: 0 errors solution-wide, and 0 warnings from every project this branch touches. The ~816 solution-wide warnings are pre-existing xUnit1051 / OTOPCUA0001 / CS86xx in untouched driver + client test projects. Tests: full solution run compared against a full run on a detached worktree at the pre-branch baseline 2e46d054. The two failure SETS are identical -- all 13 tests, same names, zero new failures. Net +26 tests: +3 Core.AlarmHistorian (drain gate), +6 Runtime (role view), +17 Host.IntegrationTests (migrator + convergence). Set comparison rather than counts, because the suite carries standing environment- and load-dependent failures a count would hide. The greps found real drift Task 6 missed -- eight live sites still naming the deleted SqliteStoreAndForwardSink, including the AdminUI /alarms/historian panel text, which is user-visible, and a in HistorianAdapterActor that resolved to nothing without warning. All repointed at LocalDbStoreAndForwardSink; CLAUDE.md's alarm-history paragraph now also records the LocalDb buffer and the primary-gated drain, and drops DatabasePath from the knob list. docs/AlarmTracking.md still promised an AlarmHistorianOptions.Validate() startup warning for a relative DatabasePath and an empty SharedSecret; both branches are gone, so it now says so. Code references to AlarmHistorian:DatabasePath reduce to exactly two intentional ones: AlarmSfLegacyMigrator.LegacyPathKey and its test. No `new SqliteConnection` remains anywhere in Core.AlarmHistorian. Recon doc gains the durable verification record: guard-deletion evidence for both vacuous passes, the two exact-set replicated-table pins (both assert set equality, so an added or a dropped registration fails), and the baseline test comparison with a per-failure account of why each of the 13 is not this branch's. Stops here per the plan. Task 8's live gate needs explicit go-ahead; nothing on this branch is to be merged. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- CLAUDE.md | 12 ++-- docs/AlarmTracking.md | 5 +- docs/drivers/Historian.Wonderware.md | 2 +- ...7-20-localdb-adoption-phase2.md.tasks.json | 5 +- docs/plans/2026-07-20-localdb-phase2-recon.md | 69 +++++++++++++++++++ .../GatewayAlarmHistorianWriter.cs | 2 +- ...wayHistorianServiceCollectionExtensions.cs | 2 +- .../Components/Pages/AlarmsHistorian.razor | 5 +- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 2 +- .../Historian/HistorianAdapterActor.cs | 2 +- .../ServiceCollectionExtensions.cs | 2 +- .../GatewayAlarmWriterFactoryTests.cs | 2 +- 12 files changed, 92 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0bed5c12..2e2db504 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -361,11 +361,13 @@ authorization uses the standard `AccessLevels.HistoryRead` bit set at materializ ### Alarm-history path (`AlarmHistorian` section) Alarm events are written through `GatewayAlarmHistorianWriter` (the gateway **`SendEvent`** path) behind -the durable **`SqliteStoreAndForwardSink`** — `AlarmHistorian:Enabled=true` swaps the `NullAlarmHistorianSink` -default for the SQLite store-and-forward queue, whose drain worker forwards batches to the gateway and uses -per-event outcomes to decide retry vs. dead-letter (never throws). The `AlarmHistorian` section carries -only the `Enabled` gate + the SQLite knobs (`DatabasePath`, `DrainIntervalSeconds`, `Capacity`, -`DeadLetterRetentionDays`, `BatchSize`, `MaxAttempts`) — the downstream gateway connection +the durable **`LocalDbStoreAndForwardSink`** — `AlarmHistorian:Enabled=true` swaps the `NullAlarmHistorianSink` +default for the store-and-forward queue, which buffers into the node's consolidated LocalDb as the +replicated `alarm_sf_events` table (see the LocalDb section) and whose drain worker forwards batches to +the gateway and uses per-event outcomes to decide retry vs. dead-letter (never throws). **Only the node +holding the Primary role drains** (a gated node reports `HistorianDrainState.NotPrimary`). The +`AlarmHistorian` section carries only the `Enabled` gate + the queue knobs (`DrainIntervalSeconds`, +`Capacity`, `DeadLetterRetentionDays`, `BatchSize`, `MaxAttempts`) — the downstream gateway connection (endpoint/key/TLS) is sourced from the `ServerHistorian` section. **Alarm-history `ReadEvents` requires the target gateway deployed with `RuntimeDb:EventReadsEnabled=true`** (the C2 SQL event-read workaround). diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 399359b7..ce5e3c63 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -383,8 +383,9 @@ AB CIP ALMD) route to AVEVA Historian via the HistorianGateway: capacity, and dead-letter retention are tunable via the `AlarmHistorian` config section (`DrainIntervalSeconds`, `Capacity`, `DeadLetterRetentionDays`); `AlarmHistorianOptions.Validate()` logs a - startup warning for an empty `SharedSecret`, a relative `DatabasePath`, - or a non-positive knob. + startup warning for a non-positive knob. (It no longer warns about + `SharedSecret` or `DatabasePath` — both keys are gone: the connection + comes from `ServerHistorian`, and the queue lives in the LocalDb.) - `HistorianAdapterActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianAdapterActor.cs`) subscribes to the cluster `alerts` DPS topic, translates each diff --git a/docs/drivers/Historian.Wonderware.md b/docs/drivers/Historian.Wonderware.md index 839ca7fc..ebf18c0b 100644 --- a/docs/drivers/Historian.Wonderware.md +++ b/docs/drivers/Historian.Wonderware.md @@ -13,7 +13,7 @@ OtOpcUa now consumes the **`ZB.MOM.WW.HistorianGateway`** sidecar through the Gi - **HistoryRead** → `GatewayHistorianDataSource` over the `ServerHistorian` appsettings section. - **Alarm history** → `GatewayAlarmHistorianWriter` (the gateway `SendEvent` path) behind the durable - `SqliteStoreAndForwardSink`; alarm-history `ReadEvents` needs the gateway running + `LocalDbStoreAndForwardSink`; alarm-history `ReadEvents` needs the gateway running `RuntimeDb:EventReadsEnabled=true`. - **Continuous historization** → a crash-safe FasterLog outbox + `ContinuousHistorizationRecorder` draining to the gateway's `WriteLiveValues` (`ContinuousHistorization` section); needs the gateway diff --git a/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json b/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json index 104b2e38..27c13a64 100644 --- a/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json +++ b/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json @@ -64,12 +64,13 @@ { "id": 7, "subject": "Task 7: DoD sweep (offline) \u2014 STOP and report after this", - "status": "pending", + "status": "completed", "blockedBy": [ 4, 5, 6 - ] + ], + "note": "Build: 0 errors solution-wide; 0 warnings from every project this branch touches (the ~816 solution-wide warnings are pre-existing xUnit1051/OTOPCUA0001/CS86xx in untouched driver + client test projects). GREPS FOUND REAL DRIFT Task 6 missed: 8 live sites still named the deleted SqliteStoreAndForwardSink - CLAUDE.md, the AdminUI /alarms/historian panel text (user-visible), HistorianAdapterActor (a that did NOT warn), Runtime + Host + 2 Driver.Historian.Gateway doc comments, 1 gateway test comment, docs/drivers/Historian.Wonderware.md; plus docs/AlarmTracking.md still claimed a Validate() startup warning for a relative DatabasePath (that branch is gone). All fixed. Code refs to AlarmHistorian:DatabasePath now reduce to exactly two intentional ones: AlarmSfLegacyMigrator.LegacyPathKey and its test. No 'new SqliteConnection' in Core.AlarmHistorian. Both exact-set pins assert the 3-table set, both directions. Guard-deletion evidence (both vacuous passes + the pin inventory) consolidated into the recon doc." }, { "id": 8, diff --git a/docs/plans/2026-07-20-localdb-phase2-recon.md b/docs/plans/2026-07-20-localdb-phase2-recon.md index e078a2c7..1d858f5a 100644 --- a/docs/plans/2026-07-20-localdb-phase2-recon.md +++ b/docs/plans/2026-07-20-localdb-phase2-recon.md @@ -300,3 +300,72 @@ Related: `Program.cs:170-173` shows an `AlarmHistorian:Enabled=true` / `ServerHistorian:Enabled=false` deployment is explicitly supported and warned about — which is exactly the rig's shape. Task 6 must confirm `ServerHistorianOptionsValidator` does not fail host start for a disabled section with an empty `Endpoint`. + +--- + +## Guard-deletion evidence (recorded at the Task 7 DoD sweep) + +Every guard this phase adds was proved by deleting it and watching the tests go red. Two of those +runs found tests that would have passed over a broken implementation. Both are recorded here because +the *shape* of the vacuity is reusable, not just the fix. + +### Control 1 — remove `IRedundancyRoleView.Publish` from `DriverHostActor` + +**3 of 6** `DriverHostActorRoleViewTests` went red. The other three assert +`ShouldServiceAsPrimary == true`, which is exactly the value the view is **seeded** with — so they +cannot distinguish "published true" from "never published at all". That split is expected and +acceptable *given* the three that do go red pin the false cases, which only a real publish can +produce. Worth knowing before someone reads a future 3/6 result as a regression. + +### Control 2 — remove `RegisterReplicated("alarm_sf_events")` from `LocalDbSetup.OnReady` + +**3 of 4** `AlarmSfConvergenceTests` went red immediately. The fourth, +`TheSameEventOnBothNodes_ConvergesToOneRow`, **passed vacuously**: with replication switched off +each node trivially held its own single row, and "count == 1 on both nodes" was satisfied by two +completely unconverged databases. The assertion could not tell convergence from isolation. + +**Fix:** enqueue a second, *distinct* event on node B and require **both** nodes to hold two rows. +Isolation now yields 1 and 2; only convergence yields 2 and 2. Re-run under the control: red. +Control restored: all 4 green. + +The general trap — an assertion whose expected value is also what a *disabled* system produces — +is the same one recorded in the `#485`/`#486` unreadable-artifact defect class. + +### Exact-set replicated-table pins + +Two, not one, and both assert set equality (ordered `ShouldBe` against the literal three-table list, +so an added *or* dropped registration fails): + +- `LocalDbSetupTests` — drives the production `OnReady` callback directly. +- `LocalDbWiringTests` — resolves `ILocalDb` from the full driver DI graph, which is what catches a + registration that never reaches a running host. + +## Test-suite evidence (Task 7 DoD sweep) + +Full solution run on the branch, compared against a **full solution run on a detached worktree at +the pre-branch baseline `2e46d054`**. Comparing failure *sets*, not counts, because the suite has a +standing set of environment- and load-dependent failures that a raw count would hide. + +**The two failure sets are identical — all 13 tests, same names, both runs. Zero new failures.** + +| Assembly | Baseline | Branch | Delta | +|---|---|---|---| +| `Core.AlarmHistorian.Tests` | 0 failed / 29 passed | 0 failed / **32** passed | **+3** (the drain-gate tests; 29 ported from the old sink's suite) | +| `Runtime.Tests` | 1 failed / 418 passed | 1 failed / **424** passed | **+6** (`DriverHostActorRoleViewTests`) | +| `Host.IntegrationTests` | 2 failed / 169 passed | 2 failed / **186** passed | **+17** (migrator + convergence + pin updates) | + +The 13 standing failures, and why each is not this branch's: + +- **3 × `DriverTypeNamesGuardTests`** — `ArgumentNullException (secretResolver)` out of + `GalaxyDriverFactoryExtensions.Register`, reached by reflection. A real pre-existing defect in a + guard test, reproduced exactly on the baseline worktree. Unrelated to this phase. +- **4 × `AbLegacy.IntegrationTests`**, **3 × `OpcUaClient.IntegrationTests`**, + **1 × `DriverProbeHandshakeE2eTests.AbCip_Green_AgainstSim`** — driver fixtures on the + `10.100.0.35` docker host are not running. +- **1 × `RoslynVirtualTagEvaluatorTests.Evaluate_racing_ClearCompiledScripts_never_fails_with_disposed`** + — times out under full-suite load; passes in isolation. +- **1 × `ContinuousHistorizationRecorderTests.Retry_after_writer_failure_eventually_acks`** — fails + under full-suite load ("writer must have been called at least twice"), and `Runtime.Tests` passes + **425/425** when run alone. Also fails on the baseline. Worth noting for anyone reading a future + run: this one is *newly observed* here only because the previous sweep's log was truncated, not + because the branch introduced it. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs index 02eb624a..1817c8a3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs @@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway; /// /// backed by the HistorianGateway SendEvent path. The -/// drain worker behind SqliteStoreAndForwardSink calls +/// drain worker behind LocalDbStoreAndForwardSink calls /// and uses the returned per-event /// to decide retry vs. dead-letter, so this writer maps every /// gateway result — success ack, the published client's typed exception hierarchy, raw diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianServiceCollectionExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianServiceCollectionExtensions.cs index 6033bde7..4df51b33 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianServiceCollectionExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianServiceCollectionExtensions.cs @@ -47,7 +47,7 @@ public static class GatewayHistorian /// — the same single gateway the read path /// () targets. The Host's AddAlarmHistorian wiring supplies /// this as the concrete the durable - /// SqliteStoreAndForwardSink drain worker delegates to, sourcing the connection from the + /// LocalDbStoreAndForwardSink drain worker delegates to, sourcing the connection from the /// ServerHistorian section (endpoint/key/TLS) rather than the legacy Wonderware-shaped /// AlarmHistorian host/port. Resolves an and the writer's /// from , falling back to the null diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor index db03988c..a957ca51 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor @@ -18,8 +18,9 @@
Snapshot from the local node's HistorianAdapterActor. Default sink is a no-op (NullAlarmHistorianSink); production wires - SqliteStoreAndForwardSink draining to the HistorianGateway - (SendEvent) behind it. Polling every @PollSeconds s. + LocalDbStoreAndForwardSink — buffering into this node's LocalDb, and + draining to the HistorianGateway (SendEvent) only while this node holds + the Primary role. Polling every @PollSeconds s.
@if (_status is null) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index b9de794b..0da86839 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -136,7 +136,7 @@ if (hasDriver) // Config-gated durable alarm-historian sink. When the AlarmHistorian section is enabled this // overrides the NullAlarmHistorianSink default from AddOtOpcUaRuntime (last registration wins) - // with a SqliteStoreAndForwardSink draining to the gateway SendEvent writer. The alarm-write path + // with a LocalDbStoreAndForwardSink draining to the gateway SendEvent writer. The alarm-write path // targets the SAME single gateway as the read path, so its connection (endpoint/key/TLS) is sourced // from the ServerHistorian section. AlarmHistorianOptions supplies only the Enabled gate + the // SQLite store-and-forward knobs (consumed inside AddAlarmHistorian) — it carries no connection fields. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianAdapterActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianAdapterActor.cs index 93eb4142..af60b86b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianAdapterActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianAdapterActor.cs @@ -15,7 +15,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Historian; /// Thin actor wrapper around . Engine code (ScriptedAlarmActor, /// Galaxy native alarm bridge, AB CIP ALMD reader) tells s to this /// actor; the actor enqueues them on the sink fire-and-forget. Production deployments register -/// against IAlarmHistorianSink; the sink owns the +/// against IAlarmHistorianSink; the sink owns the /// durable queue + drain-to-HistorianGateway-SendEvent loop. The actor here owns nothing operational beyond /// the message contract — its job is to keep the engine actors on Akka's mailbox without blocking /// them on disk I/O or gateway round-trips. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index 7dc1e921..f00f9663 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -41,7 +41,7 @@ public static class ServiceCollectionExtensions /// /// Registers shared runtime services. Currently binds /// to as the default; production deployments - /// override this with SqliteStoreAndForwardSink wrapping the HistorianGateway alarm writer. + /// override this with LocalDbStoreAndForwardSink wrapping the HistorianGateway alarm writer. /// Call this BEFORE AddAkka. /// /// The service collection to register with. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmWriterFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmWriterFactoryTests.cs index ddbfc80a..868ab7f0 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmWriterFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmWriterFactoryTests.cs @@ -7,7 +7,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests; /// /// Alarm-write cutover seam test (T13). The Host's AddAlarmHistorian wiring drains the durable -/// SqliteStoreAndForwardSink through this factory, so it must yield the gateway-backed writer — +/// LocalDbStoreAndForwardSink through this factory, so it must yield the gateway-backed writer — /// sourcing the single gateway's connection from (endpoint/key/TLS), /// not the legacy Wonderware-shaped AlarmHistorian host/port. Built offline: the underlying /// channel dials lazily, so both the factory and the writer ctor perform no network I/O (a bogus,