docs(localdb): phase-2 recon findings
Answers Task 0 with file:line citations against master 2e46d054.
The STOP condition does not fire: PayloadJson is TEXT, no BLOB column, so
alarm_sf_events can be registered for replication as-is.
Two findings reshape the plan's assumptions:
- The drain worker is not a hosted service or an actor. It is a
self-rescheduling Timer owned by the sink and started from the DI
factory, so the Primary gate has to live inside DrainOnceAsync as an
injected delegate -- Core.AlarmHistorian cannot reference
PrimaryGatePolicy, which lives in Runtime.
- A Primary gate already exists on the enqueue side
(HistorianAdapterActor.ShouldHistorize), using a different policy than
the one the plan specifies for the drain. That gate is why today's
ungated drain is safe: the Secondary never enqueues, so its queue is
empty. Replicating the table breaks that invariant, which makes the
drain gate mandatory in the same commit rather than a refinement.
Records deviations D-1 (deterministic content-hash ids so a boot-window
double-enqueue converges instead of duplicating), D-2 (gate denial must be
observable or a snapshot identity mismatch silently stops alarm history),
D-3 (Tasks 2+3 land as one commit) and D-4 (the rig will dead-letter in
~5 minutes at MaxAttempts 10 with no gateway).
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
@@ -1,15 +1,78 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-07-20-localdb-adoption-phase2.md",
|
||||
"tasks": [
|
||||
{ "id": 0, "subject": "Task 0: Recon — sink schema, seam, drain lifecycle, role-view bridge (STOP conditions)", "status": "pending" },
|
||||
{ "id": 1, "subject": "Task 1: alarm_sf_events schema + registration (+ exact-set pin update)", "status": "pending", "blockedBy": [0] },
|
||||
{ "id": 2, "subject": "Task 2: Rewire sink onto ILocalDb + delete bespoke file management (cutover 1/2)", "status": "pending", "blockedBy": [1] },
|
||||
{ "id": 3, "subject": "Task 3: Primary-gated drain via PrimaryGatePolicy (cutover 2/2 — may co-commit with Task 2)", "status": "pending", "blockedBy": [2] },
|
||||
{ "id": 4, "subject": "Task 4: One-time alarm-historian.db legacy migrator", "status": "pending", "blockedBy": [3] },
|
||||
{ "id": 5, "subject": "Task 5: Convergence + failover scenarios in the pair harness (+ positive control)", "status": "pending", "blockedBy": [3] },
|
||||
{ "id": 6, "subject": "Task 6: Rig config + docs", "status": "pending", "blockedBy": [3] },
|
||||
{ "id": 7, "subject": "Task 7: DoD sweep (offline) — STOP and report after this", "status": "pending", "blockedBy": [4, 5, 6] },
|
||||
{ "id": 8, "subject": "Task 8: Live gate on the docker-dev rig (needs explicit user go-ahead)", "status": "pending", "blockedBy": [7] }
|
||||
{
|
||||
"id": 0,
|
||||
"subject": "Task 0: Recon \u2014 sink schema, seam, drain lifecycle, role-view bridge (STOP conditions)",
|
||||
"status": "completed",
|
||||
"note": "STOP condition does NOT fire (no BLOB; PayloadJson TEXT). Recon doc: docs/plans/2026-07-20-localdb-phase2-recon.md. Key finding: the drain worker is an internal Timer inside the sink (not a hosted service/actor), so the gate is a Func<bool> ctor param; and HistorianAdapterActor already primary-gates ENQUEUE with a different policy (ShouldHistorize) - which is why today's ungated drain is safe and why replication breaks it. Deviations D-1..D-4 recorded."
|
||||
},
|
||||
{
|
||||
"id": 1,
|
||||
"subject": "Task 1: alarm_sf_events schema + registration (+ exact-set pin update)",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
0
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"subject": "Task 2: Rewire sink onto ILocalDb + delete bespoke file management (cutover 1/2)",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
1
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 3,
|
||||
"subject": "Task 3: Primary-gated drain via PrimaryGatePolicy (cutover 2/2 \u2014 may co-commit with Task 2)",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
2
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 4,
|
||||
"subject": "Task 4: One-time alarm-historian.db legacy migrator",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
3
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 5,
|
||||
"subject": "Task 5: Convergence + failover scenarios in the pair harness (+ positive control)",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
3
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 6,
|
||||
"subject": "Task 6: Rig config + docs",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
3
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 7,
|
||||
"subject": "Task 7: DoD sweep (offline) \u2014 STOP and report after this",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
4,
|
||||
5,
|
||||
6
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 8,
|
||||
"subject": "Task 8: Live gate on the docker-dev rig (needs explicit user go-ahead)",
|
||||
"status": "pending",
|
||||
"blockedBy": [
|
||||
7
|
||||
]
|
||||
}
|
||||
],
|
||||
"lastUpdated": "2026-07-20T00:00:00Z"
|
||||
"lastUpdated": "2026-07-21T00:00:00Z"
|
||||
}
|
||||
|
||||
@@ -0,0 +1,257 @@
|
||||
# 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-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`.
|
||||
Reference in New Issue
Block a user