From 25c15dd164e1ed94a4f9af570c2e87ee16f556d1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:37:48 -0400 Subject: [PATCH] =?UTF-8?q?ops(docker)+test(host)+docs:=20close=20register?= =?UTF-8?q?=20rows=20104=20+=20105=20=E2=80=94=20offline=20VACUUM=20recipe?= =?UTF-8?q?=20applied=20to=20the=20rig;=20build-time=20pin=20that=20no=20r?= =?UTF-8?q?eplicated=20table=20is=20a=20cascading-FK=20parent;=20record=20?= =?UTF-8?q?row=20106=20(LocalDb=20as=5Fof=5Fseq=3D0=20full-snapshot=20loop?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Row 104: docker/vacuum-site-localdb.sh stops both nodes of a pair together, sets auto_vacuum=INCREMENTAL + VACUUM on each node volume with integrity_check before/after, then starts both (DRY_RUN=1 reports). Run on all three rig pairs: six files 0 -> 2, ok, ~11% smaller; site-a replication reconnected with backlog 0 and no watermark change — VACUUM is replication-invisible (ledger keys by declared PK, oplog seq is INTEGER PRIMARY KEY AUTOINCREMENT). Topology guide gains the recipe. Row 105: SiteLocalDbReplicatedTableFkGuardTests builds the real site schema via OnReady (unreplicated config so 0.3.3's own guard cannot mask the culprit) and asserts every SiteLocalDbSetup.ReplicatedTables entry (now internal) exists and has no inbound CASCADE/SET NULL/SET DEFAULT foreign key, with a negative control. The constraint is enforced at build instead of at the first replicated boot. Row 106 (found during 104's live verification, NOT caused by it — the identical 127k-row full snapshot also landed at the earlier deploy restart): SnapshotStreamer.SendAsync stamps as_of_seq = MAX(seq), which is 0 once a converged sender's oplog is ack-pruned to empty; a zero-watermark receiver (rebuilt node) is never lifted, so the empty-oplog horizon rule re-streams a full snapshot on every reconnect. Sender-side one-line fix for LocalDb 0.3.4: as_of_seq = max(MAX(seq), last_acked_seq). --- CLAUDE.md | 2 +- docker/vacuum-site-localdb.sh | 108 +++++++++++++ docs/deployment/topology-guide.md | 27 ++++ .../2026-07-08-deferred-work-register.md | 5 +- .../SiteLocalDbSetup.cs | 2 +- .../SiteLocalDbReplicatedTableFkGuardTests.cs | 145 ++++++++++++++++++ 6 files changed, 285 insertions(+), 4 deletions(-) create mode 100755 docker/vacuum-site-localdb.sh create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbReplicatedTableFkGuardTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 04126198..12882cdc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,7 +93,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - **CDC registration is conditional, and both directions self-heal at boot** (arch-review WP1.3 + WP3.3, `Host/SiteLocalDbSetup.cs`). Capture triggers are installed only when this node has replication configured — `PeerAddress` **or** `ApiKey`, an OR because only the dialling half sets `PeerAddress` while the passive half carries the key alone. An unreplicated node calls **`DeregisterReplicated`** on all ten tables at boot, dropping triggers an earlier build left behind and pruning their oplog/row-version rows (idempotent; logs once at Information when something was actually cleaned). A replicating node registers with **`baselineExistingRows: true`**, which seeds `__localdb_row_version` for pre-existing rows at the LWW floor (HLC `0`, this node's id) and flags a snapshot resync — so turning replication ON for a site that has been running without it now converges on the rows already in the file instead of only on writes made after the restart. Deregistration must be symmetric (the handshake compares registered-table digests fail-closed), so replication is a both-nodes-together change in either direction. LocalDb 0.2.0 also makes backlog depth O(1) and drops the unused `__localdb_oplog_hlc` index; the on-disk bookkeeping schema goes to v2, upgraded in place on open, with no wire change (0.1.x peers still sync). - **LocalDb 0.3.0 (2026-08-15) makes that snapshot flag PER-TABLE** — the residual 0.2.1 explicitly deferred. On-disk bookkeeping schema goes to **v3** (a new `__localdb_snapshot_state` table, upgraded in place on open); still wire-compatible, and `lib_schema_version` deliberately stays `1` because the handshake compares it fail-closed for *equality*. The extension is negotiated by **capability** (`Handshake.supports_partial_snapshot`, `SnapshotBegin.tables` where **empty == all tables**), so a peer that does not advertise it is served a full snapshot — a scoped one would make it advance `last_applied_remote_seq` past deltas it never received. For the same reason **a partial snapshot moves no watermark on either side**; the overlapping deltas are still pumped and absorbed by LWW. Snapshot debt now clears on the receiver's new `SnapshotAck` rather than on enqueue, so a crash mid-snapshot re-streams the same scope. **`SiteLocalDbSetup.cs` needed no restructuring** — it already calls `RegisterReplicated` once per table in a loop, which is exactly what per-table flagging keys off. The consequence to know: **adding an eleventh table to `ReplicatedTables` on an already-replicating site now snapshots that one table**, where through 0.2.x the same edit re-streamed all eleven in full in both directions. First-boot behaviour is unchanged (all ten seed at once ⇒ the flagged set is every registered table ⇒ an ordinary full snapshot), and so is upgrading the rig in place (the ten tables are already ledgered, nothing seeds, nothing is flagged). A future "register the whole list in one call" API would silently give the coarse behaviour back. - **LocalDb 0.3.2 (2026-08-19, ScadaBridge arch-review P4-1 + P4-3; wire-compatible, no schema change):** (1) the sync pump now **faults the live session when a cap-prune invalidates its position** (horizon check on the oplog's min seq before each read batch + on the batch head after it; throws the new public `LocalDbOplogGapException` through the 0.3.1 fault-teardown) — previously a slow-but-alive peer silently skipped the pruned gap and the pair diverged until the next reconnect; (2) **transport keepalives both sides** — client `SocketsHttpHandler` pings via new validated `LocalDb:Replication:KeepAlivePingDelay`/`KeepAlivePingTimeout` (defaults 60s/20s, `Always` policy), and ScadaBridge's site gRPC listener sets Kestrel h2 keepalive (`Host/SiteKestrelSetup.cs` — server-wide, also governs the metrics port); (3) the server's single-stream guard is now **newest-wins with an awaited handover** — a fresh authenticated `Sync` cancels a dead/wedged incumbent and *waits* for it to finish (`TakeoverTimeout` 10s) before starting, so a half-open link can no longer wedge the pair behind `AlreadyExists` while the zombie side reports `LocalDbReplicationConnected=true`. - - **LocalDb 0.3.3 (2026-08-20, ScadaBridge arch-review P4-7; wire-compatible, no schema change):** two latent `LwwApplier` hazards closed before any table could hit them. (1) **`RegisterReplicated` now REFUSES a table that is the parent of a foreign key with a row-mutating `ON DELETE` action (`CASCADE`/`SET NULL`/`SET DEFAULT`) from any other table** (`LocalDbRegistrationException` naming `child.column ON DELETE `; `RESTRICT`/`NO ACTION` still allowed) — the applier's `INSERT OR REPLACE` runs with `foreign_keys=ON`, so the implicit delete fired those actions while CDC capture was suppressed, i.e. child rows changed on one node and never replicated. Registration-time only: SQLite can't add an FK to an existing table, so a later-created child is caught at the next boot's registration. None of ScadaBridge's ten tables has an FK (verified on the live rig file: zero `REFERENCES` across 29 `CREATE TABLE`s) — **keep it that way; a cascading-FK parent can never be one of `ReplicatedTables`** (register row 105). (2) The per-entry apply (delete-or-upsert + row-version ledger write) is now **atomic under a `SAVEPOINT`** — a fault between the two statements used to leave a deleted row with a live ledger entry, which `SnapshotStreamer` warns-and-skips forever; poison-catch semantics unchanged (narrow `SqliteException`, per-entry dead-letter, batch continues). Cost: two extra in-memory statements per applied entry on a backlog drain. + - **LocalDb 0.3.3 (2026-08-20, ScadaBridge arch-review P4-7; wire-compatible, no schema change):** two latent `LwwApplier` hazards closed before any table could hit them. (1) **`RegisterReplicated` now REFUSES a table that is the parent of a foreign key with a row-mutating `ON DELETE` action (`CASCADE`/`SET NULL`/`SET DEFAULT`) from any other table** (`LocalDbRegistrationException` naming `child.column ON DELETE `; `RESTRICT`/`NO ACTION` still allowed) — the applier's `INSERT OR REPLACE` runs with `foreign_keys=ON`, so the implicit delete fired those actions while CDC capture was suppressed, i.e. child rows changed on one node and never replicated. Registration-time only: SQLite can't add an FK to an existing table, so a later-created child is caught at the next boot's registration. None of ScadaBridge's ten tables has an FK (verified on the live rig file: zero `REFERENCES` across 29 `CREATE TABLE`s) — **keep it that way; a cascading-FK parent can never be one of `ReplicatedTables`** (register row 105). (2) The per-entry apply (delete-or-upsert + row-version ledger write) is now **atomic under a `SAVEPOINT`** — a fault between the two statements used to leave a deleted row with a live ledger entry, which `SnapshotStreamer` warns-and-skips forever; poison-catch semantics unchanged (narrow `SqliteException`, per-entry dead-letter, batch continues). Cost: two extra in-memory statements per applied entry on a backlog drain. **Both are pinned here** (2026-08-20): `SiteLocalDbReplicatedTableFkGuardTests` asserts at build time that no `ReplicatedTables` entry has an inbound row-mutating FK (register row 105), and **pre-2026-08-20 site files must be VACUUMed once offline** to get `auto_vacuum=INCREMENTAL` — `docker/vacuum-site-localdb.sh` (pair stopped together; done on all three rig pairs; topology guide → *one-off offline VACUUM*; row 104). **Known LocalDb defect, register row 106 (fix = 0.3.4):** a converged pair whose sender oplog is ack-pruned to empty stamps snapshots `as_of_seq 0`, so a peer with a zero watermark (rebuilt node) gets a FULL snapshot on every reconnect until the sender writes again — the rig's site-a-a does this today (127 k rows per restart). - **LocalDb 0.2.1** (two adversarial-review findings against 0.2.0, wire-compatible, no schema change) fixes: (1) `DeregisterReplicated` was deleting the HLC clock's only durable crash anchor along with the table rows — an ungraceful exit (crash, SIGKILL, power loss) on a fully-deregistered file left nothing to recover from, so the reopened clock could restart from wall-clock time and re-issue stamps below HLCs peers already hold (silently discarded by LWW); the current clock is now flushed inside the deregistration transaction, before the prune. (2) The sync inbox was an **unbounded** channel, so inbound memory was a function of what the peer sends — a mutual full snapshot (both sides of a pair baseline unconditionally when replication is enabled late) buffered whole, in memory, on both nodes at once; it is now **bounded at 64 messages** with wait-mode backpressure (unrelated to, and not to be confused with, the rig's separate `MaxBatchSize: 64` DB-read-page pin above), which required moving the snapshot to run alongside the receive loop (not inside the pre-loop handshake) so the inbox is drained for the whole session and can't deadlock itself. - All timestamps are UTC throughout the system. - Inter-cluster communication uses **three** transports, not two — **all cross-cluster command/control and data now rides gRPC** after the ClusterClient→gRPC migration's Phase 4 (`docs/plans/2026-07-22-clusterclient-to-grpc-plan.md`) deleted Akka `ClusterClient`/`ClusterClientReceptionist`: (1) **gRPC command/control** — site→central over the central-hosted `CentralControlService` (`GrpcCentralTransport`, sticky central-a→central-b channel pair; deployments/notifications/health/heartbeat/audit-ingest/reconcile), and central→site over the site-hosted `SiteCommandService` (`GrpcSiteTransport`, per-site NodeA→NodeB channel pair; the 28 lifecycle/OPC-UA/query/parked/route/failover commands); (2) **gRPC** server-streaming for real-time data (attribute values, alarm states, `SiteStreamService`); and (3) **plain token-gated HTTP** for the deployment config itself — notify-and-fetch, the site pulls the config from `DeploymentConfigEndpoints` (`ManagementService/DeploymentConfigEndpoints.cs`) with an `X-Deployment-Token` header, `AllowAnonymous` with the per-deployment token as the entire security boundary. The gRPC boundary is per-site PSK-authenticated (`ControlPlaneAuthInterceptor`, unchanged). There is **no receptionist registration** — discovery is by dialling configured endpoints; central builds one `SitePairChannelProvider` per site (addresses from `Site.GrpcNodeAAddress`/`GrpcNodeBAddress`, refreshed from the DB every 60s and on admin changes), sites dial `ScadaBridge:Communication:CentralGrpcEndpoints` (both central nodes, h2c on `CentralGrpcPort` 8083, **NOT** via Traefik). **Discovery is asymmetric by design:** central discovers site gRPC addresses from the *database* (refreshable at runtime), sites discover central from *appsettings* (`CentralGrpcEndpoints`, static — restart required; `StartupValidator` requires a Site node to list at least one). `Akka.Cluster.Tools` stays for ClusterSingleton; only the ClusterClient part is gone. **Central never buffers for an unreachable site** — the send fails with the caller's Ask/deadline timing out; a `ConnectionStateChanged` mechanism built for this was deleted as dead code. diff --git a/docker/vacuum-site-localdb.sh b/docker/vacuum-site-localdb.sh new file mode 100755 index 00000000..42bf931b --- /dev/null +++ b/docker/vacuum-site-localdb.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# docker/vacuum-site-localdb.sh +# +# One-off maintenance: convert an EXISTING site LocalDb file to auto_vacuum=INCREMENTAL. +# +# Why this exists (deferred-work register row 104, arch-review 2026-08-19 P4-6): SQLite only +# honours `PRAGMA auto_vacuum` on a table-less database, and until Low-wave T4 the pragma ran +# AFTER LocalDb had created its bookkeeping tables — so every site file created before that +# fix is permanently auto_vacuum=NONE, and the event-log cap trim's `PRAGMA incremental_vacuum` +# is a silent no-op on it: freed `site_events` pages are reused but the file never shrinks. +# Fresh files are created correctly now (`SiteLocalDbSetup.EnsureAutoVacuumOnFreshFile`); +# this script is the recipe for the files that already exist. +# +# Changing auto_vacuum on a populated file requires a full VACUUM (a rewrite of the file), +# which is why it is NOT done automatically at boot: on a ~1 GB file it holds an exclusive +# lock for minutes. It must run with BOTH nodes of the pair STOPPED: +# - stopped together, per the topology guide's "stop and start BOTH nodes together" rule +# (the pair is a replication unit; a rolling restart is unsupported); +# - VACUUM is CDC-invisible (it fires no triggers and changes no row content), the LWW +# ledger keys by declared primary key (`(table_name, pk_json)`), and the oplog `seq` +# watermark is an INTEGER PRIMARY KEY AUTOINCREMENT — all preserved by VACUUM. So the +# pair's replication state is untouched; it simply resumes when the nodes restart. +# +# Usage: +# bash docker/vacuum-site-localdb.sh # site = a | b | c (one pair) +# bash docker/vacuum-site-localdb.sh --all # every site pair, one at a time +# DRY_RUN=1 bash docker/vacuum-site-localdb.sh a # report the current pragma only +# +# Destructive? No data is deleted; but the pair is OFFLINE for the duration (seconds on the +# rig, minutes on a 1 GB production file). Each file is integrity-checked before AND after. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +COMPOSE="docker compose -f ${SCRIPT_DIR}/docker-compose.yml" +VOLUME_PREFIX="scadabridge-site" # volumes: scadabridge-site--node--data +DB_FILE="site-localdb.db" # LocalDb:Path on the rig = /app/data/site-localdb.db +SQLITE_IMAGE="alpine:3.20" # throwaway container; sqlite3 installed at run time +DRY_RUN="${DRY_RUN:-0}" + +usage() { sed -n '2,30p' "${BASH_SOURCE[0]}"; exit 1; } +[[ $# -eq 1 ]] || usage + +if [[ "$1" == "--all" ]]; then + SITES=(a b c) +else + [[ "$1" =~ ^[abc]$ ]] || usage + SITES=("$1") +fi + +# Runs a sqlite3 script against the DB inside a node's data volume and prints stdout. +sql_on_volume() { + local volume="$1" script="$2" + docker run --rm -v "${volume}:/data" "$SQLITE_IMAGE" sh -ec \ + "apk add --no-cache --quiet sqlite >/dev/null 2>&1; cd /data; sqlite3 '${DB_FILE}' \"${script}\"" +} + +vacuum_site() { + local site="$1" + local nodes=("site-${site}-a" "site-${site}-b") # compose SERVICE names (containers carry a scadabridge- prefix) + local volumes=("${VOLUME_PREFIX}-${site}-node-a-data" "${VOLUME_PREFIX}-${site}-node-b-data") + + echo "=== site-${site}" + for v in "${volumes[@]}"; do + if ! docker volume inspect "$v" >/dev/null 2>&1; then + echo " volume ${v} does not exist — skipping site-${site}"; return 0 + fi + done + + if [[ "$DRY_RUN" == "1" ]]; then + for v in "${volumes[@]}"; do + echo " ${v}: auto_vacuum=$(sql_on_volume "$v" 'PRAGMA auto_vacuum;') (0=NONE 1=FULL 2=INCREMENTAL)" + done + return 0 + fi + + echo " stopping BOTH nodes together: ${nodes[*]}" + $COMPOSE stop "${nodes[@]}" >/dev/null + + for v in "${volumes[@]}"; do + echo " ${v}:" + local before size_before size_after ic + before="$(sql_on_volume "$v" 'PRAGMA auto_vacuum;')" + size_before="$(docker run --rm -v "${v}:/data" "$SQLITE_IMAGE" stat -c %s "/data/${DB_FILE}")" + ic="$(sql_on_volume "$v" 'PRAGMA integrity_check;')" + if [[ "$ic" != "ok" ]]; then + echo " integrity_check BEFORE vacuum is not ok — aborting without touching the file:"; echo "$ic" + $COMPOSE start "${nodes[@]}" >/dev/null; exit 2 + fi + if [[ "$before" == "2" ]]; then + echo " already auto_vacuum=INCREMENTAL (${size_before} bytes) — nothing to do" + continue + fi + # The pragma is recorded in the header by the VACUUM that follows it (SQLite docs: + # "auto-vacuuming can only be changed by a VACUUM after setting the pragma"). + sql_on_volume "$v" 'PRAGMA auto_vacuum = INCREMENTAL; VACUUM;' >/dev/null + ic="$(sql_on_volume "$v" 'PRAGMA integrity_check;')" + size_after="$(docker run --rm -v "${v}:/data" "$SQLITE_IMAGE" stat -c %s "/data/${DB_FILE}")" + echo " auto_vacuum ${before} -> $(sql_on_volume "$v" 'PRAGMA auto_vacuum;'); size ${size_before} -> ${size_after} bytes; integrity_check=${ic}" + [[ "$ic" == "ok" ]] || { echo " integrity_check AFTER vacuum failed — pair left STOPPED for inspection"; exit 3; } + done + + echo " starting BOTH nodes together" + $COMPOSE start "${nodes[@]}" >/dev/null +} + +for s in "${SITES[@]}"; do vacuum_site "$s"; done +echo "done." diff --git a/docs/deployment/topology-guide.md b/docs/deployment/topology-guide.md index a24017c6..6f616bd1 100644 --- a/docs/deployment/topology-guide.md +++ b/docs/deployment/topology-guide.md @@ -357,6 +357,33 @@ a rejoin is safe and self-correcting (verified live: a node stopped and restarte with both nodes byte-identical and zero duplicates). Beyond it, rebuild the returning node's database from its peer rather than letting it rejoin. +### Site LocalDb files created before 2026-08-20 need a one-off offline VACUUM + +SQLite honours `PRAGMA auto_vacuum` only on a table-less database, and until arch-review +2026-08-19 P4-6 the pragma ran AFTER LocalDb had created its bookkeeping tables — so every site +file created before that fix is `auto_vacuum=NONE`, and the event-log cap trim's +`PRAGMA incremental_vacuum` is a silent no-op on it: freed `site_events` pages are reused, but the +file never shrinks (deferred-work register row 104). Fresh files are now created +`auto_vacuum=INCREMENTAL`; existing files need a **full `VACUUM` with the pragma set**, which +rewrites the file and is therefore deliberately NOT done at boot (minutes of exclusive lock on a +~1 GB file). + +Run it in a maintenance window with **both nodes of the pair stopped** (the rule above): + +```bash +DRY_RUN=1 bash docker/vacuum-site-localdb.sh --all # report: 0 = NONE, 2 = INCREMENTAL +bash docker/vacuum-site-localdb.sh a # one pair: stop both → VACUUM both → start both +``` + +The script integrity-checks each file before and after and leaves the pair stopped if the +post-check fails. It is safe for a replicated pair: `VACUUM` fires no triggers and changes no +row content, the LWW ledger keys by declared primary key, and the oplog `seq` watermark is an +`INTEGER PRIMARY KEY AUTOINCREMENT` — all preserved by `VACUUM`. Verified on the docker rig +2026-08-20 on all three pairs: site-a (replicated) came back `LocalDbReplicationConnected=true` +with backlog 0 and no schema or watermark change. For a non-docker deployment the recipe is the +same three statements against the stopped node's `LocalDb:Path` file: +`PRAGMA auto_vacuum = INCREMENTAL; VACUUM; PRAGMA integrity_check;`. + ### Docker-rig data files on macOS/OrbStack — a virtiofs corruption + availability hazard **Status: MITIGATED 2026-08-18 (register row 79) — the rigs' data dirs moved to named docker diff --git a/docs/plans/2026-07-08-deferred-work-register.md b/docs/plans/2026-07-08-deferred-work-register.md index 574e986f..2c99db7a 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -96,8 +96,9 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 101 | ~~**`ApplyingArtifacts_RemovedDataConnection_StopsLiveDclConnection` flaked once — suspected overlay-sweep read racing the artifact-apply write**~~ **RESOLVED 2026-08-17 (`0d3a90a9`) — hypothesis VERIFIED, and the production consequence was WORSE than the row guessed.** The race is real: the snapshot is read off the actor thread but compared against the overlay only when the piped message lands, so a read starting before an artifact apply's store writes can land after `HandleApplyArtifactDataConnectionsToDcl` populated the overlay — its stale (even empty) snapshot then "proves" the just-applied names disappeared. Overlay iteration order explains the exact fingerprint (`PlcA` removed first). Two corrections to the row's guesses: (1) it is NOT "self-healing at the next tick" — the removal evicts the hash slot the overlay's push is gated on, so the next fresh sweep clears the tombstone but re-creates nothing; the stopped connection stays dead **until an instance redeploy**; (2) the mid-write "momentarily not seeing the row" framing was the lesser half — the whole-snapshot staleness (read-before-write, land-after-apply) is the dominant window. Fix: `_definitionsWriteGeneration`, bumped on the actor thread by every artifact apply and captured at every snapshot-read dispatch (boot load + sweep); a result carrying a stale generation is discarded WHOLE — additions included, since a stale row pushed at a live connection would reconfigure it backward and flip-flop with the next fresh read. Deterministic repro fails before the fix (`StaleSweepSnapshot_RacingAnArtifactApply_DoesNotRemoveJustAppliedConnections`); `FreshEmptySweepSnapshot_StillRemovesDisappearedConnections` pins that row-90's legitimate disappearance still fires. SiteRuntime.Tests 647/647; rig redeployed, zero spurious removals/discards at boot, all rig-probe-1 tags Good. Memo §10 records the mechanism. **Postscript 2026-08-18 (merge `505f3569`, agent commit `f02e1166`): extended to the WHOLE snapshot.** The instance-config half had the same TOCTOU, and all three hazards were PROVEN real pre-fix: a stale sweep restarts an instance **backward** (`TryRestartStaleInstance` feeds the snapshot's old `ConfigJson` to `HandleDeploy` — a literal old-config redeploy; pipeline ownership only covers in-flight deploys and the old revision was never claimed), a read-pre-delete/disable **resurrects** a just-stopped instance (delete even clears the sweep claim), and the boot path skews `_deployedInstanceNames`. Fix: one `_storeWriteGeneration` over both tables, bumped at BOTH ends of every actor-thread config mutation (deploy persist, enable, disable, delete, rollback, artifact apply) — both ends because `ApplyDeployment`/`HandleDisable`/`HandleDelete` advance actor state BEFORE the off-thread write, so a completion-only bump (the row-101 original argument, which holds only for the artifact apply) leaves a window. Stale sweep discarded whole; stale boot load re-dispatched, capped at 5 consecutive (then applied with a Warning). CDC/`SiteReconciliationActor` writes deliberately never bump — discovering them is the sweep's purpose. 3 new fail-before tests + 2 fresh-generation guards pinning that row-72/78 legitimate repairs survive; SiteRuntime.Tests 652/652 x3; full suite green (one unrelated Communication.Tests gauge flake → row 102); rig probe: disable/enable/redeploy churn over multiple sweep ticks produced zero self-heal events on either site-a node, tags Good. Memo §11. | Observed by the 2026-08-17 row-100 fix session | Closed 2026-08-17; whole-snapshot extension 2026-08-18. | — | | 102 | ~~**`SiteStreamSubscribersActiveGauge_GoesToOneOnConnect_AndBackToZeroOnCancel` is delta-hardened but still flakes — the baseline can't survive other contributors CLOSING**~~ **RESOLVED 2026-08-18 via the preferred option (row-96 pattern).** The survey found the gauge moves ONLY inside the real `SiteStreamGrpcServer.SubscribeInstance` handler, and exactly two test classes drive it: `SiteStreamGrpcServerTests` (the gauge test's own class) and `CleanupVerificationTests` — `GrpcOptionsWiringTests` instantiates the server but never opens a stream, and the client/aggregator suites subscribe against fakes, so none of those can perturb the counter. Both drivers now share the `[Collection("SiteStreamSubscriberGauge")]` xunit collection (comments on both classes + at the baseline poll explain why the relative read alone was insufficient), so no other contributor can close a stream between the baseline capture and the assertions. Verified 4/4 consecutive full-suite runs green (717/717), durations unchanged (22–25 s vs the prior 24 s) — the serialization is free. Original evidence: the test polls `baseline + 1`, but the process-global gauge's baseline is captured ONCE; capture while parallel classes' streams are up → they close → the gauge settles at this test's own 1 and `baseline+1` is unreachable forever — a fixed-baseline delta only survives contributors that hold constant or increase. | Observed 2026-08-18 (one full-suite failure, `Expected: 4, Actual: 1`; 3/3 green in isolation); fixed same day | Closed 2026-08-18. | — | | 103 | ~~**Two lesser TTL-bounded circuit-teardown-vs-async-setup races remain in CentralUI**~~ **RESOLVED 2026-08-20 (Low-wave T12, agent commit `c8452136`).** Both instances closed with the Medium-wave T10 take-once discipline: `TransportImport.razor.cs` re-checks disposal after `TryLoadAsync`'s await and releases the import slot through exactly-once ownership (dispose-during-start frees the `BundleSession` slot immediately instead of stranding it for the 30-min TTL), and `Topology.razor` creates its timer under the same post-await re-check so a disposed page leaves no live timer (asserted event-style, not by sleep). Original note: (arch-review 2026-08-19 P6-6's optional tail, deliberately deferred by the fix). The permanent debug-stream leak was closed at `DebugView.razor` + `DebugStreamHub` (dispose-vs-connect take-once pattern, commit `11e623c2`), but two same-family instances with a *different shape* were left: (1) `TransportImport.razor.cs TryLoadAsync` (~:244) — a circuit torn down mid-load strands 1 of the 8 `MaxConcurrentImportSessions` slots until the 30-min `BundleSession` TTL reaps it (partially defeats row 43); (2) `Topology.razor` (~:201-214) — timer created before a disposal re-check. Both are TTL/lifetime-bounded (no permanent leak), which is why they missed the Medium bar. Fix is the same post-await `_disposed` re-check + take-once teardown, but routed through `BundleSession.ReleaseSession`/the wizard-step lifecycle rather than a session-stop call. | Arch-review 2026-08-19 P6-6 remediation (T10 explicitly scoped them out) | Open. | Next CentralUI hardening pass, or opportunistically when either file is next touched. | -| 104 | **Existing site LocalDb files stay `auto_vacuum=NONE`; `PRAGMA incremental_vacuum` after a cap trim is a no-op on them** | Arch-review 2026-08-19 P4-6 (Low-wave T4, `47f03fe6`) | `auto_vacuum` is only honoured on a table-less database, and `LocalDbSchema.EnsureCreated` had always created LocalDb's bookkeeping tables before `SiteEventLogSchema.Apply` ran — so the pragma had never once taken effect on any real file. The fix (`SiteLocalDbSetup.EnsureAutoVacuumOnFreshFile`, run before `AddZbLocalDb`'s lazy singleton constructs `SqliteLocalDb`) only helps **fresh** files; an unconditional `VACUUM` against a live ~1 GB replicated file was explicitly rejected (long exclusive lock on the active node; the rewrite is CDC-invisible but blocks the sync pump). Every node on the docker rig today predates the fix, so freed `site_events` pages there are reused, not returned to the OS — the storage cap (row 81) still bounds the data volume, only the file size stops shrinking. | Operator-scheduled `VACUUM` in a maintenance window (both nodes of a pair stopped together), or when a site's file is next recreated. | -| 105 | **Design constraint (not a defect): a LocalDb-replicated table must never be the parent of a row-mutating `ON DELETE` foreign key (`CASCADE`/`SET NULL`/`SET DEFAULT`) from another table** | Arch-review 2026-08-19 P4-7 → LocalDb 0.3.3 (scadaproj `122940c` + review follow-up `9663279`, Low-wave T5) | `LwwApplier` upserts with `INSERT OR REPLACE` under `foreign_keys=ON`, so the implicit delete fires the child action while CDC capture is suppressed — the child rows change on the applying node only and the pair diverges silently. 0.3.3 makes `RegisterReplicated` refuse such a parent at registration (clear exception naming the child FK), which is a BOOT failure for the site node, so this is enforced, not advisory. None of the ten `SiteLocalDbSetup.ReplicatedTables` has any FK today (zero `REFERENCES` on the live rig file). Model cross-table cleanup as explicit application deletes on each table (as the 3-table deploy cascade already does), never as SQLite FK actions. | Whenever a schema change adds a foreign key between site tables, or an eleventh table joins `ReplicatedTables`. | +| 104 | ~~**Existing site LocalDb files stay `auto_vacuum=NONE`; `PRAGMA incremental_vacuum` after a cap trim is a no-op on them**~~ **RESOLVED 2026-08-20 — recipe shipped and applied to the rig.** New `docker/vacuum-site-localdb.sh` (stop both nodes of a pair together → `PRAGMA auto_vacuum=INCREMENTAL; VACUUM` each volume with integrity checks before/after → start both; `DRY_RUN=1` reports) plus the topology-guide section *Site LocalDb files created before 2026-08-20 need a one-off offline VACUUM*. Run on all three rig pairs: all six files `auto_vacuum 0 → 2`, `integrity_check=ok`, ~11 % smaller (site-a 20.3 → 18.1 MB), site-a replication back `Connected=true`/backlog 0 with no watermark or schema change — the VACUUM was replication-invisible as predicted (ledger keys by declared PK; oplog `seq` is an `INTEGER PRIMARY KEY AUTOINCREMENT`). The same maintenance surfaced the unrelated row 106. Original: | Arch-review 2026-08-19 P4-6 (Low-wave T4, `47f03fe6`) | `auto_vacuum` is only honoured on a table-less database, and `LocalDbSchema.EnsureCreated` had always created LocalDb's bookkeeping tables before `SiteEventLogSchema.Apply` ran — so the pragma had never once taken effect on any real file. The fix (`SiteLocalDbSetup.EnsureAutoVacuumOnFreshFile`, run before `AddZbLocalDb`'s lazy singleton constructs `SqliteLocalDb`) only helps **fresh** files; an unconditional `VACUUM` against a live ~1 GB replicated file was explicitly rejected (long exclusive lock on the active node; the rewrite is CDC-invisible but blocks the sync pump). Every node on the docker rig today predates the fix, so freed `site_events` pages there are reused, not returned to the OS — the storage cap (row 81) still bounds the data volume, only the file size stops shrinking. | Operator-scheduled `VACUUM` in a maintenance window (both nodes of a pair stopped together), or when a site's file is next recreated. | +| 105 | ~~**Design constraint (not a defect): a LocalDb-replicated table must never be the parent of a row-mutating `ON DELETE` foreign key (`CASCADE`/`SET NULL`/`SET DEFAULT`) from another table**~~ **PINNED 2026-08-20.** Enforcement moved from the first replicated boot to the build: `SiteLocalDbReplicatedTableFkGuardTests` (Host.Tests) builds the real site schema via `OnReady` on an unreplicated config (so 0.3.3's own guard can't mask which FK is at fault), walks `SiteLocalDbSetup.ReplicatedTables` (now `internal`), asserts each exists and has no inbound row-mutating `ON DELETE` FK with the same `sqlite_master JOIN pragma_foreign_key_list` query LocalDb uses, and carries a negative control proving the checker finds a cascading child. The constraint itself stands — the row is kept as its record. Original: | Arch-review 2026-08-19 P4-7 → LocalDb 0.3.3 (scadaproj `122940c` + review follow-up `9663279`, Low-wave T5) | `LwwApplier` upserts with `INSERT OR REPLACE` under `foreign_keys=ON`, so the implicit delete fires the child action while CDC capture is suppressed — the child rows change on the applying node only and the pair diverges silently. 0.3.3 makes `RegisterReplicated` refuse such a parent at registration (clear exception naming the child FK), which is a BOOT failure for the site node, so this is enforced, not advisory. None of the ten `SiteLocalDbSetup.ReplicatedTables` has any FK today (zero `REFERENCES` on the live rig file). Model cross-table cleanup as explicit application deletes on each table (as the 3-table deploy cascade already does), never as SQLite FK actions. | Whenever a schema change adds a foreign key between site tables, or an eleventh table joins `ReplicatedTables`. | +| 106 | **LocalDb: a converged pair re-streams a FULL snapshot on EVERY reconnect once the sender's oplog has been ack-pruned to empty and the receiver's watermark is 0** | Found 2026-08-20 while verifying row 104 on the rig (site-a-a received the same 127 k-row full snapshot at both the 13:09 deploy restart and the 13:25 VACUUM restart) | Sender-side defect in `SnapshotStreamer.SendAsync`: `as_of_seq = MAX(seq)` of the oplog, which is **0** once the pair has converged and ack-pruning emptied it. The receiver's `SetLastAppliedRemoteSeqAsync` is MAX-guarded (no regression — good), but a peer whose watermark is genuinely 0 (site-a-a was rebuilt after the 2026-08-15 corruption; any wiped/re-imaged node is the same) can never be lifted by an `as_of_seq 0` snapshot, so the 0.1.2 empty-oplog horizon rule (`streamableFrom = last_acked_seq + 1`) sees `0 < 149` as a gap on every handshake and re-streams everything. Rig state: node-b `last_acked_seq=149`, oplog empty; node-a `last_applied_remote_seq=0`. Not a divergence (LWW absorbs the re-delivered rows) but a cost/availability bug — ~1 s and 127 k rows here, minutes and a full-file read on a 1 GB production pair, on every reconnect, until the sender happens to write a new row. Fix is sender-side and wire-compatible: stamp `as_of_seq = max(MAX(seq), last_acked_seq)` (a full snapshot taken now covers every delta ever pruned below `last_acked_seq`), so the receiver's MAX-guarded watermark lifts to 149 and the loop ends; add a pinning test (converged+pruned sender, zero-watermark receiver → exactly one snapshot across two handshakes). Ships as LocalDb 0.3.4. | Next LocalDb release (0.3.4) — small, well-understood, and worth doing before any production pair is rebuilt from its peer. | | 68 | ~~**`deploy.sh` run from a git worktree silently rehomes every node's bind-mount data dir**~~ **RESOLVED 2026-08-15 (guard added in the same close-out).** The compose files' bind mounts are relative (`./site-a-node-a/data`), so a deploy from a worktree checkout mounts the worktree's copies — the rig keeps running and looks healthy, but its SQLite stores split from the main checkout's and everything written there is orphaned by the next deploy from main. Bit the rows-46–53 close-out live: the seeding agent's `rig-probe-1` deploy persisted into worktree-homed dirs and "vanished" after the orchestrator's redeploy (no tombstone, no product bug — LWW/tombstone forensics ruled out deletion; the row sat in `.claude/worktrees/…/docker/site-a-node-a/data/site-localdb.db` the whole time). Both `docker/deploy.sh` and `docker-env2/deploy.sh` now refuse to run from a `.claude/worktrees/` path (override: `SCADABRIDGE_ALLOW_WORKTREE_DEPLOY=1`). | Found during the rows-46–53 close-out | Closed 2026-08-15. | — | ## Resolved (verified against the code 2026-07-10) diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs index 9d0aeb1e..bb8c125a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs @@ -57,7 +57,7 @@ public static class SiteLocalDbSetup /// both branches on purpose: a table added to one and not the other would either register /// without ever being cleaned up, or be cleaned up on a node that never registered it. /// - private static readonly string[] ReplicatedTables = + internal static readonly string[] ReplicatedTables = [ "OperationTracking", "site_events", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbReplicatedTableFkGuardTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbReplicatedTableFkGuardTests.cs new file mode 100644 index 00000000..998b93fa --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbReplicatedTableFkGuardTests.cs @@ -0,0 +1,145 @@ +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.LocalDb; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred-work register row 105 (arch-review 2026-08-19 P4-7 → LocalDb 0.3.3): a replicated +/// table must never be the parent of a foreign key with a row-mutating ON DELETE action +/// (CASCADE / SET NULL / SET DEFAULT). LocalDb's LWW applier upserts with +/// INSERT OR REPLACE under foreign_keys=ON, so the implicit delete would fire the +/// child action while change-capture is suppressed — the child rows change on the applying node +/// only and the pair diverges silently. LocalDb 0.3.3 refuses such a parent at +/// RegisterReplicated time, which on a site node is a BOOT failure of the replicating +/// pair. This test moves that discovery from the first replicated boot to the build: it walks +/// the real site schema (every table SiteLocalDbSetup.OnReady creates) and asserts none +/// of the ten has such an inbound FK. +/// +/// +/// The schema is built with an UNREPLICATED config on purpose: a replicated one would make +/// 0.3.3's own guard throw first and mask which table/FK is at fault. The checker query mirrors +/// LocalDb's (sqlite_master JOIN pragma_foreign_key_list), and a negative control proves +/// it actually finds an offender. +/// +public class SiteLocalDbReplicatedTableFkGuardTests +{ + [Fact] + public void NoReplicatedTable_IsAParentOfARowMutatingOnDeleteForeignKey() + { + var path = TempDbPath(); + try + { + var config = ConfigFor(path); + var services = new ServiceCollection(); + services.AddZbLocalDb(config, db => SiteLocalDbSetup.OnReady(db, config)); + using (var provider = services.BuildServiceProvider()) + using (var scope = provider.CreateScope()) + { + // Resolving ILocalDb runs SqliteLocalDb's constructor and then OnReady — i.e. + // creates every table the consolidated site file will ever hold. + Assert.NotNull(scope.ServiceProvider.GetRequiredService()); + } + + var tables = TableNames(path); + foreach (var table in SiteLocalDbSetup.ReplicatedTables) + { + Assert.Contains(table, tables); // the list and the schema must not drift apart + + var offenders = FindRowMutatingFkChildren(path, table); + Assert.True(offenders.Count == 0, + $"Replicated table '{table}' is the parent of a row-mutating ON DELETE foreign key " + + $"({string.Join(", ", offenders)}). LocalDb 0.3.3 refuses to register it, so a " + + "replicating site pair would fail to boot — model cross-table cleanup as explicit " + + "application deletes instead (register row 105)."); + } + } + finally + { + SqliteConnection.ClearAllPools(); + Cleanup(path); + } + } + + [Fact] + public void NegativeControl_CheckerFindsACascadingChild() + { + // The checker must be able to fail, or the test above proves nothing. + var path = TempDbPath(); + try + { + using (var conn = Open(path)) + using (var cmd = conn.CreateCommand()) + { + cmd.CommandText = + "CREATE TABLE parent_t (id TEXT PRIMARY KEY);" + + "CREATE TABLE child_cascade (id INTEGER PRIMARY KEY, parent_id TEXT REFERENCES parent_t(id) ON DELETE CASCADE);" + + "CREATE TABLE child_restrict (id INTEGER PRIMARY KEY, parent_id TEXT REFERENCES parent_t(id) ON DELETE RESTRICT);"; + cmd.ExecuteNonQuery(); + } + + var offenders = FindRowMutatingFkChildren(path, "parent_t"); + + var only = Assert.Single(offenders); + Assert.Equal("child_cascade.parent_id ON DELETE CASCADE", only); + } + finally + { + SqliteConnection.ClearAllPools(); + Cleanup(path); + } + } + + /// Same shape as LocalDb 0.3.3's EnsureNoInboundActionOnDelete query. + private static List FindRowMutatingFkChildren(string path, string parentTable) + { + using var conn = Open(path); + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT m.name, fk.\"from\", fk.\"on_delete\" " + + "FROM sqlite_master m JOIN pragma_foreign_key_list(m.name) fk " + + "WHERE m.type = 'table' AND fk.\"table\" = $t COLLATE NOCASE " + + "AND UPPER(fk.\"on_delete\") IN ('CASCADE', 'SET NULL', 'SET DEFAULT')"; + cmd.Parameters.AddWithValue("$t", parentTable); + using var reader = cmd.ExecuteReader(); + var result = new List(); + while (reader.Read()) + result.Add($"{reader.GetString(0)}.{reader.GetString(1)} ON DELETE {reader.GetString(2)}"); + return result; + } + + private static HashSet TableNames(string path) + { + using var conn = Open(path); + using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT name FROM sqlite_master WHERE type = 'table'"; + using var reader = cmd.ExecuteReader(); + var names = new HashSet(StringComparer.OrdinalIgnoreCase); + while (reader.Read()) names.Add(reader.GetString(0)); + return names; + } + + private static SqliteConnection Open(string path) + { + var conn = new SqliteConnection(new SqliteConnectionStringBuilder { DataSource = path }.ToString()); + conn.Open(); + return conn; + } + + private static string TempDbPath() => + Path.Combine(Path.GetTempPath(), $"scadabridge_fkguard_{Guid.NewGuid():N}.db"); + + private static IConfiguration ConfigFor(string path) => + new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { ["LocalDb:Path"] = path }) + .Build(); + + private static void Cleanup(string path) + { + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + try { File.Delete(path + suffix); } catch (IOException) { } + } + } +}