lmxopcua/docs/plans/2026-07-20-localdb-phase2-recon.md

# LocalDb Phase 2 — Recon findings

**Date:** 2026-07-21 · **Branch:** `feat/localdb-phase2` · **Base:** `master` `2e46d054`

Answers Task 0 of `2026-07-20-localdb-adoption-phase2.md`. Every claim carries a `file:line`
citation against the base commit. Read the **Deviations** section last — two of them change the
shape of Tasks 2/3 from what the plan assumed.

---

## 1. The sink's table schema

**STOP condition does NOT fire** — confirmed by re-reading the executed DDL, as the plan's
pre-answer instructed.

Executed DDL: `SqliteStoreAndForwardSink.cs:652-670` (`InitializeSchema`). It matches the class
doc-comment at `:17-26` exactly — no drift between documented and executed schema.

```sql
CREATE TABLE IF NOT EXISTS Queue (
    RowId          INTEGER PRIMARY KEY AUTOINCREMENT,
    AlarmId        TEXT    NOT NULL,
    EnqueuedUtc    TEXT    NOT NULL,
    PayloadJson    TEXT    NOT NULL,
    AttemptCount   INTEGER NOT NULL DEFAULT 0,
    LastAttemptUtc TEXT    NULL,
    LastError      TEXT    NULL,
    DeadLettered   INTEGER NOT NULL DEFAULT 0
);
CREATE INDEX IF NOT EXISTS IX_Queue_Drain ON Queue (DeadLettered, RowId);
```

| Question | Answer |
|---|---|
| BLOB columns? | **None.** All 8 columns are TEXT/INTEGER. Payload is `PayloadJson TEXT NOT NULL`. No base64 conversion needed; the 171 KB chunk sizing is moot. |
| PK autoincrement? | **Yes** — `RowId INTEGER PRIMARY KEY AUTOINCREMENT`. The migrator must mint deterministic `mig-{node}-{legacyId}` ids and the new table needs a TEXT PK, as the plan specified. |
| Indices to carry | One: `IX_Queue_Drain (DeadLettered, RowId)` — the drain's covering index. `alarm_sf_events` wants the same shape over (status, insertion order). |
| Largest realistic payload | `PayloadJson` is a serialized `AlarmHistorianEvent` — 10 scalar fields (`AlarmHistorianEvent.cs`), no collections, no nesting. Low single-digit KB worst case; `Comment` is the only loosely-bounded field. |

**Legacy column list** for the migrator's `pragma_table_info` intersection check:
`RowId, AlarmId, EnqueuedUtc, PayloadJson, AttemptCount, LastAttemptUtc, LastError, DeadLettered`.

`RegisterReplicated` will accept this table: no BLOB, and the PK will be app-minted TEXT.

## 2. The public seam

`IAlarmHistorianSink` (`IAlarmHistorianSink.cs:23-34`) is **two members**:

```csharp
Task EnqueueAsync(AlarmHistorianEvent evt, CancellationToken cancellationToken);
HistorianSinkStatus GetStatus();
```

The concrete `SqliteStoreAndForwardSink` additionally exposes, and these are all consumed
somewhere, so the rewire must keep them:

| Member | Cited | Consumer |
|---|---|---|
| `StartDrainLoop(TimeSpan)` | `:189` | `ServiceCollectionExtensions.cs:105` (DI factory) |
| `DrainOnceAsync(CancellationToken)` | `:320` | tests only — the deterministic drive |
| `RetryDeadLettered()` | `:511` | operator action (AdminUI) |
| `CurrentBackoff` | `:676` | tests |
| `DebugCapacityProbeCount` | `:99` | tests |
| `DefaultCapacity` / `DefaultMaxAttempts` consts | `:49`,`:53` | `AlarmHistorianOptions.cs:38,45` |
| `IDisposable` | `:679` | DI + tests; **disposes the injected writer too** |

There is **no separate enqueue/dequeue/markDelivered/deadLetter interface** — the plan's item 2
guessed at one. Dequeue, delivered-marking and dead-lettering are all *private* to the sink
(`ReadBatch :532`, `DeleteRow :564`, `DeadLetterRow :573`, `BumpAttempt :587`). That is good news:
the rewire is entirely internal to one class and the public seam is untouched.

## 3. Semantics to preserve

All defaults live in **two** places that must stay in agreement:

| Semantic | Sink default | Options default | Enforced at |
|---|---|---|---|
| `Capacity` 1,000,000 | `:49` | `AlarmHistorianOptions.cs:38` | `EnforceCapacityFastPathAsync :271` → `EnforceCapacityAsync :602` |
| `MaxAttempts` 10 | `:53` | `:45` | drain loop `:429` |
| Dead-letter retention 30 d | `:50` | `:41` | `PurgeAgedDeadLetters :638`, run once per drain tick |
| `BatchSize` 100 | ctor arg `:116` | `:31` | `ReadBatch :542` |
| `DrainIntervalSeconds` 5 | — | `:34` | `StartDrainLoop` via `ServiceCollectionExtensions.cs:105` |

Behaviours that are **not** in the plan's list but are load-bearing, and which the cutover must
not silently drop:

- **Backoff ladder** 1s→2s→5s→15s→60s (`:57-64`), applied to the timer's *next due time*
  (`RescheduleDrain :224`) so an outage genuinely slows the cadence.
- **Overflow evicts oldest non-dead-lettered rows** with a WARN and a lifetime
  `EvictedCount` counter (`:630-635`) — the durability guarantee is explicitly bounded.
- **Corrupt-payload rows dead-letter immediately** for their own RowId so they cannot stall the
  queue head (`:344-361`).
- **Cardinality guard**: a writer returning ≠1 outcome per event is treated as a batch retry
  (`:393-404`).
- **In-memory row counter with periodic resync** every 10,000 enqueues (`:89-96`, `:271-290`) —
  a perf optimisation that avoids `COUNT(*)` on the hot enqueue path.
- **Post-dispose throwing contract**: `GetStatus`/`RetryDeadLettered`/`EnqueueAsync`/`StartDrainLoop`
  throw `ObjectDisposedException`; `DrainOnceAsync` returns quietly (`:322`). Pinned by a
  regression test (`SqliteStoreAndForwardSinkTests.cs:854-865`).

## 4. Drain-worker lifecycle — **the finding that reshapes Task 3**

> The plan asked "hosted service or actor?". **Neither.**

The drain worker is a **self-rescheduling one-shot `System.Threading.Timer` owned by the sink
itself** (`_drainTimer`, `:76`; started by `StartDrainLoop :189`; callback `DrainTimerCallback :199`;
re-armed by `RescheduleDrain :224`). It is started from inside the DI singleton factory at
`ServiceCollectionExtensions.cs:105`, so it begins ticking the moment the first consumer resolves
`IAlarmHistorianSink`.

Consequences for the Primary gate:

1. **The gate has to live inside `DrainOnceAsync`**, not in a wrapper. There is no external
   scheduler to gate.
2. **`Core.AlarmHistorian` cannot see `PrimaryGatePolicy`.** The policy is in
   `Runtime/Drivers/PrimaryGatePolicy.cs`, and Runtime → Core is the reference direction.
   `Core.AlarmHistorian` references only `Core.Abstractions` (`.csproj`). So the sink takes a
   **`Func<bool>` gate delegate** (default: always-drain) and Runtime supplies the lambda that
   calls `PrimaryGatePolicy`. No new project reference, no new interface in Core.
3. **A role view is still needed** because the timer fires outside any actor. See the next point
   for who should own it.

### There is already a Primary gate — on the *enqueue* side, with a different policy

`HistorianAdapterActor` (`Historian/HistorianAdapterActor.cs`) already subscribes to
`RedundancyStateChanged` (`:86`, handler `:145`), caches `_localRole` (`:42`), and gates the
enqueue on `ShouldHistorize()` (`:97`):

```csharp
private bool ShouldHistorize() => _localRole is not (RedundancyRole.Secondary or RedundancyRole.Detached);
```

This is **not** `PrimaryGatePolicy` — it ignores driver-member count and defaults to *write* while
the role is unknown. Its comment (`:68-71`) explains why it exists: DPS fans the Primary's single
`alerts` publish to **every** node's adapter, so without the gate both nodes of a pair would
double-write.

**This is why today's ungated drain is safe, and why Phase 2 breaks that.** Today the Secondary
never enqueues, so its queue is empty and an ungated drain has nothing to send. Once
`alarm_sf_events` replicates, the Primary's rows land in the Secondary's table — and an ungated
drain there would double-deliver *every* event, continuously, not just at failover. **The drain
gate is not a nicety; it is required by the same commit that registers the table.**

### Who owns the role view

`HistorianAdapterActor`, not `DriverHostActor`. Both are spawned under
`WithOtOpcUaRuntimeActors` (`ServiceCollectionExtensions.cs:353`) which `Program.cs:133` calls only
under `hasDriver` — as are `AddAlarmHistorian` (`:175`) and `AddOtOpcUaLocalDb` (`:185`). So sink,
LocalDb and adapter are co-located by construction. The adapter wins over `DriverHostActor`
because it **already** holds the role for exactly this purpose; using it adds no new subscription
and no new way for the view to go stale while the sink keeps draining.

`DriverMemberCount` is read off Akka cluster state in `DriverHostActor.DriverMemberCount() :1446`
(try/catch → 0 on a non-cluster provider ⇒ single-node posture). Task 3 will lift that into a
small shared helper rather than copy it.

## 5. Other `DatabasePath` construction sites

- `ServiceCollectionExtensions.cs:98` — the only production site.
- `appsettings.json:57` — `"DatabasePath": "alarm-historian.db"`.
- `AlarmHistorianOptions.Validate() :54-55` — warns when the path is relative. **This warning
  disappears with the key**; the validator loses one of its five checks.
- `SqliteStoreAndForwardSinkTests.cs` — ~30 construction sites, all passing the per-test temp
  `_dbPath` seeded at `:25` and deleted in `Dispose :32`. These are the tests Task 2 rewires onto
  a temp-file `ILocalDb`.
- **`docker-dev/docker-compose.yml` has no `AlarmHistorian` keys at all** — the rig runs with
  `Enabled=false` today. Task 6 is therefore an *addition*, not an edit.

No tooling or CLI constructs the sink.

## 6. How `Enabled=false` short-circuits

`AddAlarmHistorian` returns before registering anything when the section is absent or
`Enabled != true` (`ServiceCollectionExtensions.cs:87`), leaving the `NullAlarmHistorianSink`
seeded by `AddOtOpcUaRuntime :49`. The Null sink is a pure no-op (`IAlarmHistorianSink.cs:37-52`).

Confirming the plan's own answer: **the tables are created unconditionally in `OnReady`** (cheap,
empty) because `AddOtOpcUaLocalDb` is independent of the `AlarmHistorian` gate. Only the
sink/drain stay Null. This is also required for correctness — `RegisterReplicated` must run on
both pair members regardless of their historian config, or a node that later enables the sink
would write rows that are never captured (see the CDC note below).

## 7. Library constraints re-confirmed (README, `ZB.MOM.WW.LocalDb` 0.1.3)

- **BLOB columns are rejected at registration.** Not a problem here (§1).
- **CDC semantics** — rows that existed *before* `RegisterReplicated` are neither captured nor
  snapshotted. This is the mechanical reason the migrator must run **after** registration, and
  the reason `LocalDbSetup.OnReady`'s ordering comment is load-bearing (`LocalDbSetup.cs:23-31`).
- **LWW keyed on the PK** — two nodes minting the same PK for *different* rows silently overwrite
  each other; two nodes minting the same PK for the *same* row converge. Decision D-1 below turns
  that second property into a feature.
- `ILocalDb` surface in use: `ExecuteAsync` / `QueryAsync(sql, rowMapper, params, ct)` /
  `BeginTransactionAsync` / `CreateConnection` / `RegisterReplicated`
  (`Deployment/LocalDbDeploymentArtifactCache.cs:77,82,205`).

---

## Deviations and decisions

Recorded here rather than raised as questions, per the plan's follow-up convention.

### D-1 — Enqueue ids are a deterministic content hash, not `Guid.NewGuid()`

**The plan says** GUIDs minted at enqueue (Task 2 step 2).

**The problem.** The enqueue gate (`ShouldHistorize`, default-**write** on unknown role) and the
drain gate the plan specifies (`PrimaryGatePolicy`, default-**deny** on unknown role when paired)
disagree during the boot window. On a two-node pair before the first redundancy snapshot arrives,
**both** adapters enqueue the same DPS-fanned transition. With random GUIDs those are two distinct
rows that replicate into each other's tables — the buffer *duplicates* instead of converging, and
whichever node becomes Primary later delivers both copies.

**The decision.** Mint `id` as a hash over the serialized payload. Two nodes independently
enqueuing the same fanned event produce the same PK, so LWW converges them to one row for free.
Two genuinely distinct events cannot collide: `AlarmHistorianEvent` carries a full-precision
`TimestampUtc` plus alarm id, kind, message and user, so an identical hash means an identical
event.

This costs one `SHA256.HashData` per enqueue and removes a whole duplicate-row class. It also
slightly *improves* today's behaviour, where the same boot-window fan-out is already
double-delivered because the drain is ungated on both nodes.

`ShouldHistorize` itself is left unchanged — out of scope, and with D-1 in place the two gates
disagreeing is no longer harmful.

### D-2 — Gate denial must be observable

`PrimaryGatePolicy` defaults to **deny** when the role is unknown on a multi-driver cluster. That
is the correct safety posture, but it means a redundancy-snapshot identity mismatch — the
documented 03/S5 shape that `DriverHostActor.cs:1485-1491` already warns about once — would
silently stop alarm history on *both* nodes, degrading only as retry/eviction growth minutes later.

Task 3 will therefore: log the skip once per gate-state transition (not per tick — the drain runs
every 5 s), and surface it in `HistorianSinkStatus` so `/healthz` and the AdminUI can see
"buffering, not draining, because not Primary" rather than an unexplained rising queue depth.
Adding a `HistorianDrainState` member is additive; Task 3 will check every `switch` over it.

### D-3 — Tasks 2 and 3 will land as one commit

The plan anticipated this. Confirmed necessary: the gate delegate is a constructor parameter of
the rewritten sink, so the sink cannot compile in its final shape without it, and an intermediate
commit with a rewired-but-ungated sink would be a commit in which a replicated table is drained by
both nodes. Not a state worth having in history.

### D-5 — The table keeps the legacy delete-on-ack shape, not a `status` column

**The plan proposes** `status TEXT NOT NULL DEFAULT 'pending'` with values `pending | delivered |
dead`, and drops the legacy `LastError` column.

**Two problems.** A `delivered` status means acknowledged rows accumulate in the table forever —
the plan specifies no sweeper for them, and they would also have to be excluded from the capacity
count, changing a semantic §3 says to preserve. And `LastError` is not dead weight: `DeadLetterRow`
writes the failure reason into it and `RetryDeadLettered` clears it, so it is the only operator-
facing record of *why* a row was dead-lettered.

**The decision.** Keep the legacy shape: DELETE on acknowledgement, `dead_lettered` as the same 0/1
flag, and retain `last_error`. Deleting keeps the table bounded with no second sweeper, and the
replication engine carries the delete as a tombstone (pruned after `TombstoneRetention`, 7 d), so
the peer drops its copy too — which is the property the plan wanted the `delivered` status for.
Columns then map 1:1 onto the legacy table, making the Task-4 migrator a straight copy.

One genuine change: the drain's `ORDER BY` moves from `RowId ASC` to `enqueued_at_utc, id`, because
a hashed TEXT id carries no insertion order. Round-trip ("O") timestamps sort lexicographically in
chronological order, and `id` makes the ordering total; the index covers both.

### D-6 — The migrator keys on the payload hash, not `mig-{node}-{legacyId}`

**The plan says** deterministic ids of the form `mig-{NodeName}-{legacyId}`, mirroring ScadaBridge.

**Why that is not enough here.** Node-prefixing does solve the collision the legacy AUTOINCREMENT
key would otherwise cause — node A's `RowId 7` and node B's `RowId 7` are different alarms — but it
*preserves* a duplication that ought to be collapsed. A warm pair's two legacy files **overlap**:
`HistorianAdapterActor` default-writes while its redundancy role is unknown, so both nodes accepted
the same fanned transitions during every boot window they ever had. Prefixed ids would carry every
one of those duplicates into the merged buffer permanently, for the eventual Primary to deliver
twice.

**The decision.** Reuse D-1's `AlarmSfSchema.DeriveId` (lifted out of the sink so both call sites
share one identity rule). Distinct events cannot collide; identical ones converge. It also removes
the migrator's only need for node identity from configuration, and makes a crash between commit and
rename harmless under `INSERT OR IGNORE` — which the plan's own idempotency requirement wanted
anyway.

### D-7 — The migrator's tests live in `Host.IntegrationTests`

The plan names `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests`. **That project does not exist.** The
tests sit beside `LocalDbSetupTests` in `Host.IntegrationTests`, which is where every other
LocalDb-facing test already lives. They are offline — a temp file and no external services.

### D-4 — The live gate needs `MaxAttempts` raised on the rig

Not actionable until Task 8, recorded now so it is not rediscovered there. The rig has no
HistorianGateway, so every drain attempt fails. With `MaxAttempts: 10` and the backoff ladder
capping at 60 s, buffered rows dead-letter roughly five minutes into the outage — which would make
the gate's "dead letters 0" check (step 2) fail for reasons that have nothing to do with
replication. Task 6 should set a high `MaxAttempts` on the rig, or Task 8 should assert on
dead-letter *timing* instead.

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.