docs(localdb): phase 2 plan review pass — corrections from code verification

Three verification sweeps over SiteRuntime, StoreAndForward, and Host/rig/docs
plus the LocalDb library source. Load-bearing corrections: D1 (the guarded write
has a second surviving caller, SiteReconciliationActor), D3 (the active node
already purges — Task 12 becomes a pin), D6 (new: the 4 MB gRPC cap vs
row-count-only batching), Task 1 (rewritten method — the Phase 2 tables are not
in the Phase 1 oplog), and Task 14 (do not register the notification/SMTP tables).

Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
This commit is contained in:
Joseph Doherty
2026-07-19 17:27:11 -04:00
parent f6ca82a9e2
commit 25463d522f
2 changed files with 332 additions and 154 deletions
+288 -127
View File
@@ -16,6 +16,15 @@ hand-shipped ops; the library's per-row-LWW snapshot resync replaces the chunked
**Branch:** `feat/localdb-phase2`, cut from `feat/localdb-phase1`.
> **Reviewed and corrected 2026-07-19** against the actual code (three verification sweeps over
> SiteRuntime, StoreAndForward, and Host/rig/docs, plus the LocalDb library source). Load-bearing
> corrections: D1 (the guarded write has a second surviving caller — `SiteReconciliationActor` —
> so it is NOT deleted), D3 (the active node already purges — Task 12 is now a pin, not a
> re-home), D6 (new: the 4 MB gRPC message cap × row-count-only batching), Task 1 (the Phase 2
> tables aren't in the Phase 1 oplog — the soak measures legacy-DB write rates; port 8084; real
> metric names; no `sqlite3` in containers), and Task 14 (do NOT register the two
> notification/SMTP tables). Details inline at each site.
---
## Read this before Task 1
@@ -52,17 +61,22 @@ exceeds Akka's 128 KB frame limit (`docs/known-issues/2026-06-26-deploy-config-e
Under CDC the row itself replicates over the LocalDb gRPC stream, which has no such limit. The
standby therefore never fetches at all.
This also disposes of the version guard. `StoreDeployedConfigIfNewerAsync`
The version guard, however, does **not** die with it. `StoreDeployedConfigIfNewerAsync`
(`SiteStorageService.cs:301-336`) guards its upsert with:
```sql
WHERE excluded.deployed_at > deployed_configurations.deployed_at
```
That predicate protects against **a stale fetch landing after a newer one** — a race that only
exists because the standby was independently fetching. With the fetch gone, the only writer is the
active node's deploy path, and HLC ordering on a single writer is monotonic. The guard becomes
dead code.
That predicate protects against **a stale fetch landing after a newer one**, and the method has
**two** production callers, not one: `SiteReplicationActor.HandleApplyConfigDeploy`
(`SiteReplicationActor.cs:375` — deleted in Task 15) *and* `SiteReconciliationActor`
(`SiteReconciliationActor.cs:166`) — the per-node startup self-heal that reports local inventory
to central and HTTP-fetches gap items. The reconciliation actor **survives Phase 2**, and its
stale-fetch race (a reconcile fetch completing after a newer deploy landed) still exists. **The
method and its guard stay.** Under CDC the reconcile write simply becomes a second CDC writer of
`deployed_configurations` — benign: a gap fetch only fires when the node genuinely lacks the row,
the content is deterministic per (deployment id, revision), and the guard blocks local downgrades.
> **Do not** try to reproduce the `deployed_at` guard on top of LWW. They order by different
> clocks (central's deploy time vs. the writing node's HLC) and mixing them produces a
@@ -75,13 +89,19 @@ dead code.
node — it discards every in-flight row." `SfBufferResyncPredicateTests` exists to enforce that only
a standby ever applies it — the N1 Critical regression test.
The LocalDb library's snapshot resync **merges per row under LWW and never deletes**:
The LocalDb library's snapshot resync **merges per row under LWW and never wipes**:
- `SnapshotApplier.OnBeginAsync` (`SnapshotStreamer.cs:163-170`) resets counters only. There is no
`DELETE` or `TRUNCATE` anywhere in the class.
wholesale `DELETE FROM` or `TRUNCATE` anywhere in the class.
- `OnBatchAsync` (`:172-186`) wraps each snapshot row as an ordinary oplog entry and pushes it
through the same `LwwApplier` as a delta.
- `LwwApplier.cs:69-78` discards the incoming row when the local row's HLC is higher.
- **Row-level deletes DO still replicate** — the delete trigger captures a tombstone
(`TriggerSqlGenerator`), `SnapshotStreamer` streams tombstone row-versions, and `LwwApplier`
applies an incoming tombstone as a real `DELETE`. Only the destructive whole-table replace is
gone. Caveat: tombstones are pruned after `LocalDb:Replication:TombstoneRetention` (default
7 days) — a node offline longer than that can resurrect deleted rows on rejoin; Task 21 puts
this in the runbook.
So a node with newer local rows keeps them. The failure the directional guard prevents — a stale
peer wiping a live buffer — is structurally impossible, not merely guarded against. **Port
@@ -91,16 +111,22 @@ per id wins), not as a directional-authority assertion.**
Consequence to state plainly: the standby is no longer guaranteed byte-identical to the active
node's buffer. It is guaranteed *convergent*. That is a real semantic change and Task 2 records it.
### D3. The SMTP purge is re-homed before anything is deleted
### D3. The SMTP purge already runs on the active node — pin it, don't move it
`HandleApplyArtifacts` calls `PurgeCentralOnlyNotificationConfigAsync`
(`SiteStorageService.cs:811-821`), which deletes `notification_lists` and `smtp_configurations`
including plaintext SMTP passwords left by pre-fix deployments. This is a **security cleanup riding
the replication path**, not a table sync. CDC will not reproduce it.
`PurgeCentralOnlyNotificationConfigAsync` (`SiteStorageService.cs:811-821`) deletes
`notification_lists` and `smtp_configurations` including plaintext SMTP passwords left by
pre-fix deployments. The original recon placed its only caller on the standby's replication path;
**that was wrong.** It has two callers: `DeploymentManagerActor.HandleDeployArtifacts`
(`DeploymentManagerActor.cs:1921`) — the **active** node's artifact-apply — and
`SiteReplicationActor.HandleApplyArtifacts` (`SiteReplicationActor.cs:456`), the standby copy
that dies with the actor. The purge therefore never lapses: the active-node call already exists
and stays. Task 12 shrinks to **pinning that call with a regression test** (it sits inside a
method Task 16 edits, and nothing today fails if the call is dropped).
Under CDC, if the active node purges, the deletes replicate as ordinary tombstones. So the fix is
to ensure the *active* node's deploy path performs the purge. Task 12 does this **before** any
deletion, so the purge never stops happening even for one commit.
Because no site code has written those two tables since 2026-07-10 (writers removed), the
migrator skips them (Task 9), and the active purge keeps them empty, they are **permanently empty
in the consolidated DB** — which is why Task 14 does **not** register them for replication (see
the rationale there).
### D4. `native_alarm_state` volume is measured, not assumed
@@ -111,8 +137,10 @@ sweep, 4-way target parallelism). Neither number can be turned into an oplog cap
principles.
**Task 1 is a rig soak that measures both.** Its output sets `MaxOplogRows` / `MaxOplogAge` in
Task 19. If the soak shows the shared oplog cannot absorb alarm-storm churn, the escape hatch named
in the design doc (line 140) is keyed instances — that is a library-level effort and would suspend
Task 19. If the soak shows the shared oplog cannot absorb alarm-storm churn, the escape hatch is
keyed instances — named in the **adoption** design doc
(`~/Desktop/scadaproj/docs/plans/2026-07-19-scadabridge-localdb-design.md:139-141`, "Out of
scope"; not the 07-17 library design doc) — that is a library-level effort and would suspend
this plan, so **Task 1 gates everything after Task 2.**
### D5. Cutover forecloses rolling site upgrades
@@ -122,6 +150,23 @@ rolling upgrades. With no dual-mechanism period, a site pair cannot be upgraded
one node would speak a protocol the other no longer implements. **Both nodes of a site must be
stopped and started together.** Task 21 puts this in the deployment runbook.
### D6. The 4 MB gRPC message cap replaces the 128 KB Akka frame as the size ceiling
The Akka frame limit that motivated notify-and-fetch is gone, but LocalDb sync has its own
ceiling the plan must respect: **neither side configures gRPC message sizes** — ScadaBridge's
`AddGrpc` (`Program.cs:519-521`) sets only the auth interceptor, and the library's initiator
channel is a bare `GrpcChannel.ForAddress` — so the **4 MB default receive limit** applies in
both directions. Batching is **row-count-only** (`MaxBatchSize`, default 500 —
`SyncSession.cs:227`, `SnapshotStreamer.cs:55`; there is no byte-aware chunking).
`deployed_configurations.config_json` is documented to exceed 128 KB per row; a few dozen such
rows in one batch is over 4 MB, which fails the stream and wedges replication on a poison batch.
**Task 1 measures the real max/typical `config_json` size** on the rig's legacy DB; **Task 19
sets `LocalDb:Replication:MaxBatchSize`** so `max-row-bytes × MaxBatchSize` stays comfortably
under 4 MB. If any single row approaches 4 MB, **stop**: that needs a LocalDb library change
(byte-aware batching or configurable message sizes) — a `scadaproj` effort, same class as the
keyed-instances hatch.
---
## Execution waves
@@ -133,7 +178,7 @@ stopped and started together.** Task 21 puts this in the deployment runbook.
| 2 | 5, 6 | Store rewiring — independent files, dispatch in parallel |
| 3 | 7, 8, 9 | Setup + migrators |
| 4 | 10, 11 | Port test intents as specs — **before** any deletion |
| 5 | 12, 13 | Re-home the purge, delete notify-and-fetch |
| 5 | 12, 13 | Pin the active-node purge; notify-and-fetch scope check |
| 6 | 14, 15, 16, 17 | The cutover, serial |
| 7 | 18, 19 | Convergence suite + rig config |
| 8 | 20, 21 | Live gate + docs |
@@ -151,31 +196,46 @@ This is the gate item from `2026-07-19-localdb-phase2-gate.md` §5. It is measur
**Files:**
- Create: `docs/plans/2026-07-19-localdb-phase2-soak.md`
> **Method note (corrected):** on this rig the Phase 2 tables are still in the **legacy** DBs —
> `sf_messages` in `store-and-forward.db`, `native_alarm_state` in `scadabridge.db` — and are
> **not registered** with LocalDb, so driving S&F/alarm churn moves `__localdb_oplog` **not at
> all**. This soak therefore measures (a) **source write rates from the legacy DBs** and derives
> the oplog sizing arithmetically, and (b) that the Phase 1 oplog stays healthy alongside. The
> *empirical* post-cutover drain check lives in Task 20 (evidence item 10).
>
> Also: the containers have **no `sqlite3` binary** (bare `aspnet:10.0` image), but the data dirs
> are **host bind mounts** — run `sqlite3` on the host against `docker/site-a-node-*/data/`.
> Metrics are on port **8084** inside the container (not published to the host).
**Step 1: Bring up the rig with Phase 1 replication enabled**
```bash
cd ~/Desktop/ScadaBridge
git checkout feat/localdb-phase1
cd ~/Desktop/ScadaBridge # already on feat/localdb-phase2 (= phase1 + docs)
bash docker/deploy.sh
```
Site-a's pair replicates; site-b/site-c deliberately do not (the default-OFF pin). Confirm:
```bash
docker exec scadabridge-site-a-a curl -s localhost:8080/metrics | grep '^localdb_'
docker exec scadabridge-site-a-a curl -s localhost:8084/metrics | grep '^localdb_'
```
Expected: `localdb_*` series present. If absent, the meter allowlist regressed — see
Expected: `localdb_*` series present (`localdb_oplog_depth`, `localdb_sync_*` — the meter is
`ZB.MOM.WW.LocalDb.Replication`). If absent, the meter allowlist regressed — see
`SiteServiceRegistration.ObservedMeters`.
**Step 2: Capture a baseline**
**Step 2: Capture baselines (host-side, against the bind mounts)**
```bash
docker exec scadabridge-site-a-a sqlite3 /app/data/site-localdb.db \
"SELECT COUNT(*) FROM __localdb_oplog;"
cd ~/Desktop/ScadaBridge/docker
sqlite3 site-a-node-a/data/site-localdb.db "SELECT COUNT(*) FROM __localdb_oplog;"
sqlite3 site-a-node-a/data/store-and-forward.db \
"SELECT COUNT(*), COALESCE(SUM(retry_count),0) FROM sf_messages;"
sqlite3 site-a-node-a/data/scadabridge.db "SELECT COUNT(*) FROM native_alarm_state;"
```
Record the count and the wall-clock time.
Record all values and the wall-clock time. (`COUNT + SUM(retry_count)` is the S&F row-write
proxy: every failed attempt is one `UPDATE` that increments `retry_count`.)
**Step 3: Drive sustained load for 30 minutes**
@@ -187,42 +247,69 @@ Two generators, run concurrently:
- **Alarm churn.** Drive A&C conditions on a deployed instance so `native_alarm_state` upserts
fire. This is the number that actually matters — it is unbounded by design.
**Step 4: Sample oplog depth every 5 minutes**
**Step 4: Sample write rates every 5 minutes**
```bash
cd ~/Desktop/ScadaBridge/docker
for i in $(seq 1 6); do
echo "=== t+$((i*5))m ==="
docker exec scadabridge-site-a-a sqlite3 /app/data/site-localdb.db \
"SELECT COUNT(*) FROM __localdb_oplog;"
docker exec scadabridge-site-a-a curl -s localhost:8080/metrics \
| grep -E '^localdb_(oplog_backlog|replication_dead_letters|sync_connected)'
sqlite3 site-a-node-a/data/store-and-forward.db \
"SELECT COUNT(*), COALESCE(SUM(retry_count),0) FROM sf_messages;"
sqlite3 site-a-node-a/data/scadabridge.db \
"SELECT COUNT(*), COUNT(CASE WHEN last_transition_at > datetime('now','-5 minutes') THEN 1 END) FROM native_alarm_state;"
sqlite3 site-a-node-a/data/site-localdb.db "SELECT COUNT(*) FROM __localdb_oplog;"
docker exec scadabridge-site-a-a curl -s localhost:8084/metrics \
| grep -E '^localdb_(oplog_depth|sync)'
sleep 300
done
```
**Step 5: Record findings**
Per-interval deltas of `COUNT + SUM(retry_count)` give S&F row-writes/sec; the
`last_transition_at`-window count approximates alarm upserts/sec (it undercounts refresh-only
upserts — say so in the findings if alarm volume is near a threshold).
**Step 5: Measure `config_json` sizes (D6)**
```bash
sqlite3 site-a-node-a/data/scadabridge.db \
"SELECT COUNT(*), MAX(LENGTH(config_json)), AVG(LENGTH(config_json)) FROM deployed_configurations;"
```
If the rig's configs are trivially small, also run this against a production-representative
legacy DB (wonder) before trusting the number.
**Step 6: Record findings**
Write `docs/plans/2026-07-19-localdb-phase2-soak.md` with:
- rows/sec observed for `sf_messages` and for `native_alarm_state`, separately
- peak `localdb_oplog_backlog`
- whether backlog drained between bursts or grew monotonically
- max/avg `config_json` bytes and the derived **`MaxBatchSize`** (`max-row-bytes × MaxBatchSize`
comfortably under 4 MB — D6)
- Phase 1 oplog depth over the run (should stay near zero — it is not driven by this load)
- dead-letter count (must be 0)
- **the recommended `MaxOplogRows` and `MaxOplogAge`**, with the arithmetic
(sustained rows/sec × retention window, with alarm-storm headroom)
**Step 6: The decision gate**
**Step 7: The decision gate**
If backlog grew monotonically and never drained, the shared oplog cannot absorb this write profile.
**Stop. Do not proceed to Task 3.** Report to the user: the design doc's keyed-instances escape
hatch (design doc line 140) is required first, and that is a library effort in `scadaproj`.
If the measured sustained write rate × any reasonable `MaxOplogAge` exceeds any reasonable
`MaxOplogRows` — i.e. the shared oplog cannot absorb this write profile even on paper — **stop.
Do not proceed to Task 3.** Report to the user: the keyed-instances escape hatch
(`~/Desktop/scadaproj/docs/plans/2026-07-19-scadabridge-localdb-design.md:139-141`) is required
first, and that is a library effort in `scadaproj`. Likewise stop if any single `config_json`
approaches 4 MB (D6 — needs byte-aware batching in the library). The empirical
drain-under-churn confirmation happens post-cutover in Task 20; monotonic growth there is the
same stop condition.
**Step 7: Commit**
**Step 8: Commit**
```bash
git checkout -b feat/localdb-phase2
git add docs/plans/2026-07-19-localdb-phase2-soak.md
git commit -m "docs(localdb): phase 2 rig soak findings — oplog sizing evidence"
```
(The `feat/localdb-phase2` branch already exists — it carries this plan — so commit to it;
do **not** `git checkout -b`.)
---
### Task 2: Decision record
@@ -456,11 +543,14 @@ dotnet test tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/ZB.MOM.WW.ScadaBri
Same shape as Task 5, but note the differences:
- There are **22** inline `new SqliteConnection(_connectionString)` + `OpenAsync()` pairs, not a
helper. Add a `private SqliteConnection OpenConnection() => _localDb.CreateConnection();` and
convert all of them.
- `public SqliteConnection CreateConnection()` at `:51` is a public escape hatch used by site
repositories. Keep the member; change the body to `=> _localDb.CreateConnection();`.
- There are **21** inline `new SqliteConnection(_connectionString)` + `OpenAsync()` pairs, not a
helper (lines 60, 206, 248, 311, 345, 386, 414, 443, 470, 498, 538, 583, 608, 640, 663, 692,
730, 769, 813, 837, 868 — counted before Task 4, which removes the :60 one inside
`InitializeAsync`). Add a `private SqliteConnection OpenConnection() => _localDb.CreateConnection();`
and convert all of them.
- `public SqliteConnection CreateConnection()` at `:51` is a public escape hatch used by
`SiteExternalSystemRepository` (the only repository consumer). Keep the member; change the body
to `=> _localDb.CreateConnection();`.
- The busy-timeout floor (`BusyTimeoutFloorSeconds`, `:36`) and its `SqliteConnectionStringBuilder`
normalization are **deleted** — LocalDb configures pragmas on every connection it hands out.
- `AddSiteRuntime(string siteDbConnectionString)` loses its parameter. Keep the no-arg overload at
@@ -555,9 +645,17 @@ Three, minimum:
[Fact] public void MigratedSfMessages_EnterTheOplog_SoTheyActuallyReplicate() { }
```
The third is the one that catches the ordering defect. Phase 1's equivalent asserts on
`__localdb_oplog` directly — copy that assertion. **The oplog table is `__localdb_oplog`**
(`LocalDbSchema.cs:19`), not `zb_oplog`.
The third is the one that catches the ordering defect. Phase 1's equivalent
(`SiteLocalDbLegacyMigratorTests.cs:244`, `MigratedRows_EnterTheOplog_SoTheyActuallyReplicate`)
asserts on `__localdb_oplog` directly — copy that assertion. **The oplog table is
`__localdb_oplog`** (`LocalDbSchema.cs:19`), not `zb_oplog`.
> **Harness note:** production `OnReady` does not register `sf_messages` until the Task 14
> cutover, so these tests must build their own `ILocalDb` (`TestLocalDb`), call
> `RegisterReplicated("sf_messages")` themselves, and then run the migrator — the oplog
> assertion needs live capture triggers. Correspondingly, Task 14 must keep
> `SiteLocalDbLegacyMigrator.Migrate` as the **last** call in `OnReady`, after all
> registrations.
**Step 2: Add `ResolveStoreAndForwardPath` + `MigrateStoreAndForward`**
@@ -639,8 +737,8 @@ Port these **intents** (not the mechanics) as two-node convergence assertions, u
| Apply Add twice is idempotent, newest wins | LWW gives this |
| Park when Add was lost still materialises | upsert semantics |
**Explicitly do not port:** `ReplicationServiceTests` #10 (200 interleaved ops dispatched
synchronously in strict issue order, 400 observed). That asserts the *mechanism* — inline
**Explicitly do not port:** `ReplicationServiceTests.ReplicationOperations_AreDispatchedInIssueOrder`
(`:169-195`; 200 interleaved ops dispatched synchronously in strict issue order, 400 observed). That asserts the *mechanism* — inline
fire-and-forget dispatch — not the outcome. CDC capture is asynchronous and batched by construction.
The portable intent is row 5 above. Record this in the test file as a comment so a future reader
does not think it was dropped by accident.
@@ -684,78 +782,87 @@ fetch path is deleted, so these describe code that will not exist. Replace with
that a config deploy on A converges to B **without** B making any HTTP call — assert on a fetcher
test double that records zero invocations. That is the positive proof that notify-and-fetch is gone.
> **Scope the zero-fetch assertion to the deploy-replication flow.** `SiteReconciliationActor`
> survives Phase 2 and legitimately HTTP-fetches at node startup when central reports gaps (D1).
> The two-node harness has no central, so its pass no-ops there — but do not write the assertion
> as "the standby never fetches, ever"; assert the fetcher double records zero calls **during
> deploy convergence**.
**Commit:** `test(localdb): port resync + config replication intents as CDC convergence specs`
---
### Task 12: Re-home the SMTP purge off the replication path
### Task 12: Pin the active-node SMTP purge
**Classification:** high-risk
**Estimated implement time:** ~4 min
**Classification:** standard
**Estimated implement time:** ~3 min
**Parallelizable with:** none
Per D3. **This must land before any deletion** so the purge never lapses, even for one commit.
Per D3 (corrected): the active node's artifact-apply **already** calls
`PurgeCentralOnlyNotificationConfigAsync``DeploymentManagerActor.cs:1921`, inside
`HandleDeployArtifacts` (`:1864-1963`). Nothing is re-homed. What is missing is a **pin**:
Task 16 edits this actor's wiring, and no test today fails if the purge call is dropped
(`ArtifactStorageTests` covers the storage method, not the actor's call site).
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs` (artifact-apply path)
- Test: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs`
**Step 1: Write the failing test**
**Step 1: Write the pin test (red-first by commenting out the call locally, then restore)**
```csharp
[Fact]
public async Task ApplyingArtifacts_PurgesCentralOnlyNotificationConfig_OnTheActiveNode()
public async Task ApplyingArtifacts_PurgesCentralOnlyNotificationConfig()
{
// Security cleanup: notification_lists and smtp_configurations can hold plaintext
// SMTP passwords from pre-2026-07-10 deployments. This ran on the standby via
// SiteReplicationActor.HandleApplyArtifacts; with that actor deleted the ACTIVE
// node must do it, and CDC replicates the deletes as ordinary tombstones.
// SMTP passwords from pre-2026-07-10 deployments. The ACTIVE node's artifact-apply
// purges them (DeploymentManagerActor.HandleDeployArtifacts); the standby's copy of
// this call dies with SiteReplicationActor in Task 15. This test pins the active-node
// call so Task 16's actor edits cannot silently drop it.
// Arrange: real temp-file SiteStorageService, seed one row into each table,
// drive HandleDeployArtifacts; assert both tables are empty afterwards.
}
```
**Step 2: Call `PurgeCentralOnlyNotificationConfigAsync` from the active node's artifact-apply path**
**Step 2: No production change is expected.** If recon of the call site shows otherwise, stop
and report.
The existing call site is `SiteReplicationActor.HandleApplyArtifacts`. Find where
`DeploymentManagerActor` applies artifacts locally (the same path that reaches
`_replicationActor?.Tell(new ReplicateArtifacts(command))` at `:1952`) and call it there.
**Step 3: Verify the deletes replicate** — extend the Task 11 convergence suite with a case where
A purges a seeded row and B's copy disappears.
**Commit:** `fix(site): purge central-only notification config on the active node`
**Commit:** `test(site): pin the active-node notification-config purge`
---
### Task 13: Delete the notify-and-fetch config path
### Task 13: Notify-and-fetch scope check — the guarded write stays
**Classification:** high-risk
**Estimated implement time:** ~4 min
**Classification:** standard
**Estimated implement time:** ~3 min
**Parallelizable with:** none
Per D1.
Per D1 (corrected). The original task deleted `StoreDeployedConfigIfNewerAsync` here — that was
wrong twice over: (a) it has a **second surviving caller**, and (b) deleting it before Task 15
would not even compile, since `SiteReplicationActor` still calls it until then.
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs:301-336`
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs:66-68`
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs:56-58`
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs:301-336` (doc comment only)
**Step 1: Delete `StoreDeployedConfigIfNewerAsync`** — its only caller is
`SiteReplicationActor.HandleApplyConfigDeploy`, deleted in Task 15. The unguarded
`StoreDeployedConfigAsync` (`:241-271`) is the active node's path and stays.
**Step 1: Keep `StoreDeployedConfigIfNewerAsync`.** Its callers are
`SiteReplicationActor.HandleApplyConfigDeploy` (`SiteReplicationActor.cs:375` — dies with the
actor in Task 15) **and `SiteReconciliationActor` (`SiteReconciliationActor.cs:166`)** — the
per-node startup self-heal against central, which survives Phase 2 and still needs the
`deployed_at` guard against a stale reconcile fetch. Update the method's doc comment to name
reconciliation as the remaining caller (and drop any mention of standby replication). The
unguarded `StoreDeployedConfigAsync` (`:241-271`) stays the active deploy path.
**Step 2: Check `ConfigFetchRetryCount` before removing it**
**Step 2: `ConfigFetchRetryCount` — answered, but its removal moves to Task 17.**
`IDeploymentConfigFetcher` is used by **both** the standby replication path *and* the active
singleton's `RefreshDeploymentCommand` path. Only the first is being deleted.
The grep was run during plan review: the only production reader is `SiteReplicationActor.cs:157`
(plus its validator rule and tests). `SiteReconciliationActor` does a single best-effort pass and
does not read it; `IDeploymentConfigFetcher` itself is kept (used by `DeploymentManagerActor`'s
refresh path, `SiteReconciliationActor`, and registered at `ServiceCollectionExtensions.cs:84`).
So the option (`SiteRuntimeOptions.cs:68`) and its validator rule
(`SiteRuntimeOptionsValidator.cs:56-58`) become dead **after Task 15 deletes the actor** — remove
them in Task 17 (config-key cleanup), not here, or the build breaks. Re-run
`grep -rn "ConfigFetchRetryCount" src/ tests/` there to confirm.
```bash
grep -rn "ConfigFetchRetryCount" src/ tests/
```
If the active path reads it, **keep the option and its validator rule** and note that in the commit
message. If nothing outside the deleted actor reads it, remove both.
**Commit:** `refactor(site): delete notify-and-fetch config replication — CDC ships the row`
**Commit:** `docs(site): StoreDeployedConfigIfNewerAsync serves reconciliation — guard stays under CDC`
---
@@ -771,19 +878,36 @@ never both run.
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs`
- Delete: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/ReplicationService.cs`
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardService.cs:39,243,654,805,836,872,1120,1146`
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardService.cs` — field `:39`,
ctor param `:243`, and the 6 emission call sites `:654, 805, 836, 872, 1120, 1146`
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/ServiceCollectionExtensions.cs:63-68`
(the `ReplicationService` registration) **and `:32`** (`GetRequiredService<ReplicationService>()`
inside the `StoreAndForwardService` factory at `:27-61`)
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardStorage.cs:284-306`
- Delete: `tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/ReplicationServiceTests.cs`
- Delete: `tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/StoreAndForwardReplicationTests.cs`
- Delete: `tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/ReplicationWireSerializationPinTests.cs`
**Step 1: Register all 11 tables** in `SiteLocalDbSetup.OnReady`, replacing the Task 7 comment.
Skip `notification_lists` and `smtp_configurations`? **No — register them.** They must replicate
their *deletes* (Task 12). Register all 9 config tables plus `sf_messages`.
**Step 1: Register 8 tables** in `SiteLocalDbSetup.OnReady`, replacing the Task 7 comment: the
7 config tables (`deployed_configurations`, `static_attribute_overrides`, `shared_scripts`,
`external_systems`, `database_connections`, `data_connection_definitions`, `native_alarm_state`)
plus `sf_messages`. **Do NOT register `notification_lists` or `smtp_configurations`** (this
reverses the plan's original instruction): they are permanently empty by design — no site writer
since 2026-07-10, the migrator skips them (Task 9), and the active-node purge
(`DeploymentManagerActor.cs:1921`) keeps them empty — so registering them would create a standing
replication channel whose only historical payload was plaintext SMTP passwords, for zero benefit.
Put that rationale in an `OnReady` comment so a future writer has to make an explicit decision.
**Step 2: Delete `ReplicationService`** and remove the ctor parameter + all 6 emission call sites
from `StoreAndForwardService`.
Keep `SiteLocalDbLegacyMigrator.Migrate` as the **last** call in `OnReady`, after all
registrations, so migrated rows enter the oplog through live capture triggers (the Phase 1
ordering that is silently load-bearing).
Note the two composite-PK tables are fine: LocalDb's `RegisterReplicated` supports multi-column
PKs (`SqliteLocalDb.RegisterReplicated` orders `pk` ordinals), and no Phase 2 table has a BLOB
column (which would be rejected).
**Step 2: Delete `ReplicationService`** and remove the ctor parameter (`:243`), the
`_replication` field (`:39`), and all 6 emission call sites from `StoreAndForwardService`.
**Step 3: Delete `ReplaceAllAsync`** (`:284-306`). Per D2 the library's resync merges; a destructive
delete-all must not survive into a replicated table, where CDC would capture the mass-delete and
@@ -814,14 +938,20 @@ singleton. Delete that assertion.
- Delete: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/ResyncWireSerializationPinTests.cs`
- Delete: `tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/Cluster/SfBufferResyncPredicateTests.cs`
The whole `ReplicationMessages.cs` file becomes dead: `ReplicateConfigDeploy/Remove/SetEnabled`,
`ReplicateArtifacts`, `ReplicateStoreAndForward`, `ApplyConfigDeploy/Remove/SetEnabled`,
`ApplyArtifacts`, `ApplyStoreAndForward`, plus the resync records `RequestSfBufferResync`,
`SfBufferSnapshot`, `SfBufferSnapshotChunk`, `SfBufferResyncAck`.
The whole `ReplicationMessages.cs` file becomes dead — it contains exactly the 10 records
`ReplicateConfigDeploy/Remove/SetEnabled`, `ReplicateArtifacts`, `ReplicateStoreAndForward`,
`ApplyConfigDeploy/Remove/SetEnabled`, `ApplyArtifacts`, `ApplyStoreAndForward`, whose only
consumers are `SiteReplicationActor`, the 5 `DeploymentManagerActor` Tell sites (Task 16), the
`AkkaHostedService` wiring (Task 16), and the deleted tests. The four resync records
(`RequestSfBufferResync`, `SfBufferSnapshot`, `SfBufferSnapshotChunk`, `SfBufferResyncAck`) are
**not** in this file — they are declared at the bottom of `SiteReplicationActor.cs` (`:678-707`)
and die with the actor file.
**Do not delete** `ActiveNodeEvaluator` (`Communication/ClusterState/ActiveNodeEvaluator.cs`) — the
S&F **delivery gate** still needs it. Only its doc comment at `:14` mentions replication; update
the comment, keep the type.
S&F **delivery gate** still needs it (`AkkaHostedService.cs:866``SetDeliveryGate`
`SelfIsPrimary``ActiveNodeEvaluator.SelfIsOldestUp`). Its doc comment at `:14` mentions
`SiteReplicationActor` and `:16` mentions the resync authority checks; the actor's own direct call
at `SiteReplicationActor.cs:288` goes away with the file. Update the comment, keep the type.
Confirm the deletions are complete before committing:
@@ -845,16 +975,19 @@ Expected: no matches.
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs:55,161-190,794,970,1004,1102,1952`
- Modify: `src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs:770-796,806-812,1056`
**Step 1:** Remove the `IActorRef? replicationActor = null` ctor parameter (`:184`), the
`_replicationActor` field (`:55`), and all 5 `_replicationActor?.Tell(...)` sites.
**Step 1:** Remove the `IActorRef? replicationActor = null` ctor parameter (**`:169`** — the ctor
signature spans `:161-175`; `:184` is the field *assignment*), the `_replicationActor` field
(`:55`), and all 5 `_replicationActor?.Tell(...)` sites (`:794, 970, 1004, 1102, 1952`).
The parameter is optional and positional — removing it **silently shifts every argument after it**
(`healthCollector`, `serviceProvider`, `loggerFactory`, `configFetcher`, ...). Update the
`Props.Create` at `AkkaHostedService.cs:806-812` and every test constructing this actor. Compile
errors will not catch a positional shift between two same-typed optional parameters — check each
call site by hand.
(`healthCollector`, `serviceProvider`, `loggerFactory`, `configFetcher`,
`startupLoadRetryInterval`, `configLoader`). The sharpest hazard is the **same-typed `IActorRef?`
optional immediately before it** — `dclManager` at `:168`: a caller passing positionally could
silently feed the wrong actor ref with zero compile errors. Update the `Props.Create` at
`AkkaHostedService.cs:806-812` (which passes `replicationActor` positionally at `:810`) and every
test constructing this actor — check each call site by hand.
**Step 2:** Delete `replicationService` / `replicationLogger` resolution (`:770-772`), the actor
**Step 2:** Delete `replicationService` / `replicationLogger` resolution (`:770-773`), the actor
creation (`:785-789`), the handler wiring (`:792-796`), and the actor-inventory comment at `:1056`.
**Step 3:** `activeNodeCheck` (`:781-783`) is still used by `SiteCommunicationActor` (`:816-821`).
@@ -875,18 +1008,23 @@ Keep it.
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs:113-115`
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardOptions.cs:12`
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardOptionsValidator.cs:20`
- Modify: 9 × `appsettings.Site.json` (Host template + 6 `docker/site-*` + 2 `docker-env2/site-x-*`)
- Modify: `src/ZB.MOM.WW.ScadaBridge.StoreAndForward/StoreAndForwardOptionsValidator.cs:19-21`
- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs:68` + `SiteRuntimeOptionsValidator.cs:56-58` (moved here from Task 13)
- Modify: **10** × `appsettings.Site.json` (Host template + 6 `docker/site-*` + 2 `docker-env2/site-x-*`
+ **`deploy/wonder-app-vd03/appsettings.Site.json`** — the wonder single-node install, which the
original count missed; it sets both paths *and* `ReplicationEnabled: false`)
| Key | Action |
|---|---|
| `ScadaBridge:Database:SiteDbPath` | → migration-only. **Relax the `Require` at `StartupValidator.cs:113`** or every site config keeps it mandatory forever. |
| `ScadaBridge:StoreAndForward:SqliteDbPath` | → migration-only. Relax the non-empty rule at `StoreAndForwardOptionsValidator.cs:20`. |
| `ScadaBridge:StoreAndForward:ReplicationEnabled` | → **fully dead.** Delete the property and all 9 config entries. |
| `ScadaBridge:StoreAndForward:SqliteDbPath` | → migration-only. Relax the non-empty rule at `StoreAndForwardOptionsValidator.cs:19-21`. |
| `ScadaBridge:StoreAndForward:ReplicationEnabled` | → **fully dead.** Delete the property and every config entry (including wonder's). |
| `SiteRuntime … ConfigFetchRetryCount` | → **dead after Task 15** (only reader was `SiteReplicationActor.cs:157`; see Task 13). Delete the option, its validator rule, and any config entries — confirm with `grep -rn "ConfigFetchRetryCount" src/ tests/` first. |
`ParkedOperationRelayTests.cs:39` and `ParkedMessageHandlerActorTests.cs:34` set
`ReplicationEnabled = false` — they only need the flag to exist, so remove those lines.
`StoreAndForwardOptionsTests.cs:14,26,33` assert its default; delete those assertions.
`StoreAndForwardOptionsTests.cs` touches it three times: `:14` asserts the default, `:26` sets and
`:33` asserts a customized value — remove all three references.
**Leave the path keys present in configs** with a comment — they are read by the legacy migrator and
removing them would strand un-migrated data on a node that has not yet started once.
@@ -917,9 +1055,12 @@ Scenarios, beyond Tasks 1011:
3. A burst of `native_alarm_state` upserts converges and the oplog drains.
4. Node B restarts and catches up via snapshot resync without losing rows written on B while down.
**Verify the suite is non-vacuous.** Phase 1 did this by mismatching the two API keys and confirming
all scenarios went red. Do the same and record the result in the commit message. A convergence suite
that cannot fail proves nothing.
**Verify the suite is non-vacuous.** (Corrected claim: Phase 1's convergence suite did **not** do
a mismatched-key run — it uses one shared key, `LocalDbSitePairConvergenceTests.cs:47`; wrong-key
*denial* is unit-covered separately in `Host.Tests/LocalDbSyncAuthInterceptorTests.cs`.) Prove it
directly for the new scenarios: run once with the new table registrations disabled on node B (or a
deliberately wrong key) and confirm every new scenario goes red, then restore and record the
result in the commit message. A convergence suite that cannot fail proves nothing.
**Commit:** `test(localdb): two-node convergence for config tables + sf_messages`
@@ -934,8 +1075,11 @@ that cannot fail proves nothing.
**Files:**
- Modify: `docker/site-a-node-a/appsettings.Site.json`, `docker/site-a-node-b/appsettings.Site.json`
Set `MaxOplogRows` and `MaxOplogAge` from **Task 1's measured numbers** — not from a guess. Keep
site-b/site-c unreplicated so default-OFF stays proven side by side on one rig.
Set `LocalDb:Replication:MaxOplogRows` and `LocalDb:Replication:MaxOplogAge` (exact key paths —
the lib binds the `LocalDb:Replication` section; defaults 1,000,000 rows / 7 days) from **Task 1's
measured numbers** — not from a guess. Per D6, also set `LocalDb:Replication:MaxBatchSize`
(default 500) so `max-config_json-bytes × MaxBatchSize` stays comfortably under the 4 MB gRPC
receive limit. Keep site-b/site-c unreplicated so default-OFF stays proven side by side on one rig.
**Commit:** `chore(docker): size the site-a oplog caps from the phase 2 soak`
@@ -954,15 +1098,24 @@ site-b/site-c unreplicated so default-OFF stays proven side by side on one rig.
cd ~/Desktop/ScadaBridge && bash docker/deploy.sh
```
Tooling: the containers have no `sqlite3` — run DB checks host-side against the bind mounts
(`docker/site-a-node-{a,b}/data/`); metrics via
`docker exec scadabridge-site-a-a curl -s localhost:8084/metrics` (port **8084**, not published
to the host).
Evidence required — each must be captured, not asserted:
1. **Migration ran.** `store-and-forward.db.migrated` and `scadabridge.db.migrated` exist on both
site-a nodes; row counts in the consolidated DB match the legacy files.
site-a nodes; row counts in the consolidated DB match the legacy files. (Both nodes migrate
their own legacy files independently; identical-content rows then LWW-converge — expected,
not an anomaly.)
2. **No SMTP/notification rows migrated**`SELECT COUNT(*) FROM smtp_configurations` is 0.
3. **Config converges.** Deploy an instance to site-a; both nodes' `deployed_configurations` rows
are byte-identical with identical `__localdb_row_version` and originating node id.
4. **The standby made no HTTP fetch** — nothing in its log resembling a config fetch. This is the
positive proof that notify-and-fetch is gone.
4. **The standby made no config HTTP fetch during the deploy** (both nodes up) — grep the deploy
window of its log. This is the positive proof that notify-and-fetch is gone. A
`SiteReconciliationActor` fetch at node *startup* is legitimate (it survives Phase 2) and does
not fail this check.
5. **S&F converges.** Enqueue against a dead target; the parked row appears on the peer.
6. **Site failover.** Flip the site-a pair and confirm buffered messages survive and deliver
exactly once after the flip. `docker/failover-drill.sh` targets **CENTRAL** nodes and does not
@@ -971,6 +1124,10 @@ Evidence required — each must be captured, not asserted:
`native_alarm_state` rows on the peer.
8. **Zero dead letters**, oplog drained, `localdb_*` scraped from `/metrics`.
9. **Both nodes stopped and started together** (D5) — confirm a clean rejoin.
10. **Post-cutover drain under churn** — the empirical half Task 1 could not measure pre-cutover:
re-run Task 1's two load generators against this build and watch `localdb_oplog_depth`
(and `SELECT COUNT(*) FROM __localdb_oplog`) rise **and drain between bursts**. Monotonic
growth that never drains is the same stop condition as Task 1 step 7 (keyed-instances hatch).
**If any check fails, stop and report.** Do not proceed to Task 21.
@@ -989,18 +1146,22 @@ Evidence required — each must be captured, not asserted:
- Modify: `docs/requirements/Component-StoreAndForward.md:83`
- Modify: `docs/components/StoreAndForward.md`, `SiteRuntime.md`, `Host.md`
- Modify: `docs/known-issues/2026-06-26-deploy-config-exceeds-akka-frame-size.md`
- Modify: `docs/deployment/` runbook
- Modify: `docs/deployment/installation-guide.md` + `topology-guide.md` (+ `production-checklist.md`
if it covers upgrades) — there is no file literally named "runbook"
Specific corrections:
- `Component-StoreAndForward.md:83` is a long **normative** paragraph specifying the whole chunked
resync protocol and the N5 race. Rewrite for CDC — state the new duplicate-delivery bound
explicitly rather than deleting the discussion.
- The frame-size known-issue is **resolved by** this change (D1). Mark it resolved and say why,
rather than leaving a live known-issue describing deleted code.
- **Add to the deployment runbook (D5):** a site pair must be stopped and started together. Rolling
- The frame-size known-issue is **already marked "Status: RESOLVED (2026-06-26)"** (resolved by the
notify-and-fetch rework). Amend it to record that Phase 2 deleted notify-and-fetch itself — CDC
ships the row over gRPC, so the frame constraint is gone entirely — and note the successor
ceiling: the 4 MB gRPC message cap managed via `MaxBatchSize` (D6).
- **Add to the deployment docs (D5):** a site pair must be stopped and started together. Rolling
one node at a time is no longer supported, because the legacy `SfBufferSnapshot` compatibility
handler is gone.
handler is gone. Also record the D2 caveat: a node offline longer than
`LocalDb:Replication:TombstoneRetention` (default 7 days) can resurrect deleted rows on rejoin.
- Update the scadaproj LocalDb component row: Phase 2 complete, what replicates now, what
`SiteReplicationActor` used to do.