perf(runtime): apply-path cache write off-mailbox; stash deferred dispatches

2026-08-15 17:12:01 -04:00
parent 875ec9fe47
commit 5f2e60278c
4 changed files with 458 additions and 31 deletions
@@ -54,7 +54,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
 /// <c>AttributeValuePublished</c> survives only as the message <see cref="FanCommsLossToDependencyMux"/>
 /// fabricates straight to the dependency mux; the host itself no longer receives it.
 /// </summary>
-public sealed class DriverHostActor : ReceiveActor, IWithTimers
+public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedStash
 {
    public const string DeploymentsTopic = "deployments";
    public const string DeploymentAcksTopic = "deployment-acks";
@@ -148,6 +148,20 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
    private readonly IActorRef? _scriptedAlarmHostOverride;
    private readonly ILoggingAdapter _log = Context.GetLogger();

+    /// <summary>
+    ///     Holds dispatches that arrive for a DIFFERENT deployment while one is being applied, until
+    ///     the apply transitions back to <c>Steady</c>. Assigned by the ActorCell after construction —
+    ///     it is <see langword="null"/> inside the constructor, so nothing there may touch it.
+    /// </summary>
+    /// <remarks>
+    ///     The predecessor was <c>Self.Forward(msg)</c>, which put the message back at the tail of this
+    ///     actor's OWN mailbox: with nothing else queued, the host spun dequeue → log → enqueue for the
+    ///     whole apply (a <c>FetchAndCache</c> fetch window is seconds of network I/O), burning a core
+    ///     and emitting a log line per iteration — measured at ~570k lines in 1.5 s. The stash holds the
+    ///     message OUTSIDE the mailbox, so the host simply idles until there is something real to do.
+    /// </remarks>
+    public IStash Stash { get; set; } = null!;
+
    /// <summary>The single VirtualTag-host child that spawns/reconciles Equipment-namespace
    /// VirtualTagActors and bridges their results onto the OPC UA publish actor. Spawned in
    /// <see cref="PreStart"/> when an OPC UA publish actor is wired; receives
@@ -567,6 +581,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
        _scriptedAlarmHostOverride = scriptedAlarmHostOverride;

        // Default behavior is Steady — PreStart may flip to Stale or replay an orphan apply.
+        // The ONE Become(Steady) that is not BecomeSteady(): Stash is assigned by the ActorCell after
+        // the constructor returns, so it is null here — and nothing can have been stashed yet anyway.
        Become(Steady);
    }

@@ -703,7 +719,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
        if (_dbFactory is null)
        {
            _log.Info("DriverHost {Node}: no ConfigDb on the Direct bootstrap path; entering Steady (no revision)", _localNode);
-            Become(Steady);
+            BecomeSteady();
            return;
        }

@@ -722,7 +738,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            if (latest is null)
            {
                _log.Info("DriverHost {Node}: no prior deployments; entering Steady (no revision)", _localNode);
-                Become(Steady);
+                BecomeSteady();
                return;
            }

@@ -738,7 +754,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                case NodeDeploymentStatus.Applied:
                    _currentRevision = revision;
                    _log.Info("DriverHost {Node}: recovered Applied state at rev {Rev}", _localNode, revision);
-                    Become(Steady);
+                    BecomeSteady();
                    // The revision is recovered but the in-memory driver children + OPC UA address
                    // space were lost on restart. Re-spawn + re-materialise + re-subscribe from the
                    // applied deployment so a restarted/rebuilt node restores its served state instead
@@ -749,13 +765,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                    _log.Warning("DriverHost {Node}: found orphan Applying row for deployment {Id}; replaying",
                        _localNode, latest.DeploymentId);
                    if (revision is not null) ApplyAndAck(new DeploymentId(latest.DeploymentId), revision.Value, CorrelationId.NewId());
-                    else Become(Steady);
+                    else BecomeSteady();
                    break;
                case NodeDeploymentStatus.Failed:
                default:
                    _log.Info("DriverHost {Node}: prior deployment {Id} failed; entering Steady at last known rev",
                        _localNode, latest.DeploymentId);
-                    Become(Steady);
+                    BecomeSteady();
                    break;
            }
        }
@@ -796,7 +812,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                    "DriverHost {Node}: FetchAndCache boot — the LocalDb cache is empty; entering Steady " +
                    "(no revision). The first dispatch will fetch this node's configuration from central.",
                    _localNode);
-                Become(Steady);
+                BecomeSteady();
                return;
            }

@@ -804,7 +820,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            var revision = RevisionHash.Parse(cached.RevisionHash);

            _currentRevision = revision;
-            Become(Steady);
+            BecomeSteady();
            // Restore drivers + address space + subscriptions from the cached bytes — no SQL read, no
            // re-ack (the deployment is already Applied). Same in-hand-bytes apply as the Direct-mode
            // cache boot, but this is the steady-state config source, not an outage fallback.
@@ -821,7 +837,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                "DriverHost {Node}: FetchAndCache boot — failed to read the LocalDb pointer; entering " +
                "Steady (no revision). The next dispatch will fetch.",
                _localNode);
-            Become(Steady);
+            BecomeSteady();
        }
    }

@@ -876,7 +892,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            // Stale would have started does not run here, so recovery rides on the next dispatch —
            // matching how a normally-booted node behaves.
            _currentRevision = revision;
-            Become(Steady);
+            BecomeSteady();

            ApplyCachedArtifact(deploymentId, cached.Artifact);

@@ -923,9 +939,36 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
        PushDesiredSubscriptionsFromArtifact(deploymentId, blob);
    }

+    /// <summary>
+    ///     Enters <see cref="Steady"/> and releases anything the <see cref="Applying"/> state stashed.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         A deferred dispatch must re-enter the mailbox <b>exactly once</b> and <b>in order</b> —
+    ///         <see cref="IStash.UnstashAll"/> prepends the stash to the front of the mailbox in stash
+    ///         order, which is both. Forgetting it on any one path silently strands that dispatch
+    ///         forever, and the coordinator only learns about it at its apply deadline.
+    ///     </para>
+    ///     <para>
+    ///         Every <c>Become(Steady)</c> goes through here — including the bootstrap and Stale-recovery
+    ///         paths, where the stash is provably empty (only <see cref="Applying"/> ever stashes, and
+    ///         those paths run before or outside it). Unstashing an empty stash is a no-op, and routing
+    ///         all of them through one helper is what makes "did we cover every transition?" answerable
+    ///         by grep. The constructor is the one exception: <see cref="Stash"/> is assigned by the
+    ///         ActorCell only after construction returns.
+    ///     </para>
+    /// </remarks>
+    private void BecomeSteady()
+    {
+        Become(Steady);
+        Stash.UnstashAll();
+    }
+
    private void Steady()
    {
        Receive<DispatchDeployment>(HandleDispatchFromSteady);
+        Receive<ArtifactCached>(HandleArtifactCached);
+        Receive<ArtifactCacheFailed>(HandleArtifactCacheFailed);
        Receive<GetDiagnostics>(HandleGetDiagnostics);
        Receive<DriverInstanceActor.AttributeValueBatchPublished>(ForwardBatchToMux);
        Receive<DriverInstanceActor.AttributeAlarmPublished>(ForwardNativeAlarm);
@@ -953,13 +996,22 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                    _localNode, msg.DeploymentId);
                return;
            }
-            _log.Info("DriverHost {Node}: dispatch for {Id} received while still applying {Cur}; deferring",
+
+            // Stashed, NOT re-forwarded to Self: a forward re-enqueues on this actor's own mailbox and
+            // is dequeued again immediately, spinning for the whole apply. Every transition back to
+            // Steady unstashes (see BecomeSteady), so this is logged once per deferred dispatch rather
+            // than once per spin iteration.
+            _log.Debug("DriverHost {Node}: dispatch for {Id} received while still applying {Cur}; deferring",
                _localNode, msg.DeploymentId, _applyingDeploymentId);
-            Self.Forward(msg); // re-deliver after we transition back
+            Stash.Stash();
        });
        // The FetchAndCache fetch pipes its result back here (BeginFetchAndCacheApply ran in Steady,
        // then Become(Applying)); completing the apply transitions back to Steady.
        Receive<FetchedForApply>(HandleFetchedForApply);
+        // The cache write is piped back too, and it outlives the apply that started it by design — so
+        // its completion can land in EITHER state and both must handle it or it dead-letters.
+        Receive<ArtifactCached>(HandleArtifactCached);
+        Receive<ArtifactCacheFailed>(HandleArtifactCacheFailed);
        Receive<GetDiagnostics>(HandleGetDiagnostics);
        Receive<DriverInstanceActor.AttributeValueBatchPublished>(ForwardBatchToMux);
        Receive<DriverInstanceActor.AttributeAlarmPublished>(ForwardNativeAlarm);
@@ -1614,7 +1666,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            SendAck(deploymentId, ApplyAckOutcome.Failed, reason, correlation);
            _log.Error("DriverHost {Node}: cannot apply {Id} — {Reason}", _localNode, deploymentId, reason);
            _applyingDeploymentId = null;
-            Become(Steady);
+            BecomeSteady();
            return;
        }

@@ -1706,7 +1758,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            parsed?.Dispose();
            OtOpcUaTelemetry.DeploymentApplyDurationSec.Record(sw.Elapsed.TotalSeconds);
            _applyingDeploymentId = null;
-            Become(Steady);
+            BecomeSteady();
        }
    }

@@ -1805,7 +1857,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            parsed?.Dispose();
            OtOpcUaTelemetry.DeploymentApplyDurationSec.Record(sw.Elapsed.TotalSeconds);
            _applyingDeploymentId = null;
-            Become(Steady);
+            BecomeSteady();
        }
    }

@@ -2008,10 +2060,31 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
    ///         space during the next outage — strictly worse than having no cache at all.
    ///     </para>
    ///     <para>
-    ///         Runs synchronously on the actor thread. That matches every other DB call on this path
-    ///         (all of which already block), and the alternative — piping the result back as a
-    ///         self-message — would need a handler registered in Steady, Applying AND Stale or it
-    ///         dead-letters after the <c>Become(Steady)</c> in the enclosing finally.
+    ///         <b>The store runs OFF the actor thread.</b> It is hundreds of chunk inserts into SQLite;
+    ///         blocking on it (as this did) parked the host for the duration, so it could not dequeue a
+    ///         driver value burst, a diagnostics request or the next dispatch. The task is piped back as
+    ///         <see cref="ArtifactCached"/>/<see cref="ArtifactCacheFailed"/> so completion is observed
+    ///         rather than fire-and-forgotten — the earlier objection to this shape was that the
+    ///         pipe-back needs a handler in every state it can land in, which is exactly why both
+    ///         <see cref="Steady"/> and <see cref="Applying"/> register one. (<c>Stale</c> does not: it
+    ///         is only ever entered from <see cref="Bootstrap"/>, before any apply has started a store.)
+    ///     </para>
+    ///     <para>
+    ///         <b>The apply does not depend on the outcome</b> — it never did, and the async shape keeps
+    ///         it that way. By the time this runs the deployment is recorded Applied and an Applied ACK
+    ///         has gone to the coordinator; a failure logs a Warning and changes nothing else.
+    ///     </para>
+    ///     <para>
+    ///         <b>Idempotence is unchanged.</b> The skip-if-already-cached check lives inside
+    ///         <c>LocalDbDeploymentArtifactCache.StoreAsync</c>, so it moves off-thread with the rest of
+    ///         the store rather than being lost.
+    ///     </para>
+    ///     <para>
+    ///         <b>No boot-path race.</b> The readers — <see cref="BootstrapFromCache"/> and
+    ///         <see cref="TryBootFromCache"/> — run in <see cref="PreStart"/>, before this actor has
+    ///         processed a single message and therefore before any apply could have started a store.
+    ///         The only concurrent writer a read can meet is the pair peer's replication, which the
+    ///         cache already handles.
    ///     </para>
    /// </remarks>
    /// <param name="deploymentId">The deployment whose artifact is being cached.</param>
@@ -2040,6 +2113,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            // The node's ClusterId is carried inside the artifact (ClusterNode rows), not in config
            // or on IClusterRoleInfo. A single-cluster or unscoped artifact has none, so it shares
            // one sentinel key — the pair still converges on a single pointer row either way.
+            //
+            // Resolved HERE, synchronously, and not one step later: `parsed` is owned by the calling
+            // apply and DISPOSED in its finally, which runs long before a slow store completes. The
+            // async continuation below must therefore capture only these scalars — never `parsed`.
            var scope = parsed is not null
                ? parsed.ResolveClusterScope(_localNode.Value)
                : DeploymentArtifact.ResolveClusterScope(blob, _localNode.Value);
@@ -2047,23 +2124,46 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                ? scope.ClusterId
                : SingleClusterCacheKey;

+            _log.Debug("DriverHost {Node}: caching deployment {Id} (rev {Rev}, cluster {ClusterId}, {Bytes} bytes)",
+                _localNode, deploymentId, revision, clusterId, blob.Length);
+
            _deploymentArtifactCache
                .StoreAsync(clusterId, deploymentId.ToString(), revision.Value, blob)
-                .GetAwaiter()
-                .GetResult();
-
-            _log.Debug("DriverHost {Node}: cached deployment {Id} (rev {Rev}, cluster {ClusterId}, {Bytes} bytes)",
-                _localNode, deploymentId, revision, clusterId, blob.Length);
+                .PipeTo(
+                    Self,
+                    success: () => new ArtifactCached(deploymentId),
+                    failure: ex => new ArtifactCacheFailed(deploymentId, ex.Message));
        }
        catch (Exception ex)
        {
-            _log.Error(ex,
-                "DriverHost {Node}: failed to cache applied deployment {Id}. The apply itself succeeded " +
-                "and is unaffected; this node simply has no local fallback for that revision.",
-                _localNode, deploymentId);
+            // A cache that throws SYNCHRONOUSLY (before returning a task) never produces a pipe-back,
+            // so route it to the same handler by hand rather than letting it unwind into the apply's
+            // catch — which would send a second, contradictory Failed ACK for a live deployment.
+            Self.Tell(new ArtifactCacheFailed(deploymentId, ex.Message));
        }
    }

+    /// <summary>The node-local artifact cache accepted this deployment's bytes.</summary>
+    /// <remarks>
+    ///     Piped back from <see cref="CacheAppliedArtifact"/>. Carries the id only — deliberately no
+    ///     <c>ParsedDeploymentArtifact</c> and no other apply-owned state, since the apply that started
+    ///     the store has long since disposed its parse by the time this lands.
+    /// </remarks>
+    private sealed record ArtifactCached(DeploymentId DeploymentId);
+
+    /// <summary>The node-local artifact cache could not store this deployment's bytes.</summary>
+    private sealed record ArtifactCacheFailed(DeploymentId DeploymentId, string Reason);
+
+    private void HandleArtifactCached(ArtifactCached msg) =>
+        _log.Info("DriverHost {Node}: cached applied deployment {Id} as this node's local fallback",
+            _localNode, msg.DeploymentId);
+
+    private void HandleArtifactCacheFailed(ArtifactCacheFailed msg) =>
+        _log.Warning(
+            "DriverHost {Node}: deployment {Id} could not be cached locally ({Reason}). The apply itself " +
+            "succeeded and is unaffected; this node simply has no local fallback for that revision.",
+            _localNode, msg.DeploymentId, msg.Reason);
+
    /// <summary>
    ///     Restore the served state for an already-applied deployment after a process restart.
    ///     <see cref="Bootstrap"/> recovers <see cref="_currentRevision"/> from NodeDeploymentState,
@@ -2600,7 +2700,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
        if (_fetchAndCacheMode || _dbFactory is null)
        {
            Timers.Cancel("retry-db");
-            Become(Steady);
+            BecomeSteady();
            return;
        }

@@ -2623,7 +2723,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
                UpsertNodeDeploymentState(new DeploymentId(latestSealed.DeploymentId),
                    NodeDeploymentStatus.Applied, failureReason: null);
            }
-            Become(Steady);
+            BecomeSteady();
        }
        catch (Exception ex)
        {