From 232bff5df7e1c35a0fbe2020d1493e5a1e9e4b1c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Jul 2026 01:27:02 -0400 Subject: [PATCH 1/3] fix(localdb): close both Phase 1 live-gate follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Wiped-node back-fill (live gate check 4). Fixed upstream in ZB.MOM.WW.LocalDb 0.1.2 and pinned here. The gate's diagnosis was slightly off: the oplog cap was not the gate. Snapshot detection measured the peer's gap against the oldest surviving oplog row and read an empty oplog as "no gap possible" — and an empty oplog is the steady state of a converged pair, since ack-pruning deletes everything the peer confirmed. The healthy state was the one state that could not heal a wiped peer. Now measured against last_acked_seq when the oplog is empty. Pinned at this level too, not just the library's: the pair harness grows a WipePassiveAsync (a NEW database — a rebuilt node comes back with a new node id and a zero watermark), and the new scenario asserts the emptied oplog as its precondition before wiping, then requires the deployment artifact to come back byte-identical with no new deploy. Verified RED against the pinned 0.1.1 (times out waiting for back-fill) and green on 0.1.2. 2. Oplog growth on default-OFF nodes (live gate check 8), fixed at the source: StoreAsync now skips entirely when the pointer already names this deployment/revision, the SHA matches the bytes, and the expected chunk count is present. Re-caching an artifact the node already holds — every restart's boot-from-cache, every RestoreApplied — writes nothing and mints no oplog rows, where before it cost a delete plus an insert per chunk plus a pointer update, all identical to what was already there. Identity is over the bytes and the chunks are counted, both deliberately: a re-composed artifact can carry the same ids with different bytes, and a pointer can name a deployment whose chunks are missing, where skipping would make an unreadable cache permanent. Both guards verified RED against a naive pointer-only skip. The gate's stated bound was also wrong and is corrected in the doc: growth was never headed for the 1M row cap. AddZbLocalDbReplication is registered unconditionally, so MaintenanceBackgroundService runs on default-OFF nodes and the 7-day MaxOplogAge cap prunes. Runtime.Tests 412/0/31, Host.IntegrationTests LocalDb 45/45. Pre-existing and unrelated: AbCip_Green_AgainstSim (needs the AB CIP docker fixture, red on a clean master too) and a flaky Roslyn race test (green 2/2 in isolation). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- Directory.Packages.props | 4 +- .../2026-07-20-localdb-pair-replication.md | 9 ++ .../2026-07-20-localdb-phase1-live-gate.md | 27 +++++- .../LocalDbDeploymentArtifactCache.cs | 76 ++++++++++++++++ .../LocalDb/LocalDbPairConvergenceTests.cs | 48 ++++++++++ .../LocalDb/LocalDbPairHarness.cs | 56 ++++++++++-- .../LocalDbDeploymentArtifactCacheTests.cs | 88 +++++++++++++++++++ 7 files changed, 295 insertions(+), 13 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index b62ff772..c569cf82 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -127,8 +127,8 @@ package floors the native SQLitePCLRaw.lib.e_sqlite3 at 2.1.12, so consumers inherit the CVE-2025-6965-fixed native binary without needing the surgical pin below. --> - - + + diff --git a/docs/operations/2026-07-20-localdb-pair-replication.md b/docs/operations/2026-07-20-localdb-pair-replication.md index 5bc1dea5..c8d979cb 100644 --- a/docs/operations/2026-07-20-localdb-pair-replication.md +++ b/docs/operations/2026-07-20-localdb-pair-replication.md @@ -72,6 +72,15 @@ The replication library's passive endpoint verifies **no** authentication — th If a node is offline **longer than the tombstone-retention window**, a delete that happened during its outage may no longer be expressible as a tombstone on rejoin — a pruned deployment could briefly reappear until the next deploy re-prunes it. Keep pair outages well inside that window. +- **A rebuilt node is back-filled by its peer.** If a node loses its LocalDb file — a wiped volume, + a re-imaged host, a fresh container — it rejoins with an empty database and its peer snapshots the + cached configuration back to it, with no new deploy and no central SQL. Two things are worth + knowing: this needs **`ZB.MOM.WW.LocalDb` ≥ 0.1.2** (on `0.1.1` a converged pair has pruned every + oplog row on ack, and the wiped node was never snapshotted — it stayed empty until the next + deploy), and it heals the *cache*, so the node regains boot-from-cache for future outages. A node + wiped **during** a central outage is still repopulated by its peer under 0.1.2; a node with **no** + peer replication configured self-heals only from central on its next successful apply. + - **What boot-from-cache does NOT cover.** The cache is a *fallback for central-SQL outages at boot*, not a replacement for central. A **new deployment still requires central SQL** — the cache is only read when the central fetch fails at startup. When central returns, the node resumes diff --git a/docs/plans/2026-07-20-localdb-phase1-live-gate.md b/docs/plans/2026-07-20-localdb-phase1-live-gate.md index 379ca4be..2008438d 100644 --- a/docs/plans/2026-07-20-localdb-phase1-live-gate.md +++ b/docs/plans/2026-07-20-localdb-phase1-live-gate.md @@ -33,11 +33,11 @@ Rig: 6 nodes rebuilt from branch `feat/localdb-phase1` @ `ce9fa07f`, all up. | 1 | Rig up, site-a pair healthy, `/metrics` shows `localdb_` series | ✅ PASS | Both site-a nodes `/healthz` 200 Healthy; `/metrics` shows `localdb_oplog_depth` + `localdb_sync_reconnects_total` on meter `ZB.MOM.WW.LocalDb.Replication` (allowlist works); site-a-2 served `/localdb_sync.v1.LocalDbSync/Sync` with routing `match_status=success` (interceptor passed, session established); both oplogs depth 0 | | 2 | Deploy → both site-a nodes' pointer + artifacts byte-identical, same HLC + origin | ✅ PASS | Deploy `6e687451…` (rev `efb04c79…`) via `POST :9200/api/deployments` → both site-a-1 and site-a-2 `deployment_pointer` = `SITE-A / 6e687451… / efb04c79… / sha EFB04C79…` (identical); artifact 1 chunk each; **pointer `__localdb_row_version` identical on both: hlc `116955620225646592`, origin node_id `a0576df6-…`, tombstone 0** — one row replicated, not two derivations; both oplogs depth 0 | | 3 | Boot-from-cache: SQL down, restart site-a-2, serves cached config w/ signal | ✅ PASS (found + fixed a defect) | With SQL stopped, site-a-2 restart logged `RUNNING FROM CACHE — … booted deployment 6e687451…`; **address space materialised from the cached blob**: `AddressSpaceApplier: applied plan (added=18)` / `OpcUaPublish: applied rebuild (added=18)`. Browse confirms **16 nodes on site-a-2 = 16 on site-a-1**. **Defect the gate caught (`9137cb41`):** pre-fix the rebuild re-read the artifact from the down ConfigDb and no-op'd, so the cache-booted node served **1** node vs the healthy peer's 16 — it logged success but browsed empty. Fixed by passing the in-hand cached blob to `RebuildAddressSpace`; regression test added | -| 4 | Replicated-cache payoff: wipe a-2 DB, repopulate, then SQL-down boot | ⚠️ PARTIAL — 1 limitation documented, 1 defect fixed | Wiped a-2's LocalDb volume, restarted with **SQL down**: sync session re-established (a-2 served a `Sync` call) but a-2's cache stayed **0/0/0** and a-1's oplog was **0** — the deploy's rows had been acked by the old a-2 and pruned, so there was no delta to send and **no snapshot-resync fired**. ⇒ **Limitation: pure replication does not back-fill a fully-wiped, already-converged node** (library delta-replication; snapshot-resync is gated on the oplog cap, which the running engine never hits). Separately found a **defect (`c6a9f93a`)**: recovery via `RestoreApplied` never wrote the cache (only fresh `ApplyAndAck` did), so a wiped node stayed cache-less. Fixed: `RestoreApplied` now re-caches. Verified — after the fix, wiped a-2 restarted with SQL up: `restored served state … on bootstrap` → cache repopulated `SITE-A / 6e687451`, chunks=1. **Net: a wiped node self-heals its cache from central on next boot (regaining boot-from-cache for future outages); it is not back-filled by its peer during the same outage.** | +| 4 | Replicated-cache payoff: wipe a-2 DB, repopulate, then SQL-down boot | ⚠️ PARTIAL — 1 defect fixed; the limitation is since **CLOSED** (LocalDb 0.1.2 — see Summary) | Wiped a-2's LocalDb volume, restarted with **SQL down**: sync session re-established (a-2 served a `Sync` call) but a-2's cache stayed **0/0/0** and a-1's oplog was **0** — the deploy's rows had been acked by the old a-2 and pruned, so there was no delta to send and **no snapshot-resync fired**. ⇒ **Limitation: pure replication does not back-fill a fully-wiped, already-converged node** (library delta-replication; snapshot-resync is gated on the oplog cap, which the running engine never hits). Separately found a **defect (`c6a9f93a`)**: recovery via `RestoreApplied` never wrote the cache (only fresh `ApplyAndAck` did), so a wiped node stayed cache-less. Fixed: `RestoreApplied` now re-caches. Verified — after the fix, wiped a-2 restarted with SQL up: `restored served state … on bootstrap` → cache repopulated `SITE-A / 6e687451`, chunks=1. **Net: a wiped node self-heals its cache from central on next boot (regaining boot-from-cache for future outages); it is not back-filled by its peer during the same outage.** | | 5 | Transport kill: pause a-2, oplog rises; unpause, drains to 0 | ✅ PASS | `docker pause` on a-2 severed the sync transport; during the window a-2 accumulated **oplog=2** unacked local writes (its re-cache) while a-1 held 0. `docker unpause` → a-2's oplog **drained to 0 within 5s**, both converged on the same pointer. Note: the rig's redeploys carry an identical revision, so a fresh cache write on a-1 short-circuits (no-op) — the store-during-partition variant is covered by the automated harness scenario `WritesWhileTransportDown_SurviveRejoin` (proven red without `RegisterReplicated`) | | 6 | Retention: 3rd deploy prunes oldest; chunks gone + tombstones on BOTH nodes | ✅ PASS | Cached 3 distinct deployment-ids (via deploy + restart re-cache). Both nodes retain exactly the newest 2 (`475df87a`, `962962dc`); the oldest `6e687451`'s chunks are **0 on both** (pruned); **1 artifact tombstone on both** — the prune replicated as a tombstone, not left as a live chunk | | 7 | Both-nodes-together restart: clean rejoin, identical counts, no I/O errors | ✅ PASS | Restarted site-a-1 + site-a-2 together: **0** `disk I/O error`/`SQLITE_IOERR`/`corrupt` lines in either log; both re-cached and converged to **identical** counts (ptr `962962dc…`, chunks=2, rowver=3). Byte-identical convergence preserved across a simultaneous restart | -| 8 | site-b default-OFF pin: cache works locally, Healthy, no sync listener on 9001 | ✅ PASS (+1 finding) | site-b-1 cache works locally (`SITE-B / 6e687451`, chunks=1, from its own apply); listeners **`[::]:8080` only — no 9001**; port 9001 **not bound** (`/proc/net/tcp`); `/healthz` **200**; no sync/reconnect metric series. **Finding (follow-up, not a blocker):** `localdb_oplog_depth=5` — because `OnReady` registers replication unconditionally, a default-OFF node captures cache writes into its oplog but has no peer to ack/prune them, so the oplog grows slowly (≈2 rows/deploy, amplified by the re-cache-on-restore fix). Bounded by the library `MaxOplogRows` cap (1M); worth a periodic pruner or skip-if-unchanged in a follow-up | +| 8 | site-b default-OFF pin: cache works locally, Healthy, no sync listener on 9001 | ✅ PASS (+1 finding, since **CLOSED** by the skip-if-unchanged cache write — see Summary) | site-b-1 cache works locally (`SITE-B / 6e687451`, chunks=1, from its own apply); listeners **`[::]:8080` only — no 9001**; port 9001 **not bound** (`/proc/net/tcp`); `/healthz` **200**; no sync/reconnect metric series. **Finding (follow-up, not a blocker):** `localdb_oplog_depth=5` — because `OnReady` registers replication unconditionally, a default-OFF node captures cache writes into its oplog but has no peer to ack/prune them, so the oplog grows slowly (≈2 rows/deploy, amplified by the re-cache-on-restore fix). Bounded by the library `MaxOplogRows` cap (1M); worth a periodic pruner or skip-if-unchanged in a follow-up | ## Summary @@ -56,13 +56,32 @@ missed**, each now fixed on-branch with a regression test: 4. **Cache not repopulated on `RestoreApplied`** (`c6a9f93a`) — a wiped/fresh-volume node recovered its served state from central but stayed cache-less, unable to boot-from-cache on the next outage. -**Two documented limitations (follow-ups, not blockers):** +**Two documented limitations (follow-ups, not blockers) — both since CLOSED:** - **No replication back-fill of a fully-wiped node** (check 4): a peer's already-acked cache rows are pruned from its oplog and snapshot-resync is gated on the (never-hit) oplog cap, so a wiped node is not healed by its peer — it self-heals from central on next boot instead. + → **FIXED in `ZB.MOM.WW.LocalDb` 0.1.2** (scadaproj `cad3bcb`). The diagnosis in this doc was + slightly off: the cap was not the gate. `ComputeSnapshotRequiredAsync` measured the peer's gap + against the *oldest surviving oplog row* and read an empty oplog as "no gap possible" — and an + empty oplog is the **steady state of a converged pair**, since ack-pruning deletes everything the + peer has confirmed. So the healthy state was the one state that could not heal a wiped peer. With + an empty oplog the gap is now measured against `last_acked_seq`. Covered at both levels: + `SnapshotResyncTests.WipedPeer_AfterEverythingWasAckedAndPruned_StillGetsSnapshot` in the library, + and `LocalDbPairConvergenceTests.WipedNode_IsBackFilledByItsPeer_WithoutAnyNewDeploy` here — + the latter verified RED against the pinned 0.1.1 and green on 0.1.2. - **Oplog growth on default-OFF nodes** (check 8): `OnReady` registers replication unconditionally, so a node with no peer accumulates unacked/unpruned oplog rows (~2/deploy). Slow, bounded by the 1M cap; a periodic pruner or a skip-if-unchanged cache write would close it. + → **FIXED at the source**: `LocalDbDeploymentArtifactCache.StoreAsync` now **skips entirely** when + the pointer already names this deployment/revision, the SHA matches the bytes, and the expected + chunk count is present — so re-caching an artifact the node already holds (every restart's + boot-from-cache, every `RestoreApplied`) writes nothing and mints no oplog rows. Two guard tests + pin what the skip must NOT do (stale bytes under an unchanged revision; a matching pointer over + missing chunks), both verified RED against a naive pointer-only skip. + Also correcting this row's bound: the growth was never headed for the 1M cap. `AddZbLocalDbReplication` + is registered unconditionally, so `MaintenanceBackgroundService` runs on default-OFF nodes too and + the **7-day `MaxOplogAge`** cap prunes — the true bound was ~7 days of writes, not 1M rows. -Not merged, per the plan. The two limitations are recorded as Phase 1 follow-ups. +Merged to `master` as `c957db52` (2026-07-21) and pushed. Both follow-ups closed on +`fix/localdb-phase1-followups` (2026-07-21). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/LocalDbDeploymentArtifactCache.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/LocalDbDeploymentArtifactCache.cs index a29c08e3..8d66b3d0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/LocalDbDeploymentArtifactCache.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/LocalDbDeploymentArtifactCache.cs @@ -63,6 +63,14 @@ public sealed class LocalDbDeploymentArtifactCache : IDeploymentArtifactCache var cachedAtUtc = FormatTimestamp(DateTimeOffset.UtcNow); var chunkCount = (artifact.Length + ChunkSize - 1) / ChunkSize; + if (await IsAlreadyCachedAsync(clusterId, deploymentId, revisionHash, sha, chunkCount, ct)) + { + _logger.LogDebug( + "Deployment {DeploymentId} (revision {RevisionHash}) is already cached intact for cluster {ClusterId}; skipping the re-store.", + deploymentId, revisionHash, clusterId); + return; + } + // One transaction for the whole store. A pointer that commits without its chunks — or // chunks that commit without the pointer — is a cache that reads back as a corrupt hit // rather than a clean miss, which is the one outcome this type exists to prevent. @@ -155,6 +163,74 @@ public sealed class LocalDbDeploymentArtifactCache : IDeploymentArtifactCache await tx.CommitAsync(ct); } + /// + /// Whether this exact artifact is already cached and readable for the cluster, so + /// storing it again would rewrite every row to its current value. + /// + /// + /// + /// Why this exists. Every write to a replicated table fires a capture trigger and + /// mints an oplog row. Re-caching an artifact the node already holds — which is what a + /// restart's boot-from-cache and every RestoreApplied recovery does — would + /// therefore cost a delete plus an insert per chunk plus a pointer update, all carrying + /// values identical to what is already there. On a replicating pair that is pure churn; + /// on an unpaired (default-OFF) node there is no peer to ack those rows, so they + /// accumulate in the oplog until the library's age cap prunes them. The Phase 1 live gate + /// saw exactly that (check 8). + /// + /// + /// Identity is over the bytes, not the ids. The pointer must match on + /// artifact_sha256 as well as the deployment id and revision hash: a re-composed + /// artifact can legitimately carry the same ids with different bytes, and keeping the + /// stale copy would serve a boot-from-cache node the wrong address space while every + /// id-shaped signal claimed it was right. + /// + /// + /// The chunk count is load-bearing. A pointer can name a deployment whose chunks + /// are missing or partial — a half-replicated artifact, or a prune that raced the + /// pointer. Skipping on the pointer alone would make that state permanent, because the + /// re-store that repairs it is the call being skipped. + /// + /// + /// Accepted consequence: a skipped store leaves applied_at_utc at the + /// moment the artifact was first cached rather than last confirmed. Refreshing it + /// would mint the very oplog row this check exists to avoid. Nothing reads it as a + /// liveness signal; retention ranks distinct deployments, which are unaffected. + /// + /// + private async Task IsAlreadyCachedAsync( + string clusterId, string deploymentId, string revisionHash, string sha, int chunkCount, + CancellationToken ct) + { + var matches = await _db.QueryAsync( + """ + SELECT ( + SELECT COUNT(*) + FROM deployment_artifacts + WHERE cluster_id = @ClusterId + AND deployment_id = @DeploymentId + AND chunk_count = @ChunkCount + ) + FROM deployment_pointer + WHERE cluster_id = @ClusterId + AND deployment_id = @DeploymentId + AND revision_hash = @RevisionHash + AND artifact_sha256 = @Sha + """, + r => r.GetInt32(0), + new + { + ClusterId = clusterId, + DeploymentId = deploymentId, + RevisionHash = revisionHash, + Sha = sha, + ChunkCount = chunkCount, + }, + ct); + + return matches.Count == 1 && matches[0] == chunkCount; + } + /// public async Task GetCurrentAsync( string clusterId, CancellationToken ct = default) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairConvergenceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairConvergenceTests.cs index bfa317d3..42f92826 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairConvergenceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairConvergenceTests.cs @@ -103,6 +103,54 @@ public sealed class LocalDbPairConvergenceTests "node B to prune dep1 (with tombstones) while keeping dep3"); } + [Fact] + public async Task WipedNode_IsBackFilledByItsPeer_WithoutAnyNewDeploy() + { + // The Phase 1 live gate's check 4, as an offline scenario. A node whose LocalDb volume is + // lost used to come back permanently empty: the pair had already converged, so every oplog + // row was acked and pruned, and the healthy node had no delta left to send. Nothing short of + // a fresh deploy could repopulate it — which is the worst possible moment to depend on + // central being reachable, since the cache exists precisely for when it is not. + // + // Fixed in ZB.MOM.WW.LocalDb 0.1.2 (snapshot resync now measures the peer's gap against + // last_acked_seq when the oplog is empty). Pin the payoff at this level, not just the + // library's: what matters here is that the deployment artifact itself comes back. + await using var harness = new LocalDbPairHarness(); + await harness.StartAsync(); + + var deploymentId = Guid.NewGuid().ToString("N"); + var artifact = MultiChunkArtifact(seed: 20); + await harness.CacheA.StoreAsync(Cluster, deploymentId, "rev-1", artifact, + TestContext.Current.CancellationToken); + + await LocalDbPairHarness.WaitUntilAsync( + async () => await ReadArtifactAsync(harness.CacheB, Cluster) is { } b && b.AsSpan().SequenceEqual(artifact), + "the artifact to converge before the wipe"); + + // Convergence alone is not the precondition — an emptied oplog on A is. That is the state + // that used to make back-fill impossible, so assert it rather than assume it. + await LocalDbPairHarness.WaitUntilAsync( + async () => await LocalDbPairHarness.OplogDepthAsync(harness.A) == 0, + "node A's oplog to be fully acked and pruned"); + + await harness.WipePassiveAsync(); + Assert.Null(await ReadArtifactAsync(harness.CacheB, Cluster)); + + await harness.RestartPairAsync(); + + // No new deploy, no central: the rebuilt node gets its cached configuration back from its + // peer alone, byte-identical, and can boot from cache again. + await LocalDbPairHarness.WaitUntilAsync( + async () => + { + var cached = await harness.CacheB.GetCurrentAsync(Cluster, TestContext.Current.CancellationToken); + return cached is not null + && cached.DeploymentId == deploymentId + && cached.Artifact.AsSpan().SequenceEqual(artifact); + }, + "the wiped node to be back-filled from its peer"); + } + [Fact] public async Task WritesWhileTransportDown_SurviveRejoin() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairHarness.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairHarness.cs index d2815590..a221dee1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairHarness.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairHarness.cs @@ -69,10 +69,13 @@ public sealed class LocalDbPairHarness : IAsyncDisposable private readonly string _apiKeyB; private readonly string _pathA = Path.Combine(Path.GetTempPath(), $"otopcua-pairA-{Guid.NewGuid():N}.db"); - private readonly string _pathB = Path.Combine(Path.GetTempPath(), $"otopcua-pairB-{Guid.NewGuid():N}.db"); + + // Not readonly: WipePassiveAsync replaces node B's database wholesale, which is what a rebuilt + // node with a lost volume actually is — a new file, and with it a new node id. + private string _pathB = Path.Combine(Path.GetTempPath(), $"otopcua-pairB-{Guid.NewGuid():N}.db"); private readonly ServiceProvider _dbProviderA; - private readonly ServiceProvider _dbProviderB; + private ServiceProvider _dbProviderB; private IHost? _serverHost; // node B — passive private IHost? _initiatorHost; // node A — dials the peer @@ -125,6 +128,30 @@ public sealed class LocalDbPairHarness : IAsyncDisposable _serverHost = null; } + /// + /// Destroys node B's database and replaces it with an empty one, leaving the pairing config + /// untouched — the rebuilt-node case: a lost volume, a re-imaged host, a fresh container. + /// + /// + /// A new file, not a truncated one, because that is what the real thing is: the node id lives + /// in the database, so a rebuilt node comes back with a new identity and a zero watermark. The + /// caller follows with to bring the wiped node back online. + /// + public async Task WipePassiveAsync() + { + await StopPassiveAsync(); + await _dbProviderB.DisposeAsync(); + + // Pooled connections outlive the provider; without this the delete silently no-ops on + // Windows and the "wiped" node would still hold its rows. + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + DeleteDatabaseFiles(_pathB); + + _pathB = Path.Combine(Path.GetTempPath(), $"otopcua-pairB-{Guid.NewGuid():N}.db"); + _dbProviderB = BuildDatabaseProvider(_pathB); + _ = B; + } + /// /// Brings node B back on a NEW loopback port and re-dials from A. The initiator re-reads the /// peer address on each reconnect, so this is a genuine rejoin over the same databases. @@ -201,6 +228,17 @@ public sealed class LocalDbPairHarness : IAsyncDisposable return rows[0]; } + /// + /// Counts oplog rows on a node. Zero means every local change has been acked by the peer and + /// pruned — the steady state of a converged pair, and the state from which a wiped peer can + /// only be healed by a snapshot. + /// + public static async Task OplogDepthAsync(ILocalDb db) + { + var rows = await db.QueryAsync("SELECT COUNT(*) FROM __localdb_oplog", static r => r.GetInt64(0)); + return rows[0]; + } + /// Counts tombstone rows in __localdb_row_version for one table on a node. public static async Task TombstoneCountAsync(ILocalDb db, string table) { @@ -313,12 +351,16 @@ public sealed class LocalDbPairHarness : IAsyncDisposable await _dbProviderB.DisposeAsync(); Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); - foreach (var path in new[] { _pathA, _pathB }) + DeleteDatabaseFiles(_pathA); + DeleteDatabaseFiles(_pathB); + } + + /// Deletes a SQLite database and its WAL/shm sidecars, best effort. + private static void DeleteDatabaseFiles(string path) + { + foreach (var suffix in new[] { "", "-wal", "-shm" }) { - foreach (var suffix in new[] { "", "-wal", "-shm" }) - { - try { File.Delete(path + suffix); } catch { /* best effort */ } - } + try { File.Delete(path + suffix); } catch { /* best effort */ } } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Deployment/LocalDbDeploymentArtifactCacheTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Deployment/LocalDbDeploymentArtifactCacheTests.cs index 86bb8154..d34c5ccc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Deployment/LocalDbDeploymentArtifactCacheTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Deployment/LocalDbDeploymentArtifactCacheTests.cs @@ -90,6 +90,23 @@ public sealed class LocalDbDeploymentArtifactCacheTests : IDisposable "SELECT DISTINCT deployment_id FROM deployment_artifacts ORDER BY deployment_id", r => r.GetString(0)); + /// + /// Total capture-trigger rows in the replication oplog — the direct measure of how much + /// replication traffic (and, on an unpaired node, how much permanent oplog growth) a store + /// costs. + /// + private async Task OplogDepthAsync() + => (await _db.QueryAsync("SELECT COUNT(*) FROM __localdb_oplog", r => r.GetInt64(0)))[0]; + + /// + /// The LWW version stamps of every replicated row, as a stable ordered projection. Comparing + /// it across a call proves whether that call touched replicated state at all. + /// + private async Task> RowVersionsAsync() + => await _db.QueryAsync( + "SELECT table_name, pk_json, hlc FROM __localdb_row_version ORDER BY table_name, pk_json", + r => $"{r.GetString(0)}|{r.GetString(1)}|{r.GetInt64(2)}"); + [Fact] public async Task StoreThenGetCurrent_RoundTripsAMultiChunkArtifactByteForByte() { @@ -221,6 +238,77 @@ public sealed class LocalDbDeploymentArtifactCacheTests : IDisposable cached.Artifact.ShouldBe(second); } + [Fact] + public async Task Store_WritesNothingWhenTheCachedArtifactIsAlreadyIdentical() + { + // Phase 1 live gate, check 8: every store rewrote the chunks and the pointer unconditionally, + // so re-caching an artifact the node already holds — which is what boot-from-cache and + // RestoreApplied both do on every restart — minted fresh oplog rows for a row nobody changed. + // On a replicating pair that is pure churn; on a default-OFF node there is no peer to ack + // them, so they sit in the oplog until the age cap eventually prunes them. + var cache = CreateCache(); + var artifact = RandomArtifact(300 * 1024); + + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", artifact); + + var oplogDepth = await OplogDepthAsync(); + var rowVersions = await RowVersionsAsync(); + + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", artifact); + + (await OplogDepthAsync()).ShouldBe(oplogDepth); + (await RowVersionsAsync()).ShouldBe(rowVersions); + + // Skipping must not cost readability: the artifact still reads back intact. + var cached = await cache.GetCurrentAsync(ClusterA); + cached.ShouldNotBeNull(); + cached.DeploymentId.ShouldBe("dep-1"); + cached.Artifact.ShouldBe(artifact); + } + + [Fact] + public async Task Store_RewritesWhenTheCachedChunksAreIncompleteDespiteAMatchingPointer() + { + // The dangerous half of skip-if-unchanged. A pointer can name a deployment whose chunks are + // gone or partial — a half-replicated artifact, or a prune that raced the pointer. Trusting + // the pointer alone would turn a repairable cache into a permanently unreadable one, because + // the re-store that would have fixed it is exactly the call being skipped. + var cache = CreateCache(); + var artifact = RandomArtifact(300 * 1024); + + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", artifact); + await _db.ExecuteAsync( + "DELETE FROM deployment_artifacts WHERE deployment_id = @DeploymentId AND chunk_index = 2", + new { DeploymentId = "dep-1" }); + (await cache.GetCurrentAsync(ClusterA)).ShouldBeNull(); + + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", artifact); + + (await CountChunksAsync("dep-1")).ShouldBe(3); + var cached = await cache.GetCurrentAsync(ClusterA); + cached.ShouldNotBeNull(); + cached.Artifact.ShouldBe(artifact); + } + + [Fact] + public async Task Store_RewritesWhenTheArtifactBytesChangedUnderAnUnchangedRevision() + { + // The identity check must be over the bytes, not just the ids. A rebuilt artifact carrying + // the same deployment id and revision hash is a real case (the same revision re-composed), + // and silently keeping the stale bytes would serve a boot-from-cache node the wrong address + // space with every id-shaped signal saying it was correct. + var cache = CreateCache(); + + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", RandomArtifact(1024)); + + var rebuilt = RandomArtifact(1024); + await cache.StoreAsync(ClusterA, "dep-1", "rev-1", rebuilt); + + var cached = await cache.GetCurrentAsync(ClusterA); + cached.ShouldNotBeNull(); + cached.Artifact.ShouldBe(rebuilt); + } + [Fact] public async Task GetCurrentUnkeyed_ReturnsTheOnlyCachedArtifact() { From f587e7972eeb63de08bf4d74226f2a3def042950 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Jul 2026 02:14:00 -0400 Subject: [PATCH 2/3] docs(localdb): live-gate the two Phase 1 follow-ups; pin LocalDb 0.1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate closed both follow-ups on the docker-dev rig and found a third defect that offline tests could not have found first: it lived behind the one being fixed. Once 0.1.2 made back-fill work, a rebuilt node could be observed writing for the first time — and its writes went nowhere, because the healthy node kept the old peer's seq watermark. Library 0.1.1 -> 0.1.2 -> 0.1.3 over the course of the gate; both consumers now pinned to 0.1.3. Check 4 (was PARTIAL, now PASS): with SQL stopped and site-a-2's LocalDb volume destroyed, a-1 logged "Snapshot sent (as_of_seq 0, 4 rows)" — a line that could not appear on 0.1.1 — and a-2 came back with a row_version dump byte-identical to a-1's, carrying a-1's origin node ids and its ORIGINAL applied_at_utc. It then booted from cache and served 17 ns=2 nodes, diff-identical to its peer, from a configuration it never applied and could not have fetched. Check 8 (PASS): two restarts with an unchanged artifact left the oplog at 7 and the pointer timestamp frozen — the re-cache was skipped outright. Positive control: a real config change still wrote, 7 -> 10 -> 13. Also records one unrelated pre-existing finding the gate's SQL flapping surfaced: a transient ConfigDb error during artifact load empties the served address space while logging "rebuild becomes no-op". The cache layer correctly refused to store the bad artifact and the node recovered on restart, but the two logs disagree and it is the same class as the Phase 1 gate's check-3 defect. Untouched by this work; wants its own issue. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- Directory.Packages.props | 4 +- .../2026-07-20-localdb-pair-replication.md | 9 ++- .../2026-07-20-localdb-phase1-live-gate.md | 12 +++- .../2026-07-21-localdb-followups-live-gate.md | 68 +++++++++++++++++++ 4 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 docs/plans/2026-07-21-localdb-followups-live-gate.md diff --git a/Directory.Packages.props b/Directory.Packages.props index c569cf82..82221614 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -127,8 +127,8 @@ package floors the native SQLitePCLRaw.lib.e_sqlite3 at 2.1.12, so consumers inherit the CVE-2025-6965-fixed native binary without needing the surgical pin below. --> - - + + diff --git a/docs/operations/2026-07-20-localdb-pair-replication.md b/docs/operations/2026-07-20-localdb-pair-replication.md index c8d979cb..78422482 100644 --- a/docs/operations/2026-07-20-localdb-pair-replication.md +++ b/docs/operations/2026-07-20-localdb-pair-replication.md @@ -75,9 +75,12 @@ The replication library's passive endpoint verifies **no** authentication — th - **A rebuilt node is back-filled by its peer.** If a node loses its LocalDb file — a wiped volume, a re-imaged host, a fresh container — it rejoins with an empty database and its peer snapshots the cached configuration back to it, with no new deploy and no central SQL. Two things are worth - knowing: this needs **`ZB.MOM.WW.LocalDb` ≥ 0.1.2** (on `0.1.1` a converged pair has pruned every - oplog row on ack, and the wiped node was never snapshotted — it stayed empty until the next - deploy), and it heals the *cache*, so the node regains boot-from-cache for future outages. A node + knowing: this needs **`ZB.MOM.WW.LocalDb` ≥ 0.1.3**, and it heals the *cache*, so the node regains + boot-from-cache for future outages. (On `0.1.1` a converged pair has pruned every oplog row on ack + and the wiped node was never snapshotted — it stayed empty until the next deploy. On `0.1.2` it is + back-filled but its OWN writes are silently dropped until its restarted seq counter climbs past the + peer's stale watermark, so the pair looks converged right up until the moment only the rebuilt node + applies a deploy.) A node wiped **during** a central outage is still repopulated by its peer under 0.1.2; a node with **no** peer replication configured self-heals only from central on its next successful apply. diff --git a/docs/plans/2026-07-20-localdb-phase1-live-gate.md b/docs/plans/2026-07-20-localdb-phase1-live-gate.md index 2008438d..6bd03ec3 100644 --- a/docs/plans/2026-07-20-localdb-phase1-live-gate.md +++ b/docs/plans/2026-07-20-localdb-phase1-live-gate.md @@ -61,7 +61,8 @@ missed**, each now fixed on-branch with a regression test: - **No replication back-fill of a fully-wiped node** (check 4): a peer's already-acked cache rows are pruned from its oplog and snapshot-resync is gated on the (never-hit) oplog cap, so a wiped node is not healed by its peer — it self-heals from central on next boot instead. - → **FIXED in `ZB.MOM.WW.LocalDb` 0.1.2** (scadaproj `cad3bcb`). The diagnosis in this doc was + → **FIXED in `ZB.MOM.WW.LocalDb` 0.1.2 + 0.1.3** (scadaproj `cad3bcb` + `3b7489a`), live-gated in + `2026-07-21-localdb-followups-live-gate.md`. The diagnosis in this doc was slightly off: the cap was not the gate. `ComputeSnapshotRequiredAsync` measured the peer's gap against the *oldest surviving oplog row* and read an empty oplog as "no gap possible" — and an empty oplog is the **steady state of a converged pair**, since ack-pruning deletes everything the @@ -69,7 +70,11 @@ missed**, each now fixed on-branch with a regression test: an empty oplog the gap is now measured against `last_acked_seq`. Covered at both levels: `SnapshotResyncTests.WipedPeer_AfterEverythingWasAckedAndPruned_StillGetsSnapshot` in the library, and `LocalDbPairConvergenceTests.WipedNode_IsBackFilledByItsPeer_WithoutAnyNewDeploy` here — - the latter verified RED against the pinned 0.1.1 and green on 0.1.2. + the latter verified RED against the pinned 0.1.1 and green on 0.1.2. **0.1.3 closes a second, + deeper half that only became reachable once back-fill worked:** the rebuilt node's OWN writes were + silently dropped, because `last_applied_remote_seq` is a watermark in the *peer's* seq space and a + rebuilt peer numbers from 1 again. The follow-up live gate caught that one — offline tests could + not have, since the scenario had no test while it was still a documented limitation. - **Oplog growth on default-OFF nodes** (check 8): `OnReady` registers replication unconditionally, so a node with no peer accumulates unacked/unpruned oplog rows (~2/deploy). Slow, bounded by the 1M cap; a periodic pruner or a skip-if-unchanged cache write would close it. @@ -84,4 +89,5 @@ missed**, each now fixed on-branch with a regression test: the **7-day `MaxOplogAge`** cap prunes — the true bound was ~7 days of writes, not 1M rows. Merged to `master` as `c957db52` (2026-07-21) and pushed. Both follow-ups closed on -`fix/localdb-phase1-followups` (2026-07-21). +`fix/localdb-phase1-followups` (2026-07-21) and live-gated — see +`2026-07-21-localdb-followups-live-gate.md`, which found a third defect in the process. diff --git a/docs/plans/2026-07-21-localdb-followups-live-gate.md b/docs/plans/2026-07-21-localdb-followups-live-gate.md new file mode 100644 index 00000000..ae028c2e --- /dev/null +++ b/docs/plans/2026-07-21-localdb-followups-live-gate.md @@ -0,0 +1,68 @@ +# LocalDb Phase 1 follow-ups — live gate (docker-dev rig) + +> Closes the two limitations left open by `2026-07-20-localdb-phase1-live-gate.md` (checks 4 and 8). +> Rig: local `docker-dev/docker-compose.yml`, 6 nodes rebuilt from `fix/localdb-phase1-followups`. +> site-a pair replicates; central + site-b stay default-OFF. +> +> **Safety rules followed** (same as the Phase 1 gate): all DB inspection via `docker cp` of the +> `db`/`-wal`/`-shm` triplet, querying the copy — never host `sqlite3` on the live WAL file. + +## Result + +**Both follow-ups closed, and the gate found a third defect** — one that was *unreachable* until the +first fix landed. Library `ZB.MOM.WW.LocalDb` went `0.1.1` → `0.1.2` → `0.1.3` in the course of this +gate; both consumers (OtOpcUa, ScadaBridge) are pinned to `0.1.3`. + +| # | Check | Result | Evidence | +|---|---|---|---| +| 8 | Oplog stops growing on a default-OFF node | ✅ PASS | site-b-1 restarted twice with an unchanged cached artifact: `oplog_depth` **7 → 7 → 7**, `deployment_pointer.applied_at_utc` frozen at `05:34:58` across both — the re-cache was skipped outright. Repeated on the `0.1.3` build after a real deploy: **13 → 13**. Pre-fix this grew ~2 rows per restart forever, because a default-OFF node has no peer to ack and prune them. | +| 8b | …but a genuinely new deployment still writes (positive control) | ✅ PASS | A real config change (renamed tag `SITEA-tag`) produced rev `822bdf34`, then `002c16b3`. site-b-1's oplog **7 → 10 → 13** and its pointer advanced each time. The skip suppresses *identical* re-writes only; it is not a dead cache-write path. | +| 4 | A wiped node is back-filled by its peer, no new deploy, central down | ✅ PASS (was ⚠️ PARTIAL) | Precondition asserted first — site-a-1's oplog **0**, `needs_snapshot 0`: the exact state that used to make back-fill impossible. Then SQL stopped, site-a-2's LocalDb **volume destroyed**, container recreated. a-1 logged `Snapshot sent (as_of_seq 0, 4 rows)` — a line that could not appear on `0.1.1`. a-2 came back with pointer `SITE-A / 475df87a @ 03:09:12` (a-1's **original** timestamp, not a re-derived one), chunks 2, and a `__localdb_row_version` dump **byte-identical to a-1's**, carrying origin node ids `6812ca9c` and `935659a5` — the rebuilt node's own id appears nowhere, so the rows were replicated, not recreated. | +| 4b | …and the back-filled node then boots from cache | ✅ PASS | With SQL still down, site-a-2 logged `RUNNING FROM CACHE — … booted deployment 475df87a… cached at 2026-07-21T03:09:12`, `raw subtree materialised (containers=16, tags=1)`. OPC UA browse of both nodes: **17 ns=2 nodes each, diff-identical** — the rebuilt node serves exactly what its healthy peer does, from a configuration it never applied itself and could not have fetched. | +| 4c | The rebuilt node's OWN writes reach its peer | ✅ PASS **after a third fix** (see below) | On `0.1.2` this failed: a-1 held `last_applied_remote = 10` while the rebuilt a-2's oplog was seqs **1,2,3** and its `last_acked` stayed **0** — never accepted. On `0.1.3`, a-1 logs `Replication peer identity changed (ebb10854 -> c85d8b1d): the peer was rebuilt … Resetting the inbound watermark`, and after the next deploy a-1 shows `last_applied_remote = 3` / a-2 shows `last_acked = 3` with a drained oplog and identical row_versions. | + +## The third defect (the reason this gate was worth running) + +`0.1.2` made back-fill work, which for the first time let a rebuilt node be observed *writing*. It +turned out its writes went nowhere. + +`last_applied_remote_seq` is a watermark in the **peer's** seq space, and a rebuilt peer is a new +database whose oplog numbers from 1. Two independent bugs stacked, and fixing either alone leaves the +data stuck: + +1. The healthy node kept advertising the **old** peer's watermark. The *sending* side seeds its pump + directly from that number (`_sentThruSeq`), so the rebuilt node skipped its entire oplog. + → the watermark (and the observed peer clock) is now reset when the peer's node id changes. + `last_acked_seq` is deliberately **not** reset: it describes our own oplog's pruned horizon and is + exactly what `0.1.2` keys the snapshot decision off — clearing it would switch back-fill back off. +2. A peer claiming to have applied more of our stream than we have ever produced can only be + remembering a previous incarnation of *us*. → that claim is now rejected in favour of pumping from + the start; LWW makes the re-send harmless. This mirrors the clamp `RecordPeerAckAsync` already + applies to acks. + +Both were reproduced offline as a RED test before fixing +(`SnapshotResyncTests.RebuiltPeer_OwnWritesReachTheHealthyNode_DespiteTheOldPeersWatermark`). + +**Why offline tests could not have found it first:** the wiped-peer scenario had no test because the +scenario was believed impossible to recover from — it was a *documented limitation*, not a bug. The +gate is what turned the limitation into a reproducible case, and the second defect only existed +behind the first. + +## Unrelated finding (NOT a follow-up of this work — pre-existing, worth its own issue) + +While SQL was being stopped and started to drive check 4, site-b-1 took a deploy whose artifact load +hit a transient ConfigDb error, and **emptied its served address space**: + +``` +[06:11:20 ERR] An error occurred using the connection to database 'OtOpcUa' on server 'sql,1433'. +[06:11:20 WRN] OpcUaPublish: failed to load artifact for deployment 43d54397…; rebuild becomes no-op +[06:11:20 INF] AddressSpaceApplier: applied plan (kind=PureRemove, added=0, removed=16, rebuild=True) +``` + +The log says "rebuild becomes no-op" and the applier then removed all 16 nodes — those two disagree. +The cache layer behaved correctly and refused to store the bad artifact ("not caching deployment … its +artifact was empty or could not be loaded, so it is not a usable last-known-good configuration"), and +the node recovered fully on restart (`containers=16`). But a transient ConfigDb blip emptying the +served address space instead of holding last-known-good is the same failure *class* as the Phase 1 +gate's check-3 defect. Untouched by this work — site-b-1 has replication off entirely — and induced by +this gate's own SQL flapping. From 09df44067522e52b78dfb9c1e4615d9cf139f9e1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Jul 2026 02:17:32 -0400 Subject: [PATCH 3/3] docs(localdb): link the live gate's unrelated finding to issue #485 Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- docs/plans/2026-07-21-localdb-followups-live-gate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plans/2026-07-21-localdb-followups-live-gate.md b/docs/plans/2026-07-21-localdb-followups-live-gate.md index ae028c2e..d2c6ac54 100644 --- a/docs/plans/2026-07-21-localdb-followups-live-gate.md +++ b/docs/plans/2026-07-21-localdb-followups-live-gate.md @@ -48,7 +48,7 @@ scenario was believed impossible to recover from — it was a *documented limita gate is what turned the limitation into a reproducible case, and the second defect only existed behind the first. -## Unrelated finding (NOT a follow-up of this work — pre-existing, worth its own issue) +## Unrelated finding (NOT a follow-up of this work — pre-existing; filed as `lmxopcua` issue #485) While SQL was being stopped and started to drive check 4, site-b-1 took a deploy whose artifact load hit a transient ConfigDb error, and **emptied its served address space**: