diff --git a/docs/components/SiteRuntime.md b/docs/components/SiteRuntime.md index fd9dcdfa..3d846c1d 100644 --- a/docs/components/SiteRuntime.md +++ b/docs/components/SiteRuntime.md @@ -215,6 +215,8 @@ Both `AlarmActor` and `NativeAlarmActor` tell the `InstanceActor` an `AlarmState Config replication has no actor. `SiteReplicationActor` — which received `ReplicateConfigDeploy` / `ReplicateConfigRemove` / `ReplicateConfigSetEnabled` / `ReplicateArtifacts` / `ReplicateStoreAndForward` from the active node's `DeploymentManagerActor`, tracked the peer through cluster membership events, and forwarded each command to `/user/site-replication` via `ActorSelection` — was deleted in LocalDb Phase 2, together with its notify-and-fetch exchange (the standby was told a deploy had happened and then HTTP-fetched the config itself). The site's config tables are now replicated by LocalDb CDC, so a deploy on either node reaches the other as an ordinary row change. `SiteReconciliationActor` still fetches over HTTP at node startup when central reports gaps; that is a different path and it survives. +Since register row 67 that pass also heals the **runtime**, not only the file. It runs after `DeploymentManagerActor`'s boot-time SQLite load, so a config it stored used to start no instance actor — an enabled instance missing at boot needed a *second* restart to run. Each enabled instance it heals now gets a fire-and-forget `EnsureInstanceStartedCommand` sent through the deployment-manager singleton proxy; the handler creates the instance actor only if one is not already running, re-reads the stored row to honour `is_enabled`, and steps aside for any in-flight deploy. A start emits a Warning log line and a `deployment` site event so the heal is visible. + ## Usage ### Lifecycle commands diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 5f48a48a..ea305300 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -96,6 +96,8 @@ flowchart TD > **Startup config load is retried (S5)**: step 1's deployed-config read is best-effort against local SQLite, which can transiently fail (locked/busy). A failed load must **not** leave the site as a silent zero-instance node — it is re-attempted on a fixed interval until it succeeds, then the retry self-cancels. +> **Startup reconcile heals the RUNTIME, not just the config file (register row 67)**: `SiteReconciliationActor` runs on **every** site node (not a singleton) and performs one best-effort pass shortly after boot — it reports the node's local deployed inventory to central and fetches + stores any config central reports as a gap. That pass fires **after** step 1's deployed-config read has already run, so storing a config started nothing: Instance Actors are created only by the boot-time load above and by an explicit deploy/enable. An instance whose config was missing at boot (lost volume, first boot after a restore, rehomed data dir) therefore stayed **dead until a second restart**, whose load finally picked it up — silently, with nothing surfacing "config stored, actor not running". The pass now raises a fire-and-forget `EnsureInstanceStartedCommand` for every **enabled** instance it healed, routed through the **deployment-manager singleton proxy** so Instance Actors are only ever created under the singleton regardless of which node's pass found the gap. The handler is deliberately advisory: it **no-ops when the actor is already running** (both nodes legitimately raise it for the same instance), **re-reads the durable row** — which is the authority on both the config body and enabled/disabled state, so a disable landing after the fetch still wins and the instance stays stopped — **defers to any in-flight deploy/redeploy** for that instance, preserving the deploy gate's ordering guarantees and the all-or-nothing-per-instance rule (it never applies a config, only starts one already fully persisted), and treats a missing row as a quiet no-op (the normal case on an **unreplicated** site whose peer node healed its own file). A successful reconcile start is **surfaced**, not silent: a Warning log line plus an operational `deployment` site event, because reaching it means the node booted without a config for an instance that should have been running. + > **Per-instance compilation during staggered startup — CLOSED (WP3.1)**: this was a deferred optimization (P6) affecting failover time-to-recover; each Instance Actor Roslyn-compiled its own scripts inside its own start, serialising a site's whole recovery behind compilation. Each staggered startup batch now runs a **pre-warm step**: before the batch's Instance Actors are created, the Deployment Manager compiles that batch's distinct script bodies and trigger expressions on a background task and pipes `BatchCompileWarmed` back, so every `PreStart` compile is a cache hit. Instance Actor start still calls the compilation service — it remains the correctness backstop — but in every warmed path those calls are memoised lookups. Cost: one extra message per batch. > **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ. (It no longer clears wholesale on overflow: WP3.1 replaced that latency cliff with approximate-LRU batch eviction, see **Compile cache eviction** below.) Pinned by a reference-equality regression test rather than by watching memory. @@ -124,6 +126,7 @@ flowchart TD - **Disable**: Stops the Instance Actor and its children. Retains the deployed configuration in SQLite so the instance can be re-enabled without redeployment. - **Enable**: Creates a new Instance Actor from the stored configuration (same as startup). - **Delete**: Stops the Instance Actor and its children, removes the deployed configuration from local SQLite. Does **not** clear store-and-forward messages. +- **Ensure Started** (`EnsureInstanceStartedCommand`, site-internal — never sent by central): advisory, idempotent request to make sure an already-persisted, enabled instance is actually running. Raised by the startup reconcile self-heal; see the note under **Startup Behavior**. Unlike the three commands above it has **no reply** and applies no configuration — it only starts an Instance Actor for a config that is already fully stored, and no-ops when the actor is running, the stored row says disabled, the row is absent, or a deploy for that instance is in flight. ### Debug Snapshot Routing - Receives `DebugSnapshotRequest` from the Communication Layer and forwards to the Instance Actor by unique name (same lookup as `SubscribeDebugViewRequest`). diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs new file mode 100644 index 00000000..43973aa3 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs @@ -0,0 +1,29 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; + +/// +/// Site-internal request to make sure an already-persisted instance is actually RUNNING. +/// Routes via the deployment-manager singleton proxy so the Instance Actor is only ever +/// created under the singleton, whichever node raised the request. +/// +/// Emitted by SiteReconciliationActor after its node-STARTUP pass fetches and stores +/// a config the node was missing. The reconcile heal path writes the FILE; without this the +/// runtime was left untouched, so an instance whose config was absent at boot (lost volume, +/// first boot after a restore, rehomed data dir) stayed dead until the NEXT restart, whose +/// SQLite load finally started it. +/// +/// +/// Fire-and-forget and idempotent. There is no reply: the handler no-ops when the +/// instance is already running, when an in-flight deploy will create it anyway, or when the +/// durable row says the instance is disabled. The stored row — not this message — is the +/// authority on both the config body and enabled/disabled state, so a disable that landed +/// after the reconcile fetch still wins. +/// +/// +/// Unique name of the instance that should be running. +/// +/// Short human-readable provenance for the request (e.g. "startup reconcile"), carried +/// verbatim into the log line and the operational site event so the heal is not silent. +/// +public record EnsureInstanceStartedCommand( + string InstanceUniqueName, + string Reason); diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs index c6e7f9bc..4f4aee45 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs @@ -1024,6 +1024,11 @@ akka {{ // guarded-writes it (orphans are logged, never deleted). Requires the HTTP // config fetcher; if it is somehow absent the self-heal is skipped (best-effort — // replication remains the primary path and the next restart retries). + // The dmProxy is passed so the pass heals the RUNTIME too (register row 67): each + // enabled instance it restores gets an EnsureInstanceStartedCommand routed to the + // singleton, which starts the Instance Actor if the boot-time load missed it. Going + // through the PROXY (not a local ActorOf) keeps Instance Actors under the singleton + // regardless of which node's reconcile pass found the gap. if (deploymentConfigFetcher != null) { var reconcileLogger = _serviceProvider.GetRequiredService() @@ -1037,7 +1042,8 @@ akka {{ _nodeOptions.NodeName, reconcileLogger, null, - null)), + null, + dmProxy)), "site-reconciliation"); _logger.LogInformation( "SiteReconciliationActor created (per-node startup self-heal) for site {SiteId} node {Node}", diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index 638b06ab..eb6ba472 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -255,6 +255,13 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers Receive(HandleRefreshFetched); Receive(HandleRefreshFetchFailed); + // Reconcile self-heal: a config the node was missing at boot has just been + // fetched + stored by SiteReconciliationActor, which heals the FILE only. Start the + // Instance Actor too, so a missing-config boot no longer needs a SECOND restart to + // run. Idempotent and enabled-state-aware; the durable row is the authority. + Receive(HandleEnsureInstanceStarted); + Receive(HandleEnsureInstanceStartedResult); + // Query-the-site-before-redeploy idempotency. // Central asks for the instance's currently-applied deployment identity // before re-sending a deployment whose prior record is stuck InProgress @@ -1283,6 +1290,129 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers _logger.LogInformation("Instance {Instance} enabled", instanceName); } + /// + /// Reconcile self-heal (deferred register row 67): makes sure an instance whose config was + /// (re)stored out-of-band is actually RUNNING. + /// + /// + /// Why this exists. Instance Actors were created from exactly two paths — the + /// boot-time SQLite load () and an explicit + /// deploy/enable command. SiteReconciliationActor's node-STARTUP fetch runs AFTER + /// that boot load has already read storage, so a config it healed into the file started no + /// actor: the instance stayed dead until the next restart's load finally picked it up. + /// This handler closes that gap without touching the deploy pipeline. + /// + /// + /// + /// Guards. The command is advisory and fire-and-forget, so every reason to do + /// nothing is a silent return: the actor is already running (idempotent — both nodes' + /// reconcile actors legitimately raise this for the same instance), or an in-flight + /// deploy/redeploy owns the instance and will create the actor itself. Deferring to the + /// deploy paths is what keeps the deploy gate's ordering guarantees and the + /// all-or-nothing-per-instance rule intact — this never applies a config, it only starts + /// one that is already fully persisted. + /// + /// + private void HandleEnsureInstanceStarted(EnsureInstanceStartedCommand command) + { + var instanceName = command.InstanceUniqueName; + + if (!ShouldAttemptEnsureStart(instanceName)) + return; + + // Re-read the durable row off the actor thread. The stored row — NOT the requesting + // message — is the authority on the config body AND on enabled/disabled state, so a + // disable that landed after the reconcile fetch still wins and the instance stays + // stopped. + Task.Run(async () => + { + var configs = await _storage.GetAllDeployedConfigsAsync(); + var config = configs.FirstOrDefault(c => c.InstanceUniqueName == instanceName); + return new EnsureInstanceStartedResult(command, config, null); + }).ContinueWith(t => t.IsCompletedSuccessfully + ? t.Result + : new EnsureInstanceStartedResult(command, null, t.Exception?.GetBaseException().Message)) + .PipeTo(Self); + } + + /// + /// Shared guard for the two halves of the ensure-started round trip. Re-evaluated after + /// the storage read because the instance's state can change while that read is in flight + /// (a deploy or delete may have arrived on the mailbox in between). + /// + private bool ShouldAttemptEnsureStart(string instanceName) => + !_instanceActors.ContainsKey(instanceName) && + !_deployWarms.ContainsKey(instanceName) && + !_terminatingActorsByName.ContainsKey(instanceName) && + !_inFlightDeployments.ContainsKey(instanceName); + + /// + /// Actor-thread half of the reconcile self-heal: creates the Instance Actor for a config + /// that is present and enabled in storage but has no running actor, and SURFACES the heal + /// (log + operational site event) so "config stored, actor not running" is no longer + /// silent. + /// + private void HandleEnsureInstanceStartedResult(EnsureInstanceStartedResult result) + { + var instanceName = result.Command.InstanceUniqueName; + var reason = result.Command.Reason; + + if (result.Error != null) + { + _logger.LogWarning( + "Ensure-started ({Reason}) for {Instance} could not read the deployed config: {Error}", + reason, instanceName, result.Error); + return; + } + + // No stored row: nothing to start. Normal when the config was superseded/removed + // between the reconcile fetch and this read, and on an UNREPLICATED site whose peer + // node healed its own file — this singleton's storage legitimately has no such row. + if (result.Config == null) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: no deployed config on this node — nothing to start", + reason, instanceName); + return; + } + + // Disabled instances stay stopped. The heal restores the config row either way; a + // disabled instance having a row but no actor is the correct steady state. + if (!result.Config.IsEnabled) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: instance is disabled — leaving it stopped", + reason, instanceName); + return; + } + + // Re-check the guards: a deploy/delete for this instance may have arrived while the + // storage read was in flight, and it is authoritative over this advisory start. + if (!ShouldAttemptEnsureStart(instanceName)) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: already running or superseded by an in-flight deploy", + reason, instanceName); + return; + } + + CreateInstanceActor(instanceName, result.Config.ConfigJson); + _deployedInstanceNames.Add(instanceName); + UpdateInstanceCounts(); + + // Surface the heal. Warning (not Info) on purpose: reaching here means the node + // booted WITHOUT a config for an enabled, successfully-deployed instance — the + // condition is repaired, but it is abnormal and worth an operator's attention. + _logger.LogWarning( + "Instance {Instance} started by {Reason} self-heal — its config was missing at boot " + + "and has been restored; the instance was NOT running until now", + instanceName, reason); + LogDeploymentEvent("Warning", instanceName, + $"Instance {instanceName} started by {reason} self-heal", + "Config was missing at node boot and was restored by reconciliation; " + + "the Instance Actor was not running and has now been started."); + } + /// /// Deletes an instance: stops the actor and removes config from SQLite. /// Note: store-and-forward messages are NOT cleared per design decision. @@ -2419,6 +2549,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers internal record BatchState(List Configs, int NextIndex); internal record EnableResult( EnableInstanceCommand Command, DeployedInstance? Config, string? Error, IActorRef OriginalSender); + + /// + /// Carries the durable row read for an back onto + /// the actor thread. No original sender: the ensure-started path is fire-and-forget, so + /// there is nobody to answer. + /// + internal record EnsureInstanceStartedResult( + EnsureInstanceStartedCommand Command, DeployedInstance? Config, string? Error); internal record DeployPersistenceResult( string DeploymentId, string InstanceName, bool Success, string? Error, IActorRef OriginalSender, bool IsRedeploy); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs index 557569b8..be5827fe 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs @@ -14,7 +14,9 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// read the node's local deployed inventory from SQLite, /// report it to central via the SiteCommunicationActor Ask /// ( → ), -/// fetch each gap item's config over HTTP and guarded-write it, and +/// fetch each gap item's config over HTTP and guarded-write it, +/// ask the Deployment Manager singleton to START each enabled instance it just +/// healed, and /// LOG (never delete) any orphan the node still has but central no longer deploys. /// /// @@ -36,6 +38,17 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// The site does NOT carry the central fetch base URL in its own config — it uses /// from central's reply. /// +/// +/// The pass heals the runtime, not just the file (deferred register row 67). Storing a +/// config starts nothing by itself: the Deployment Manager creates Instance Actors from the +/// boot-time SQLite load, which has already run by the time this pass fires. So for every +/// ENABLED instance whose config this pass stored, an +/// is sent to the Deployment Manager singleton, which starts the actor if it is not already +/// running. Without it, an instance whose config was missing at boot (lost volume, first boot +/// after a restore, rehomed data dir) needed a SECOND restart before it ran again. +/// The command is advisory, idempotent, and deliberately fire-and-forget — a site with no +/// Deployment Manager ref configured simply keeps the old file-only behavior. +/// /// public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers { @@ -44,6 +57,7 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers private readonly SiteStorageService _storage; private readonly IDeploymentConfigFetcher _configFetcher; private readonly IActorRef _siteCommunicationActor; + private readonly IActorRef? _deploymentManager; private readonly string _siteIdentifier; private readonly string _nodeId; private readonly ILogger _logger; @@ -71,6 +85,14 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers /// Defaults to 5 seconds. /// /// Round-trip timeout for the reconcile Ask to central. Defaults to 30 seconds. + /// + /// The Deployment Manager singleton PROXY. Every enabled instance this pass heals is + /// followed by a fire-and-forget so the runtime + /// heals with the file (register row 67). Routing through the proxy — rather than starting + /// anything locally — is what keeps Instance Actors under the singleton no matter which + /// node's reconcile pass found the gap. Optional: when null the pass stores configs and + /// starts nothing, exactly as it did before. + /// public SiteReconciliationActor( SiteStorageService storage, IDeploymentConfigFetcher configFetcher, @@ -79,11 +101,13 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers string nodeId, ILogger logger, TimeSpan? initialDelay = null, - TimeSpan? askTimeout = null) + TimeSpan? askTimeout = null, + IActorRef? deploymentManager = null) { _storage = storage; _configFetcher = configFetcher; _siteCommunicationActor = siteCommunicationActor; + _deploymentManager = deploymentManager; _siteIdentifier = siteIdentifier; _nodeId = nodeId; _logger = logger; @@ -153,6 +177,9 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers var fetched = 0; var failed = 0; + // Enabled instances healed by this pass, to be started via the Deployment Manager once + // the pass reports back on the actor thread (row 67). + var healedEnabled = new List(); // Fetch + guarded-write each gap item. Per-item failure must not abort the rest. foreach (var item in response.Gap) @@ -168,6 +195,12 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers .ConfigureAwait(false); fetched++; + // Only ENABLED instances are candidates to start; a disabled one correctly + // keeps a config row with no running actor. The Deployment Manager re-checks + // this against the durable row anyway — this is just the cheap filter. + if (item.IsEnabled) + healedEnabled.Add(item.InstanceUniqueName); + _logger.LogInformation( "Reconcile: fetched + stored config for {Instance} (deployment {DeploymentId}, rev {Revision})", item.InstanceUniqueName, item.DeploymentId, item.RevisionHash); @@ -198,7 +231,7 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers name); } - return ReconcilePassResult.Completed(fetched, failed, response.OrphanNames.Count); + return ReconcilePassResult.Completed(fetched, failed, response.OrphanNames.Count, healedEnabled); } private void HandleReconcilePassResult(ReconcilePassResult result) @@ -216,6 +249,23 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers _logger.LogInformation( "Reconcile pass for site {Site} node {Node} complete: {Fetched} fetched, {Failed} failed, {Orphans} orphan(s)", _siteIdentifier, _nodeId, result.Fetched, result.Failed, result.Orphans); + + // Heal the RUNTIME, not just the file (row 67). Storing a config starts nothing: the + // Deployment Manager's boot-time SQLite load has already run by now, so without this + // an instance missing at boot would stay dead until the next restart. Fire-and-forget + // through the singleton proxy — the handler is idempotent, re-checks the durable row's + // enabled state, and defers to any in-flight deploy. + if (_deploymentManager == null || result.HealedEnabled.Count == 0) + return; + + foreach (var instanceName in result.HealedEnabled) + { + _logger.LogInformation( + "Reconcile: requesting start of healed instance {Instance} on site {Site}", + instanceName, _siteIdentifier); + _deploymentManager.Tell( + new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + } } // ── Internal messages ── @@ -227,21 +277,31 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers private RunReconcile() { } } - /// Summary of one reconcile pass, piped to Self for logging. - private sealed record ReconcilePassResult(int Fetched, int Failed, int Orphans, Exception? Error) + /// + /// Summary of one reconcile pass, piped to Self for logging and for the + /// ensure-started dispatch. is built off the actor thread but + /// only ever read on it, so no synchronization is needed. + /// + private sealed record ReconcilePassResult( + int Fetched, int Failed, int Orphans, IReadOnlyList HealedEnabled, Exception? Error) { /// Creates a result for a reconcile pass that ran to completion. /// Number of instances successfully fetched/reconciled. /// Number of instances that failed to reconcile. /// Number of local instances no longer deployed at central. + /// + /// Names of the ENABLED instances whose config this pass stored — each gets an + /// so the runtime heals with the file. + /// /// A completed with no error. - public static ReconcilePassResult Completed(int fetched, int failed, int orphans) - => new(fetched, failed, orphans, null); + public static ReconcilePassResult Completed( + int fetched, int failed, int orphans, IReadOnlyList healedEnabled) + => new(fetched, failed, orphans, healedEnabled, null); /// Creates a result for a reconcile pass that failed before completing. /// The exception that caused the pass to fail. /// A faulted carrying the error. public static ReconcilePassResult Faulted(Exception error) - => new(0, 0, 0, error); + => new(0, 0, 0, [], error); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs new file mode 100644 index 00000000..13788bd7 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs @@ -0,0 +1,384 @@ +using System.Collections.Concurrent; +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Lifecycle; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Deployment; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; +using ZB.MOM.WW.ScadaBridge.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// Regression tests for deferred register row 67 — startup reconcile healed the FILE but not +/// the RUNTIME. +/// +/// +/// Instance Actors are created from exactly two paths: the Deployment Manager's boot-time SQLite +/// load, and an explicit deploy/enable command. 's +/// node-STARTUP fetch runs AFTER that boot load has already read storage, so a config it healed +/// into the file started nothing — the enabled, successfully-deployed instance stayed dead until +/// the NEXT restart, whose load finally picked it up. Proven live on site-a 2026-08-15: +/// "Reconcile: fetched + stored config" with no "InstanceActor started" after it, and a debug +/// snapshot answering instanceNotFound. +/// +/// +/// +/// The fix is an raised by the reconcile pass and +/// handled by the Deployment Manager singleton. These tests pin both halves plus the seam +/// between them. +/// +/// +public class ReconcileStartsInstanceTests : TestKit, IDisposable +{ + private const string SiteIdentifier = "site-1"; + private const string NodeId = "node-a"; + + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly TestLocalDb _localDb; + + public ReconcileStartsInstanceTests() + { + _localDb = TestLocalDb.CreateTemp("reconcile-start-test"); + _storage = new SiteStorageService( + _localDb.Db, NullLogger.Instance); + _storage.InitializeAsync().GetAwaiter().GetResult(); + _compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedScriptLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + } + + void IDisposable.Dispose() + { + // Shut the actors down BEFORE the ILocalDb they write through is disposed. + Shutdown(); + var path = _localDb.Path; + _localDb.Dispose(); + TestLocalDb.DeleteFiles(path); + } + + private IActorRef CreateDeploymentManager(IServiceProvider? serviceProvider = null) => + ActorOf(Props.Create(() => new DeploymentManagerActor( + _storage, + _compilationService, + _sharedScriptLibrary, + null, + new SiteRuntimeOptions(), + NullLogger.Instance, + dclManager: null, + healthCollector: null, + serviceProvider: serviceProvider, + loggerFactory: null, + configFetcher: null))); + + private IActorRef CreateReconciliationActor( + IActorRef siteCommunicationActor, + IDeploymentConfigFetcher fetcher, + IActorRef? deploymentManager = null) => + ActorOf(Props.Create(() => new SiteReconciliationActor( + _storage, + fetcher, + siteCommunicationActor, + SiteIdentifier, + NodeId, + NullLogger.Instance, + TimeSpan.FromMilliseconds(50), // initial delay — fast for tests + TimeSpan.FromSeconds(5), // ask timeout + deploymentManager))); + + private static string MakeConfigJson(string instanceName) + { + var config = new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "TestAttr", Value = "42", DataType = "Int32" } + ] + }; + return JsonSerializer.Serialize(config); + } + + /// + /// Asks the Deployment Manager for a debug snapshot — the exact probe that answered + /// instanceNotFound on the live rig. InstanceNotFound == false means an + /// Instance Actor is running and routable. + /// + private async Task InstanceIsRunningAsync(IActorRef deploymentManager, string instanceName) + { + var snapshot = await deploymentManager.Ask( + new SubscribeDebugViewRequest(instanceName, Guid.NewGuid().ToString()), + TimeSpan.FromSeconds(5)); + return !snapshot.InstanceNotFound; + } + + // ── The reproduction ── + + /// + /// THE row-67 reproduction, end to end over one shared storage file: the node boots with no + /// config for an enabled instance (so the Deployment Manager's startup load starts nothing), + /// then the startup reconcile pass fetches and stores that config. The instance must be + /// RUNNING afterwards — not merely present in SQLite waiting for a second restart. + /// + /// + /// Fails before the fix at the final assert: the config row is stored, but the debug + /// snapshot still answers InstanceNotFound = true because nothing ever created the + /// Instance Actor. + /// + /// + [Fact] + public async Task StartupReconcile_StoringConfigForEnabledInstance_StartsTheInstanceActor() + { + // Boot the Deployment Manager against EMPTY storage — this is the missing-config boot + // (lost volume / first boot after a restore / rehomed data dir). + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + // Baseline: the instance is genuinely not running. + Assert.False(await InstanceIsRunningAsync(dm, "HealedPump")); + + // Now the startup reconcile pass runs and central reports the instance as a gap. + const string instanceName = "HealedPump"; + var configJson = MakeConfigJson(instanceName); + var fetcher = new FakeConfigFetcher(_ => Task.FromResult(configJson)); + var commProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dm); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem(instanceName, "dep-healed", "rev-healed", true, "tok-healed")], + [], + "http://central:9000")); + + // The file heals... + await AwaitAssertAsync(async () => + { + var configs = await _storage.GetAllDeployedConfigsAsync(); + Assert.Contains(configs, c => c.InstanceUniqueName == instanceName); + }, TimeSpan.FromSeconds(5)); + + // ...and so must the RUNTIME. This is the assertion that fails pre-fix. + await AwaitAssertAsync(async () => + Assert.True( + await InstanceIsRunningAsync(dm, instanceName), + "Reconcile stored the config but the InstanceActor was never started " + + "— the instance would stay dead until the next restart (register row 67)"), + TimeSpan.FromSeconds(10)); + } + + /// + /// The heal must not be silent: starting a reconcile-healed instance emits an operational + /// deployment site event naming the instance, so an operator can see that a node + /// booted without a config and self-repaired. + /// + [Fact] + public async Task ReconcileStartedInstance_EmitsASiteEvent() + { + var siteLog = new FakeSiteEventLogger(); + var dm = CreateDeploymentManager(new SingleServiceProvider(siteLog)); + await Task.Delay(500); + + const string instanceName = "SurfacedPump"; + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-s", "rev-s", true); + + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + + await AwaitAssertAsync(() => + { + Assert.Contains(siteLog.OfType("deployment"), r => + r.InstanceId == instanceName && + r.Source == "DeploymentManagerActor" && + r.Message.Contains("self-heal", StringComparison.OrdinalIgnoreCase)); + return Task.CompletedTask; + }, TimeSpan.FromSeconds(10)); + } + + // ── The reconcile half: which instances get a start request ── + + /// + /// An ENABLED instance healed by the pass produces an ensure-started request to the + /// Deployment Manager proxy. + /// + [Fact] + public void ReconcilePass_RequestsStart_ForHealedEnabledInstance() + { + var fetcher = new FakeConfigFetcher(_ => Task.FromResult("{\"instanceUniqueName\":\"B\"}")); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("B", "depB", "rev2", true, "tok-b")], + [], + "http://central:9000")); + + var ensure = dmProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal("B", ensure.InstanceUniqueName); + Assert.Contains("reconcile", ensure.Reason, StringComparison.OrdinalIgnoreCase); + } + + /// + /// A DISABLED instance healed by the pass must NOT be started — a disabled instance + /// legitimately has a config row and no running actor. + /// + [Fact] + public void ReconcilePass_DoesNotRequestStart_ForHealedDisabledInstance() + { + var fetcher = new FakeConfigFetcher(_ => Task.FromResult("{\"instanceUniqueName\":\"D\"}")); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("D", "depD", "revD", false, "tok-d")], + [], + "http://central:9000")); + + dmProbe.ExpectNoMsg(TimeSpan.FromSeconds(2)); + } + + /// + /// An item that failed to fetch was never stored, so it must not be started either. + /// + [Fact] + public void ReconcilePass_DoesNotRequestStart_ForFailedFetch() + { + var fetcher = new FakeConfigFetcher(_ => + Task.FromException(new DeploymentConfigFetchException("boom", isSuperseded: false))); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("Bad", "depBad", "revBad", true, "tok-bad")], + [], + "http://central:9000")); + + dmProbe.ExpectNoMsg(TimeSpan.FromSeconds(2)); + } + + // ── The Deployment Manager half: the ensure-started contract ── + + /// + /// The command is idempotent: both site nodes' reconcile actors legitimately raise it for + /// the same instance, and a repeat must not disturb (or replace) the running actor. + /// + [Fact] + public async Task EnsureInstanceStarted_IsIdempotent_WhenAlreadyRunning() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + const string instanceName = "IdempotentPump"; + dm.Tell(new DeployInstanceCommand( + "dep-idem", instanceName, "sha256:abc", + MakeConfigJson(instanceName), "admin", DateTimeOffset.UtcNow)); + ExpectMsg(TimeSpan.FromSeconds(5)); + await Task.Delay(1000); + + var before = await ResolveInstanceChildAsync(dm, instanceName); + + // Two redundant ensure-started requests, as the two nodes would send. + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + await Task.Delay(1000); + + // Same actor ref — nothing was restarted or replaced, and it is still routable. + var after = await ResolveInstanceChildAsync(dm, instanceName); + Assert.Equal(before, after); + Assert.True(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// The durable row is the authority on enabled/disabled state: a DISABLED instance stays + /// stopped even if an ensure-started request arrives for it (e.g. a disable landed between + /// the reconcile fetch and the start request). + /// + [Fact] + public async Task EnsureInstanceStarted_DisabledInstance_StaysStopped() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + const string instanceName = "DisabledPump"; + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-d", "rev-d", false); + + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + await Task.Delay(1500); + + Assert.False(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// An ensure-started request for an instance with no stored config on THIS node is a quiet + /// no-op — the normal case on an unreplicated site whose peer node healed its own file. + /// The Deployment Manager must not crash or invent an instance. + /// + [Fact] + public async Task EnsureInstanceStarted_UnknownInstance_IsAQuietNoOp() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + dm.Tell(new EnsureInstanceStartedCommand("NeverHeardOf", "startup reconcile")); + await Task.Delay(1500); + + Assert.False(await InstanceIsRunningAsync(dm, "NeverHeardOf")); + + // The manager is still alive and serving other work. + dm.Tell(new DeployInstanceCommand( + "dep-after", "AfterPump", "sha256:abc", + MakeConfigJson("AfterPump"), "admin", DateTimeOffset.UtcNow)); + Assert.Equal( + DeploymentStatus.Success, + ExpectMsg(TimeSpan.FromSeconds(5)).Status); + } + + /// + /// Resolves the Instance Actor child of the Deployment Manager by path — the direct + /// "the InstanceActor exists" check, complementing the debug-snapshot probe. + /// + private async Task ResolveInstanceChildAsync(IActorRef deploymentManager, string instanceName) => + await Sys.ActorSelection(deploymentManager.Path / instanceName) + .ResolveOne(TimeSpan.FromSeconds(5)); + + /// + /// In-test fake : runs a per-deploymentId behavior and + /// records every call thread-safely (the fetch runs on a pool thread). + /// + private sealed class FakeConfigFetcher : IDeploymentConfigFetcher + { + private readonly Func> _behavior; + public ConcurrentQueue<(string BaseUrl, string DeploymentId, string Token)> Calls { get; } = new(); + + public FakeConfigFetcher(Func> behavior) => _behavior = behavior; + + public async Task FetchAsync( + string centralFetchBaseUrl, string deploymentId, string token, CancellationToken ct) + { + Calls.Enqueue((centralFetchBaseUrl, deploymentId, token)); + await Task.Yield(); + return await _behavior(deploymentId); + } + } +}