diff --git a/docs/plans/2026-07-19-localdb-adoption-phase2.md b/docs/plans/2026-07-19-localdb-adoption-phase2.md index a4315d6a..ea5fd918 100644 --- a/docs/plans/2026-07-19-localdb-adoption-phase2.md +++ b/docs/plans/2026-07-19-localdb-adoption-phase2.md @@ -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()` + 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 10–11: 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. diff --git a/docs/plans/2026-07-19-localdb-adoption-phase2.md.tasks.json b/docs/plans/2026-07-19-localdb-adoption-phase2.md.tasks.json index 41bd42b7..36401179 100644 --- a/docs/plans/2026-07-19-localdb-adoption-phase2.md.tasks.json +++ b/docs/plans/2026-07-19-localdb-adoption-phase2.md.tasks.json @@ -12,66 +12,83 @@ "date": "2026-07-19", "by": "user", "choice": "Full scope as designed", - "note": "User chose both surfaces (config tables + sf_messages) in one phase, deleting both bespoke mechanisms together, over the recommended split of S&F-first. The four open design questions are therefore resolved INSIDE this plan as D1-D5 rather than deferred." + "note": "User chose both surfaces (config tables + sf_messages) in one phase, deleting both bespoke mechanisms together, over the recommended split of S&F-first. The four open design questions are therefore resolved INSIDE this plan as D1-D6 rather than deferred." + }, + "reviewPass": { + "date": "2026-07-19", + "note": "Plan verified against actual code (3 verification sweeps + LocalDb library source) and corrected in place. Headline corrections: D1 (StoreDeployedConfigIfNewerAsync has a SECOND surviving caller, SiteReconciliationActor.cs:166 - the method and guard STAY; original Task 13 would also not have compiled, deleting a method whose caller dies only in Task 15), D3 (the active node ALREADY purges at DeploymentManagerActor.cs:1921 - Task 12 became a pin test, nothing is re-homed), D6 added (4 MB gRPC receive cap x row-count-only MaxBatchSize batching; config_json > 128 KB documented - measure in Task 1, size MaxBatchSize in Task 19, single row near 4 MB = stop/lib work), Task 1 rewritten (Phase 2 tables are NOT registered on the Phase 1 rig so driven churn never reaches __localdb_oplog - measure legacy-DB write rates + arithmetic; metrics port 8084 not 8080; real metric names are localdb_oplog_depth / localdb_sync_*, the plan's localdb_oplog_backlog/replication_dead_letters/sync_connected never existed; containers have no sqlite3 - use host-side sqlite3 on the bind mounts; branch already exists, no checkout -b), Task 14 (register 8 tables NOT 11 - notification_lists/smtp_configurations deliberately unregistered, reversing the original instruction; keep Migrate LAST in OnReady)." }, "decisions": [ { "id": "D1", - "subject": "Config moves to CDC; notify-and-fetch is DELETED, not preserved", - "evidence": "SiteReplicationActor sends id+fetch-coords only because the config blob exceeds Akka's 128KB frame (docs/known-issues/2026-06-26-deploy-config-exceeds-akka-frame-size.md). LocalDb sync is gRPC - no such limit. StoreDeployedConfigIfNewerAsync's guard (SiteStorageService.cs:325, `WHERE excluded.deployed_at > deployed_configurations.deployed_at`) protects only against a stale FETCH racing; with no fetch there is no race. Do NOT reproduce the guard on top of LWW - deployed_at and HLC are different clocks and mixing them is non-convergent." + "subject": "Config moves to CDC; notify-and-fetch is DELETED - but the guarded write STAYS", + "evidence": "SiteReplicationActor sends id+fetch-coords only because the config blob exceeds Akka's 128KB frame (docs/known-issues/2026-06-26-deploy-config-exceeds-akka-frame-size.md, already marked RESOLVED by the notify-and-fetch rework). LocalDb sync is gRPC - no such limit (but see D6). CORRECTED: StoreDeployedConfigIfNewerAsync (SiteStorageService.cs:301-336, guard at :325) has TWO production callers - SiteReplicationActor.cs:375 (dies in Task 15) AND SiteReconciliationActor.cs:166 (per-node startup self-heal vs central, SURVIVES Phase 2, stale-fetch race still real). Method + guard stay; reconcile becomes a benign second CDC writer. Do NOT reproduce the guard on top of LWW - deployed_at and HLC are different clocks and mixing them is non-convergent." }, { "id": "D2", "subject": "ReplaceAllAsync deleted; the N1 directional guard becomes unnecessary", - "evidence": "LocalDb's snapshot resync MERGES per-row LWW and never deletes: SnapshotApplier.OnBeginAsync (SnapshotStreamer.cs:163-170) resets counters only, no DELETE anywhere in the class; OnBatchAsync (:172-186) routes snapshot rows through the same LwwApplier as deltas; LwwApplier.cs:69-78 discards an incoming row whose HLC is lower. So a stale peer cannot wipe a live buffer - structurally impossible, not guarded. SEMANTIC CHANGE: the standby is convergent, no longer byte-identical." + "evidence": "LocalDb's snapshot resync MERGES per-row LWW and never WIPES: SnapshotApplier.OnBeginAsync (SnapshotStreamer.cs:163-170) resets counters only; OnBatchAsync (:172-186) routes snapshot rows through the same LwwApplier as deltas; LwwApplier.cs:69-78 discards an incoming row whose HLC is lower. Row-level deletes DO replicate (delete-trigger tombstones, streamed by SnapshotStreamer, applied as real DELETEs by LwwApplier) - only the destructive whole-table replace is gone. Caveat: tombstones pruned after TombstoneRetention (default 7d); a node offline longer can resurrect deleted rows (runbook, Task 21). SEMANTIC CHANGE: the standby is convergent, no longer byte-identical." }, { "id": "D3", - "subject": "The SMTP purge is re-homed BEFORE anything is deleted", - "evidence": "SiteReplicationActor.HandleApplyArtifacts calls PurgeCentralOnlyNotificationConfigAsync (SiteStorageService.cs:811-821), deleting notification_lists + smtp_configurations incl. plaintext SMTP passwords from pre-2026-07-10 deployments. A security cleanup riding the replication path; CDC will not reproduce it. Task 12 moves it to the active node's deploy path before any deletion, so it never lapses." + "subject": "CORRECTED: the SMTP purge already runs on the active node - pin it, don't move it", + "evidence": "PurgeCentralOnlyNotificationConfigAsync (SiteStorageService.cs:811-821) has TWO callers: DeploymentManagerActor.cs:1921 (ACTIVE node's HandleDeployArtifacts, :1864-1963) and SiteReplicationActor.cs:456 (standby copy, dies with the actor). The purge never lapses; the original 're-home before any deletion' premise was false. Task 12 = pin test only (ArtifactStorageTests covers the storage method, not the actor call site Task 16 edits). No site writer to notification_lists/smtp_configurations since 2026-07-10 (verified: only test seeding inserts exist) + migrator skips them => permanently empty in the consolidated DB => Task 14 does NOT register them." }, { "id": "D4", "subject": "native_alarm_state volume is MEASURED, not assumed", - "evidence": "scadabridge.db is not only config - native_alarm_state mirrors live A&C conditions (NativeAlarmActor.cs:504) and is the highest-volume table in either DB. sf_messages worst case ~50 row-writes/sec. Task 1 is a rig soak that measures both and sets MaxOplogRows/MaxOplogAge. If backlog grows monotonically, STOP: the design doc's keyed-instances escape hatch (line 140) is a scadaproj library effort that would suspend this plan." + "evidence": "scadabridge.db is not only config - native_alarm_state mirrors live A&C conditions (NativeAlarmActor.cs:504) and is the highest-volume table in either DB. sf_messages worst case ~50 row-writes/sec. Task 1 measures both AT THE LEGACY-DB SOURCE (they are not in the Phase 1 oplog - see reviewPass) and sets MaxOplogRows/MaxOplogAge arithmetically; Task 20 evidence 10 does the empirical post-cutover drain check. If growth is monotonic, STOP: keyed-instances escape hatch = ~/Desktop/scadaproj/docs/plans/2026-07-19-scadabridge-localdb-design.md:139-141 (adoption design doc, NOT the 07-17 lib doc), a scadaproj library effort that would suspend this plan." }, { "id": "D5", "subject": "Cutover forecloses rolling site upgrades", - "evidence": "SiteReplicationActor retains a legacy monolithic SfBufferSnapshot handler for rolling upgrades. With no dual-mechanism period, 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." + "evidence": "SiteReplicationActor retains a legacy monolithic SfBufferSnapshot handler for rolling upgrades. With no dual-mechanism period, 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 docs (installation-guide.md/topology-guide.md - no file literally named runbook)." + }, + { + "id": "D6", + "subject": "NEW (review pass): 4 MB gRPC message cap replaces the 128 KB Akka frame as the size ceiling", + "evidence": "Neither side configures gRPC message sizes (ScadaBridge AddGrpc at Program.cs:519-521 sets only the auth interceptor; the lib's initiator channel is bare GrpcChannel.ForAddress) => 4 MB default receive limit both directions. Batching is row-count-only (MaxBatchSize default 500; SyncSession.cs:227, SnapshotStreamer.cs:55; no byte-aware chunking). deployed_configurations.config_json documented >128 KB/row; a few dozen such rows in one batch exceeds 4 MB and wedges the stream on a poison batch. Task 1 measures max/avg config_json bytes; Task 19 sets LocalDb:Replication:MaxBatchSize so max-row-bytes x MaxBatchSize << 4 MB; any single row near 4 MB = STOP (needs byte-aware batching or size knobs in the LocalDb lib - scadaproj effort)." } ], "reconFindings": [ "The gate doc named 2 test files as the specification; the real spec is 5 - it missed StoreAndForwardReplicationTests.cs (incl. the only Requeue coverage), ReplicationWireSerializationPinTests.cs, ResyncWireSerializationPinTests.cs, and SfBufferResyncPredicateTests.cs (the N1 Critical regression test).", "sf_messages has NO version column today - ON CONFLICT(id) DO UPDATE has no comparison predicate (StoreAndForwardStorage.cs:331-345). 'Newest wins' is bare arrival order. LWW-by-HLC is an IMPROVEMENT here, not a regression.", - "No autoincrement-integer PKs exist anywhere in Phase 2 scope - all 9 config tables use natural TEXT or composite TEXT keys, sf_messages is TEXT. Phase 1's site_events GUID conversion has no Phase 2 analogue.", + "No autoincrement-integer PKs exist anywhere in Phase 2 scope - all 9 config tables use natural TEXT or composite TEXT keys, sf_messages is TEXT. LocalDb RegisterReplicated SUPPORTS composite PKs (ordered pk ordinals) and rejects BLOB columns - no Phase 2 table has one (verified). Phase 1's site_events GUID conversion has no Phase 2 analogue.", "SiteStorageService has NO foreign keys. RemoveDeployedConfigAsync (:343-376) is a manual 3-statement cascade in one transaction. Under CDC these become three independent delete streams that LWW may reorder - the most likely real defect in the plan (Task 18 scenario 2).", - "DeploymentManagerActor's replicationActor is an OPTIONAL POSITIONAL parameter (:184). Removing it silently shifts every argument after it. Compilers will not catch a shift between two same-typed optional params - check every call site by hand (Task 16).", - "ActiveNodeEvaluator must NOT be deleted - the S&F delivery gate still uses it. Only its doc comment mentions replication.", - "ConfigFetchRetryCount may not be fully dead: IDeploymentConfigFetcher serves BOTH the standby replication path and the active singleton's RefreshDeploymentCommand path. Verify before removing (Task 13 step 2).", - "notification_lists and smtp_configurations are deliberately NOT migrated (Task 9) - migrating them would resurrect plaintext SMTP passwords from a pre-fix legacy file into a replicated table." + "DeploymentManagerActor's replicationActor is an OPTIONAL POSITIONAL parameter at :169 (ctor :161-175; :184 is the field ASSIGNMENT). The same-typed IActorRef? optional dclManager sits immediately BEFORE it at :168 - that's the real silent-shift hazard. Props.Create passes it positionally at AkkaHostedService.cs:810. Check every call site by hand (Task 16).", + "ActiveNodeEvaluator must NOT be deleted - the S&F delivery gate still uses it (AkkaHostedService.cs:866 SetDeliveryGate -> SelfIsPrimary -> SelfIsOldestUp). Doc comment :14/:16 mentions replication; SiteReplicationActor.cs:288 calls it directly (dies with the actor).", + "ConfigFetchRetryCount's ONLY production reader is SiteReplicationActor.cs:157 (verified) - dead after Task 15; removed in Task 17 (removing it in Task 13, before the actor deletion, would not compile). IDeploymentConfigFetcher is KEPT: DeploymentManagerActor refresh path + SiteReconciliationActor + DI at ServiceCollectionExtensions.cs:84.", + "notification_lists and smtp_configurations are deliberately NOT migrated (Task 9) AND NOT registered (Task 14, corrected) - migrating or replicating them would resurrect/ship plaintext SMTP passwords; they are permanently empty by design (writers removed 2026-07-10; verified only test-seeding inserts exist).", + "REVIEW-PASS ADDITIONS: SiteReconciliationActor (runs on EVERY node at startup, best-effort self-heal vs central) is the second caller of both StoreDeployedConfigIfNewerAsync and IDeploymentConfigFetcher - it survives Phase 2 and constrains Tasks 11/13/20 (a startup fetch on the standby is legitimate; zero-fetch assertions must scope to the deploy window).", + "SiteStorageService has 21 (not 22) inline connection+OpenAsync pairs: 60,206,248,311,345,386,414,443,470,498,538,583,608,640,663,692,730,769,813,837,868. CreateConnection():51's only repository consumer is SiteExternalSystemRepository.", + "StoreAndForwardService: :39 is the _replication FIELD, :243 the ctor param; exactly 6 emission sites (:654,805,836,872,1120,1146). ServiceCollectionExtensions.cs:32 also resolves ReplicationService inside the StoreAndForwardService factory - must go in Task 14.", + "ReplicationMessages.cs holds ONLY the 10 Replicate*/Apply* records; the four SfBuffer resync records live at the bottom of SiteReplicationActor.cs (:678-707) and die with the actor file.", + "10 (not 9) appsettings.Site.json files set the legacy paths - deploy/wonder-app-vd03/appsettings.Site.json was missed (also sets ReplicationEnabled:false).", + "Rig facts: site metrics on port 8084 (AnyIP in-container, NOT published to host; 8080 is Traefik); real metric names localdb_oplog_depth / localdb_sync_* (meter ZB.MOM.WW.LocalDb.Replication); containers (aspnet:10.0) have NO sqlite3 - data dirs are host bind mounts, run sqlite3 host-side; ReplicationOptions bind at LocalDb:Replication:* (lib binds the section).", + "Phase 1's LocalDbSitePairConvergenceTests uses ONE shared API key (:47) - it never did a mismatched-key non-vacuity run; wrong-key denial is unit-covered in Host.Tests/LocalDbSyncAuthInterceptorTests.cs. Task 18's non-vacuity check must be done directly for the new scenarios.", + "MigrateEvents synthesizes deterministic 'mig-{node}-{legacyId}' ids (NOT fresh GUIDs - crash-rerun idempotency). Migration runs AFTER RegisterReplicated in OnReady (order is load-bearing); Task 8's oplog pin test must register sf_messages on its own TestLocalDb since production doesn't register it until Task 14." ], "tasks": [ - {"id": 1, "subject": "Task 1: Rig soak - measure oplog growth under real write rates", "status": "pending", "classification": "high-risk", "note": "GATES EVERYTHING AFTER TASK 2. Stop the plan if oplog backlog grows monotonically."}, + {"id": 1, "subject": "Task 1: Rig soak - measure legacy-DB write rates + config_json sizes (corrected method)", "status": "pending", "classification": "high-risk", "note": "GATES EVERYTHING AFTER TASK 2. Phase 2 tables are NOT in the Phase 1 oplog - measure at the legacy DBs host-side, size caps arithmetically; empirical drain check moves to Task 20 evidence 10. Stop conditions: on-paper oplog overflow OR any config_json near 4 MB (D6)."}, {"id": 2, "subject": "Task 2: Decision record - close the phase 2 gate", "status": "pending", "classification": "trivial", "blockedBy": [1]}, {"id": 3, "subject": "Task 3: Extract StoreAndForwardSchema.Apply", "status": "pending", "classification": "small", "blockedBy": [2]}, {"id": 4, "subject": "Task 4: Extract SiteStorageSchema.Apply", "status": "pending", "classification": "small", "blockedBy": [2]}, {"id": 5, "subject": "Task 5: Rewire StoreAndForwardStorage onto ILocalDb", "status": "pending", "classification": "high-risk", "blockedBy": [3]}, {"id": 6, "subject": "Task 6: Rewire SiteStorageService onto ILocalDb", "status": "pending", "classification": "high-risk", "blockedBy": [4]}, {"id": 7, "subject": "Task 7: Extend SiteLocalDbSetup with the new DDL (not yet registered)", "status": "pending", "classification": "standard", "blockedBy": [5, 6]}, - {"id": 8, "subject": "Task 8: Extend the legacy migrator for sf_messages", "status": "pending", "classification": "high-risk", "blockedBy": [7]}, - {"id": 9, "subject": "Task 9: Extend the legacy migrator for the 9 config tables", "status": "pending", "classification": "high-risk", "blockedBy": [7]}, + {"id": 8, "subject": "Task 8: Extend the legacy migrator for sf_messages", "status": "pending", "classification": "high-risk", "blockedBy": [7], "note": "Oplog pin test registers sf_messages on its own TestLocalDb - production OnReady doesn't register it until Task 14."}, + {"id": 9, "subject": "Task 9: Extend the legacy migrator for the config tables (skip notification/smtp)", "status": "pending", "classification": "high-risk", "blockedBy": [7]}, {"id": 10, "subject": "Task 10: Port the S&F replication test intents as CDC specs", "status": "pending", "classification": "standard", "blockedBy": [8]}, - {"id": 11, "subject": "Task 11: Port the resync + directional-authority tests", "status": "pending", "classification": "standard", "blockedBy": [9]}, - {"id": 12, "subject": "Task 12: Re-home the SMTP purge off the replication path", "status": "pending", "classification": "high-risk", "blockedBy": [10, 11], "note": "MUST land before any deletion so the security purge never lapses."}, - {"id": 13, "subject": "Task 13: Delete the notify-and-fetch config path", "status": "pending", "classification": "high-risk", "blockedBy": [12]}, - {"id": 14, "subject": "Task 14: Register the Phase 2 tables and delete ReplicationService", "status": "pending", "classification": "high-risk", "blockedBy": [13], "note": "THE CUTOVER. Registration + bespoke deletion in ONE commit so both mechanisms never run together."}, - {"id": 15, "subject": "Task 15: Delete SiteReplicationActor and its messages", "status": "pending", "classification": "high-risk", "blockedBy": [14]}, - {"id": 16, "subject": "Task 16: Clean up DeploymentManagerActor and AkkaHostedService", "status": "pending", "classification": "standard", "blockedBy": [15]}, - {"id": 17, "subject": "Task 17: Config-key cleanup", "status": "pending", "classification": "standard", "blockedBy": [16]}, - {"id": 18, "subject": "Task 18: Two-node convergence suite for the Phase 2 tables", "status": "pending", "classification": "high-risk", "blockedBy": [17]}, - {"id": 19, "subject": "Task 19: Rig configuration", "status": "pending", "classification": "small", "blockedBy": [17]}, - {"id": 20, "subject": "Task 20: Live gate on the docker rig", "status": "pending", "classification": "high-risk", "blockedBy": [18, 19]}, + {"id": 11, "subject": "Task 11: Port the resync + directional-authority tests", "status": "pending", "classification": "standard", "blockedBy": [9], "note": "Zero-fetch assertion scoped to deploy convergence - SiteReconciliationActor's startup fetch is legitimate and survives."}, + {"id": 12, "subject": "Task 12: Pin the active-node SMTP purge (corrected: already exists at DeploymentManagerActor.cs:1921)", "status": "pending", "classification": "standard", "blockedBy": [10, 11], "note": "Pin test only - no production change. Guards Task 16's actor edits from silently dropping the purge call."}, + {"id": 13, "subject": "Task 13: Notify-and-fetch scope check - guarded write STAYS for SiteReconciliationActor", "status": "pending", "classification": "standard", "blockedBy": [12], "note": "CORRECTED: do NOT delete StoreDeployedConfigIfNewerAsync (second caller SiteReconciliationActor.cs:166 survives; deleting here wouldn't compile anyway). Doc-comment update only; ConfigFetchRetryCount removal moved to Task 17."}, + {"id": 14, "subject": "Task 14: Register the 8 Phase 2 tables and delete ReplicationService", "status": "pending", "classification": "high-risk", "blockedBy": [13], "note": "THE CUTOVER. Register 7 config tables + sf_messages - NOT notification_lists/smtp_configurations (reversed from original; see D3). Keep Migrate LAST in OnReady. Registration + bespoke deletion in ONE commit."}, + {"id": 15, "subject": "Task 15: Delete SiteReplicationActor and its messages", "status": "pending", "classification": "high-risk", "blockedBy": [14], "note": "Resync records live at SiteReplicationActor.cs:678-707, not ReplicationMessages.cs."}, + {"id": 16, "subject": "Task 16: Clean up DeploymentManagerActor and AkkaHostedService", "status": "pending", "classification": "standard", "blockedBy": [15], "note": "Ctor param at :169; same-typed IActorRef? dclManager at :168 BEFORE it is the silent-shift hazard; Props.Create positional at AkkaHostedService.cs:810."}, + {"id": 17, "subject": "Task 17: Config-key cleanup (incl. ConfigFetchRetryCount; 10 appsettings files)", "status": "pending", "classification": "standard", "blockedBy": [16]}, + {"id": 18, "subject": "Task 18: Two-node convergence suite for the Phase 2 tables", "status": "pending", "classification": "high-risk", "blockedBy": [17], "note": "Non-vacuity must be proven directly (Phase 1 never did the mismatched-key run the original plan claimed)."}, + {"id": 19, "subject": "Task 19: Rig configuration (MaxOplogRows/MaxOplogAge + MaxBatchSize per D6)", "status": "pending", "classification": "small", "blockedBy": [17]}, + {"id": 20, "subject": "Task 20: Live gate on the docker rig (10 evidence items)", "status": "pending", "classification": "high-risk", "blockedBy": [18, 19], "note": "Includes the post-cutover oplog drain-under-churn check (evidence 10) that Task 1 cannot measure pre-cutover. Host-side sqlite3 on bind mounts; metrics on :8084 in-container."}, {"id": 21, "subject": "Task 21: Documentation truth pass", "status": "pending", "classification": "standard", "blockedBy": [20]} ], "knownFlakes": [ @@ -85,5 +102,5 @@ } ], "lastUpdated": "2026-07-19", - "phase2Status": "PLANNED - not started. Task 1 (rig soak) gates the rest." + "phase2Status": "PLANNED - not started. Reviewed + corrected 2026-07-19. Task 1 (rig soak) gates the rest." }