From cf69d03f9bf2cb973d9d229ed85fbddaf46a59fb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 13:17:27 -0400 Subject: [PATCH] perf(runtime): Direct-mode apply threads the in-hand artifact (1 SQL read per apply, not 3) --- .../Drivers/DriverHostActor.cs | 25 +++++++++-- .../Drivers/DriverHostActorTests.cs | 42 +++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index e56b1c31..79cd29a8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -1752,10 +1752,19 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // Trigger the OPC UA address-space rebuild so the local SDK reflects the new // composition. The publish actor handles the load-compose-diff-apply pipeline; we // just forward the same correlation id so the audit trail joins up. - _opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace(correlation, deploymentId)); + // + // The already-read bytes ride along. ReconcileDrivers just read this exact blob out of central + // SQL and handed it back (it is non-null here — the null case returned above), so a bytes-less + // RebuildAddressSpace would make the publish actor re-read the same immutable row, and the + // SubscribeBulk pass would read it a THIRD time: three round-trips per apply for one blob that + // cannot have changed between them. Threading it is also the shape the FetchAndCache apply has + // always had (HandleFetchedForApply), for the harder reason that it has no ConfigDb to re-read. + // Consequence: OpcUaPublishActor.LoadArtifact / LoadLatestArtifact are no longer on any apply + // path — they remain only for a legacy/dev caller that Tells RebuildAddressSpace without bytes. + _opcUaPublishActor?.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RebuildAddressSpace(correlation, deploymentId, appliedBlob)); // SubscribeBulk pass: hand each driver its desired tag references so live values flow into // the just-rebuilt address space instead of staying BadWaitingForInitialData. - PushDesiredSubscriptions(deploymentId); + PushDesiredSubscriptionsFromArtifact(deploymentId, appliedBlob); CacheAppliedArtifact(deploymentId, revision, appliedBlob); // Reaching here means the artifact came from central, so the node is no longer serving a // cache-sourced configuration. Note this only clears on a real apply: a dispatch of the @@ -1987,10 +1996,18 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// the OPC UA sink and resume after reconnects. Drivers with no configured tags get an empty set /// (which clears any stale subscription from a previous deployment). /// + /// + /// The bootstrap-restore entry point only. Neither apply path reaches it any more: both + /// and already hold the artifact bytes + /// and call directly, so this ConfigDb read exists + /// for , which restores from a deployment id recovered out of + /// NodeDeploymentState and may hold no blob of its own. + /// private void PushDesiredSubscriptions(DeploymentId deploymentId) { - // Phase 4 belt-and-suspenders: Direct-only read (the FetchAndCache path calls - // PushDesiredSubscriptionsFromArtifact with bytes in hand). No ConfigDb ⇒ nothing to load. + // Phase 4 belt-and-suspenders: a ConfigDb read, so it can only be the restore path on a node that + // has one (every apply path calls PushDesiredSubscriptionsFromArtifact with bytes in hand). + // No ConfigDb ⇒ nothing to load. if (_dbFactory is null) { _log.Warning("DriverHost {Node}: no ConfigDb to load artifact for SubscribeBulk ({Id}); skipping", _localNode, deploymentId); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs index 8f192617..a712a339 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs @@ -8,6 +8,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; using ZB.MOM.WW.OtOpcUa.Runtime.ScriptedAlarms; using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; @@ -199,6 +200,47 @@ public sealed class DriverHostActorTests : RuntimeActorTestBase plan.PredicateSource.ShouldContain("ctx.GetTag"); } + /// Verifies that a Direct-mode apply hands the artifact bytes it has already read to the + /// address-space rebuild instead of leaving the publish actor to re-read the same row from central SQL. + /// + /// The Direct apply reads Deployment.ArtifactBlob once (ReconcileDrivers returns it as + /// appliedBlob) and then used to Tell RebuildAddressSpace with NO bytes — so + /// OpcUaPublishActor.HandleRebuild re-read the row, and PushDesiredSubscriptions read it + /// a third time: three central-SQL reads of one immutable blob per apply. Carrying the in-hand bytes + /// collapses that to one. The FetchAndCache path has always been shaped this way, because it has no + /// ConfigDb to re-read from — this asserts Direct mode now matches it. Byte-equality (rather than + /// merely non-null) is the assertion that matters: a rebuild fed the WRONG artifact would materialise + /// a configuration nobody deployed. + /// + [Fact] + public void Direct_mode_apply_hands_the_already_read_artifact_to_the_rebuild() + { + var db = NewInMemoryDbFactory(); + var deploymentId = V3ArtifactSeeder.SeedV3Deployment(db, RevA, + rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) }, + unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) }); + + byte[] seeded; + using (var seedCtx = db.CreateDbContext()) + seeded = seedCtx.Deployments.Single(d => d.DeploymentId == deploymentId.Value).ArtifactBlob!; + seeded.Length.ShouldBeGreaterThan(0); + + var coordinator = CreateTestProbe(); + var publish = CreateTestProbe(); + var actor = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + opcUaPublishActor: publish.Ref)); + + actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId())); + coordinator.ExpectMsg(PresenceBudget).Outcome.ShouldBe(ApplyAckOutcome.Applied); + + var rebuild = publish.ExpectMsg(PresenceBudget); + rebuild.DeploymentId.ShouldBe(deploymentId); + rebuild.Artifact.ShouldNotBeNull(); + rebuild.Artifact.ShouldBe(seeded); + } + private static DeploymentId SeedDeploymentWithScriptedAlarm( IDbContextFactory db, RevisionHash rev) {