diff --git a/docs/deployment/topology-guide.md b/docs/deployment/topology-guide.md index 84343034..8e2a49dd 100644 --- a/docs/deployment/topology-guide.md +++ b/docs/deployment/topology-guide.md @@ -157,6 +157,26 @@ Each site has its own two-node cluster: - SQLite persistence: each node owns its own consolidated LocalDb database, kept in step by asynchronous CDC replication over a gRPC sync stream (LocalDb Phase 1 + 2). The nodes do NOT share a SQLite file. +- CDC capture triggers are installed **only on a node that has replication configured** — + `LocalDb:Replication:PeerAddress` *or* `LocalDb:Replication:ApiKey`. Either key counts, because + only the initiating half of a pair sets `PeerAddress` (one bidirectional stream, dialled by one + side); the passive half carries the key alone. A deliberately unreplicated node — site-b and + site-c on the rig — runs with no triggers at all and stops paying the per-write capture cost. + +#### Turning replication ON for a site that has been running without it + +Set the keys on **both** nodes and restart both. Two things to know before you do: + +- **Existing rows are not baselined.** Capture is change-data-capture: rows written while the node + had no triggers were never recorded in `__localdb_row_version`, and LocalDb's snapshot resync + streams from that ledger, so it will not ship them. The pair converges on everything written + *after* the restart and stays silently divergent on everything before it. Start from a copy of one + node's database on both sides, or accept that only new writes converge. +- **A node upgraded in place keeps stale triggers.** The guard decides whether triggers are + *installed*, not whether existing ones are removed, and the library has no removal API yet. A + database file first created by a build that always registered keeps capturing until that lands. + Recreating the node's data volume clears it — which is what a schema-change redeploy does on the + docker rig, so the rig is unaffected. ### Site Pair Upgrades — stop and start BOTH nodes together diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs index beb7f311..74698fa7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs @@ -21,6 +21,14 @@ namespace ZB.MOM.WW.ScadaBridge.Host; /// with no error anywhere. /// /// +/// Registration is conditional on the node actually having a replication peer — +/// see . An unreplicated node used to install the +/// full CDC trigger set anyway and then pay for it on every single write, forever, for an +/// oplog nothing ever drains. The ordering rule above is unaffected: when registration is +/// skipped there is no capture to be too late for, and the legacy migrator still runs +/// (its rows simply do not replicate, which is what "unreplicated node" means). +/// +/// /// Runs inside the AddZbLocalDb onReady callback, once, before any caller /// receives the singleton. onReady is a synchronous /// Action<ILocalDb>, hence the direct CreateConnection() rather than @@ -32,11 +40,14 @@ namespace ZB.MOM.WW.ScadaBridge.Host; public static class SiteLocalDbSetup { /// - /// Creates the site node's replicated tables, opts them into change capture, and - /// migrates any pre-Phase-1 databases in. + /// Creates the site node's tables, opts them into change capture when this node + /// replicates, and migrates any pre-Phase-1 databases in. /// /// The LocalDb instance being initialized. - /// Configuration, for the legacy migrator's old path keys and node name. + /// + /// Configuration, for the replication predicate, the legacy migrator's old path keys, + /// and the node name. + /// public static void OnReady(ILocalDb db, IConfiguration config) { ArgumentNullException.ThrowIfNull(db); @@ -53,39 +64,93 @@ public static class SiteLocalDbSetup StoreAndForwardSchema.Apply(connection); } - // Both tables qualify: each has an explicit primary key (RegisterReplicated - // rejects tables without one) and no BLOB columns (which json_object cannot - // capture). Registration is idempotent and installs the capture triggers. - db.RegisterReplicated("OperationTracking"); - db.RegisterReplicated("site_events"); + if (ReplicationIsConfigured(config)) + { + // Both tables qualify: each has an explicit primary key (RegisterReplicated + // rejects tables without one) and no BLOB columns (which json_object cannot + // capture). Registration is idempotent and installs the capture triggers. + db.RegisterReplicated("OperationTracking"); + db.RegisterReplicated("site_events"); - // Phase 2: the store-and-forward buffer and the seven site configuration tables. - // These replaced the bespoke SiteReplicationActor and StoreAndForward - // ReplicationService, which shipped hand-written Add/Remove/Park/Requeue operations - // over Akka; both were deleted in the same commit that added these lines, so the - // two mechanisms never ran at once. - // - // Both composite-PK tables are fine: RegisterReplicated orders multi-column PKs by - // ordinal. No Phase 2 table has a BLOB column, which it would reject. - db.RegisterReplicated("sf_messages"); - db.RegisterReplicated("deployed_configurations"); - db.RegisterReplicated("static_attribute_overrides"); - db.RegisterReplicated("shared_scripts"); - db.RegisterReplicated("external_systems"); - db.RegisterReplicated("database_connections"); - db.RegisterReplicated("data_connection_definitions"); - db.RegisterReplicated("native_alarm_state"); + // Phase 2: the store-and-forward buffer and the seven site configuration tables. + // These replaced the bespoke SiteReplicationActor and StoreAndForward + // ReplicationService, which shipped hand-written Add/Remove/Park/Requeue operations + // over Akka; both were deleted in the same commit that added these lines, so the + // two mechanisms never ran at once. + // + // Both composite-PK tables are fine: RegisterReplicated orders multi-column PKs by + // ordinal. No Phase 2 table has a BLOB column, which it would reject. + db.RegisterReplicated("sf_messages"); + db.RegisterReplicated("deployed_configurations"); + db.RegisterReplicated("static_attribute_overrides"); + db.RegisterReplicated("shared_scripts"); + db.RegisterReplicated("external_systems"); + db.RegisterReplicated("database_connections"); + db.RegisterReplicated("data_connection_definitions"); + db.RegisterReplicated("native_alarm_state"); - // notification_lists and smtp_configurations are created but deliberately NOT - // registered. They are permanently empty by design — the site-side write paths were - // removed on 2026-07-10, the legacy migrator skips them, and the active node's - // artifact apply purges them on every deploy. Registering them would open a standing - // replication channel whose only historical payload was plaintext SMTP passwords, in - // exchange for replicating nothing. Anyone adding them here should first establish - // that a site has a legitimate reason to hold SMTP credentials at all. + // notification_lists and smtp_configurations are created but deliberately NOT + // registered. They are permanently empty by design — the site-side write paths were + // removed on 2026-07-10, the legacy migrator skips them, and the active node's + // artifact apply purges them on every deploy. Registering them would open a standing + // replication channel whose only historical payload was plaintext SMTP passwords, in + // exchange for replicating nothing. Anyone adding them here should first establish + // that a site has a legitimate reason to hold SMTP credentials at all. + } // AFTER registration, so migrated rows enter the oplog and reach the peer like // any other write. Before it, they would be invisible to replication forever. + // Deliberately OUTSIDE the guard: an unreplicated node still has to absorb its + // pre-Phase-1 files, and there is no peer for its rows to be invisible to. SiteLocalDbLegacyMigrator.Migrate(db, config); } + + /// + /// Whether this node participates in LocalDb replication, and therefore needs the CDC + /// capture triggers at all. + /// + /// + /// + /// Either key counts, and that is not redundancy — it is the only predicate that is + /// true on BOTH halves of a replicated pair. Replication is one bidirectional + /// stream that exactly one side dials, so only the initiator sets + /// LocalDb:Replication:PeerAddress; the passive node has an ApiKey and + /// nothing else. (The rig is the reference: site-a node-a carries PeerAddress + ApiKey, + /// site-a node-b carries ApiKey alone, and site-b/site-c carry no Replication + /// section at all.) Keying on PeerAddress alone would strip capture from every passive + /// node — its local writes would stop reaching the initiator, and the pair would + /// silently converge in one direction only. + /// + /// + /// The keys are read straight from configuration rather than through + /// IOptions<ReplicationOptions> because this runs inside the + /// AddZbLocalDb factory, where resolving another option would nest a service + /// resolution inside a singleton construction. The section name matches what + /// AddZbLocalDbReplication binds, so the two cannot disagree about which node + /// replicates. + /// + /// + /// Known residual: a database file that was registered by an OLDER build keeps + /// its stale __localdb_* triggers — this only decides whether new ones are + /// installed, and the library has no removal API yet (it arrives with the WP3.3 library + /// work, which will also drop them on an unconfigured node). On the docker rig this is + /// moot: a schema change recreates the volumes. On a long-lived unreplicated node + /// upgraded in place, capture continues until that lands. + /// + /// + /// The other direction has a consequence too: turning replication ON for a site + /// that has been running without it does NOT baseline the rows already in the file. + /// Capture never recorded them in __localdb_row_version, and the snapshot resync + /// streams from that ledger, so the pair converges only on writes made after the + /// restart. Seed both nodes from one node's database if the existing rows matter. See + /// docs/deployment/topology-guide.md. + /// + /// + private static bool ReplicationIsConfigured(IConfiguration config) + { + var section = config.GetSection("LocalDb:Replication"); + + return !string.IsNullOrWhiteSpace(section["PeerAddress"]) + || !string.IsNullOrWhiteSpace(section["ApiKey"]); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbCdcRegistrationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbCdcRegistrationTests.cs new file mode 100644 index 00000000..52586a7a --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbCdcRegistrationTests.cs @@ -0,0 +1,227 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.LocalDb; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// WP1.3 — CDC capture is installed only on a node that actually replicates. +/// +/// +/// +/// These assert on the TRIGGERS in sqlite_master rather than on +/// ILocalDb.ReplicatedTables, because the trigger set is what costs anything: the +/// registry entry is a dictionary lookup, while each trigger runs two extra INSERTs plus a +/// json_object serialization of the full row inside every write transaction on the +/// table. An unreplicated node paid that on every store-and-forward enqueue, every static +/// override, every event log row, forever, for an oplog with no reader. +/// +/// +/// The naming __localdb_{table}_{ai|au|ad} is the library's +/// (TriggerSqlGenerator.TriggerName), matched by prefix here so a fourth trigger kind +/// would be caught rather than quietly ignored. +/// +/// +public class SiteLocalDbCdcRegistrationTests : IDisposable +{ + private readonly string _root; + private readonly List _providers = []; + + public SiteLocalDbCdcRegistrationTests() + { + _root = Path.Combine(Path.GetTempPath(), $"localdb-cdc-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_root); + } + + public void Dispose() + { + foreach (var provider in _providers) + { + try { provider.Dispose(); } catch { /* best effort */ } + } + + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + try { Directory.Delete(_root, recursive: true); } catch { /* best effort */ } + GC.SuppressFinalize(this); + } + + [Fact] + public void UnreplicatedNode_InstallsNoCaptureTriggers() + { + // No LocalDb:Replication section at all — site-b and site-c on the rig. + var db = BuildDatabase(Config()); + + Assert.Empty(CaptureTriggers(db)); + } + + [Fact] + public void UnreplicatedNode_StillCreatesEveryTable() + { + // Skipping registration must not skip the DDL: the node is fully functional + // standalone, it just has nobody to ship its changes to. A guard placed around the + // schema block instead of the registration block would fail here and nowhere else. + var db = BuildDatabase(Config()); + + var tables = TableNames(db); + + foreach (var table in AllSiteTables) + Assert.Contains(table, tables); + } + + [Fact] + public void InitiatorNode_InstallsCaptureTriggers() + { + // PeerAddress + ApiKey — site-a node-a, the half that dials. + var db = BuildDatabase(Config( + peerAddress: "http://peer:8083", apiKey: "cdc-test-key")); + + AssertCaptureTriggersCoverTheReplicatedTables(db); + } + + [Fact] + public void PassiveNode_WithApiKeyButNoPeerAddress_StillInstallsCaptureTriggers() + { + // The reason the predicate is an OR. Site-a node-b sets ApiKey and nothing else: + // one bidirectional stream, dialled by one side. Keying on PeerAddress alone would + // leave this node capturing nothing, so its own writes would never reach the + // initiator and the pair would converge in one direction only — silently. + var db = BuildDatabase(Config(apiKey: "cdc-test-key")); + + AssertCaptureTriggersCoverTheReplicatedTables(db); + } + + [Fact] + public void ReplicatedNode_InstallsNoCaptureTriggersOnTheCentralOnlyNotificationTables() + { + // The security property, restated at the trigger level: notification_lists and + // smtp_configurations exist but must never be captured, because the only payload + // they ever historically held was plaintext SMTP passwords. + var db = BuildDatabase(Config(apiKey: "cdc-test-key")); + + var triggers = CaptureTriggers(db); + + Assert.DoesNotContain(triggers, t => t.StartsWith("__localdb_notification_lists_", StringComparison.Ordinal)); + Assert.DoesNotContain(triggers, t => t.StartsWith("__localdb_smtp_configurations_", StringComparison.Ordinal)); + } + + [Fact] + public void UnreplicatedNode_StillRunsTheLegacyMigrator() + { + // The migrator is deliberately outside the guard. It renames the legacy file to + // ".migrated" on success, which is the observable proof it ran without needing a + // capture trigger to look at. + var legacyPath = Path.Combine(_root, "legacy-store-and-forward.db"); + SeedLegacyStoreAndForward(legacyPath); + + _ = BuildDatabase(Config(storeAndForwardPath: legacyPath)); + + Assert.False(File.Exists(legacyPath)); + Assert.True(File.Exists(legacyPath + ".migrated")); + } + + // ---- helpers ---------------------------------------------------------------------- + + /// The ten replicated tables plus the two deliberately-unregistered ones. + private static readonly string[] AllSiteTables = + [ + "OperationTracking", "site_events", "sf_messages", "deployed_configurations", + "static_attribute_overrides", "shared_scripts", "external_systems", + "database_connections", "data_connection_definitions", "native_alarm_state", + "notification_lists", "smtp_configurations", + ]; + + private static readonly string[] ReplicatedTables = + [ + "OperationTracking", "site_events", "sf_messages", "deployed_configurations", + "static_attribute_overrides", "shared_scripts", "external_systems", + "database_connections", "data_connection_definitions", "native_alarm_state", + ]; + + private static void AssertCaptureTriggersCoverTheReplicatedTables(ILocalDb db) + { + var triggers = CaptureTriggers(db); + + foreach (var table in ReplicatedTables) + { + // All three kinds, so a partial install is a failure rather than a pass. + foreach (var suffix in new[] { "ai", "au", "ad" }) + Assert.Contains($"__localdb_{table}_{suffix}", triggers); + } + } + + private ILocalDb BuildDatabase(IConfiguration config) + { + var provider = new ServiceCollection() + .AddZbLocalDb(config, db => SiteLocalDbSetup.OnReady(db, config)) + .BuildServiceProvider(); + _providers.Add(provider); + + return provider.GetRequiredService(); + } + + private IConfiguration Config( + string? peerAddress = null, string? apiKey = null, string? storeAndForwardPath = null) + { + var values = new Dictionary + { + ["LocalDb:Path"] = Path.Combine(_root, "consolidated.db"), + ["ScadaBridge:Node:NodeName"] = "node-a", + + // Every legacy path is pinned inside this test's own directory. Left unset they + // resolve CWD-relative (./data/…), and the migrator RENAMES whatever it finds — + // so an unlucky run could eat a real file from the test binary's output folder. + ["ScadaBridge:StoreAndForward:SqliteDbPath"] = + storeAndForwardPath ?? Path.Combine(_root, "absent-store-and-forward.db"), + ["ScadaBridge:Database:SiteDbPath"] = Path.Combine(_root, "absent-scadabridge.db"), + ["ScadaBridge:SiteEventLog:DatabasePath"] = Path.Combine(_root, "absent-events.db"), + ["ScadaBridge:OperationTracking:ConnectionString"] = + $"Data Source={Path.Combine(_root, "absent-tracking.db")}", + }; + + if (peerAddress is not null) values["LocalDb:Replication:PeerAddress"] = peerAddress; + if (apiKey is not null) values["LocalDb:Replication:ApiKey"] = apiKey; + + return new ConfigurationBuilder().AddInMemoryCollection(values).Build(); + } + + private static void SeedLegacyStoreAndForward(string path) + { + using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={path}"); + connection.Open(); + ZB.MOM.WW.ScadaBridge.StoreAndForward.StoreAndForwardSchema.Apply(connection); + } + + /// + /// Every LocalDb capture trigger in the file. Matched on the library's + /// __localdb_ prefix in C# rather than with SQL LIKE, where the underscores + /// are single-character wildcards and the pattern would need escaping to mean itself. + /// + private static HashSet CaptureTriggers(ILocalDb db) + { + using var connection = db.CreateConnection(); + using var cmd = connection.CreateCommand(); + cmd.CommandText = "SELECT name FROM sqlite_master WHERE type = 'trigger'"; + using var reader = cmd.ExecuteReader(); + + var names = new HashSet(StringComparer.Ordinal); + while (reader.Read()) + { + var name = reader.GetString(0); + if (name.StartsWith("__localdb_", StringComparison.Ordinal)) names.Add(name); + } + + return names; + } + + private static HashSet TableNames(ILocalDb db) + { + using var connection = db.CreateConnection(); + using var cmd = connection.CreateCommand(); + cmd.CommandText = "SELECT name FROM sqlite_master WHERE type = 'table'"; + using var reader = cmd.ExecuteReader(); + + var names = new HashSet(StringComparer.Ordinal); + while (reader.Read()) names.Add(reader.GetString(0)); + return names; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs index 4340a148..fc4a296c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs @@ -56,9 +56,16 @@ public class SiteLocalDbWiringTests : IDisposable ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", - // The consolidated site database. No Replication section at all — this - // fixture is also the default-OFF pin: storage must work standalone. + // The consolidated site database, configured exactly like the PASSIVE half of + // the rig's replicated pair (site-a node-b): an ApiKey and no PeerAddress. + // + // The key is what makes this a replicating node, and therefore what makes the + // CDC registration assertions below apply at all — capture is installed only + // when replication is configured. The absent PeerAddress keeps this fixture the + // default-OFF pin at the same time: nothing dials, so the engine must still + // resolve and idle rather than throw or report a connection. ["LocalDb:Path"] = _tempDbPath, + ["LocalDb:Replication:ApiKey"] = "wiring-test-localdb-sync-key", }); builder.Services.AddGrpc(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/LocalDbSitePairHarness.cs b/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/LocalDbSitePairHarness.cs index 0bc73e80..20bf2e48 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/LocalDbSitePairHarness.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/LocalDbSitePairHarness.cs @@ -113,6 +113,12 @@ public abstract class LocalDbSitePairHarness : IAsyncLifetime { ["LocalDb:Path"] = path, ["ScadaBridge:Node:NodeName"] = nodeName, + // OnReady installs the CDC capture triggers only on a node that has + // replication configured, so the key has to be here and not only in + // ReplicationConfig below — without it these nodes would run the sync + // engine over an oplog nothing ever writes to, and every convergence + // scenario would time out with two intact but unrelated databases. + ["LocalDb:Replication:ApiKey"] = SharedApiKey, // Point the legacy migrators at paths that do not exist, so they no-op rather // than picking up stray files from the test working directory. The two Phase 2 // defaults matter most: unlike the Phase 1 pair they resolve inside ./data/,