From c4fc1f8ecd894d09a539913ae052743fe582cd6f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 14 Aug 2026 22:36:15 -0400 Subject: [PATCH] perf(runtime): split trigger evals from the blocking pool; bounded, deadline-aware execution --- CLAUDE.md | 2 +- docs/requirements/Component-SiteRuntime.md | 73 +-- .../Messages/Health/SiteHealthReport.cs | 19 + .../ISiteHealthCollector.cs | 22 +- .../SiteHealthCollector.cs | 21 +- .../Actors/AlarmActor.cs | 242 ++++++-- .../Actors/AlarmExecutionActor.cs | 167 ----- .../Actors/DeploymentManagerActor.cs | 339 ++++++++++- .../Actors/InstanceActor.cs | 6 +- .../Actors/NativeAlarmActor.cs | 2 +- .../Actors/ScriptActor.cs | 296 ++++++--- .../Actors/ScriptExecutionActor.cs | 341 ----------- .../Scripts/ScriptExecutionScheduler.cs | 315 +++++++++- .../Scripts/ScriptRunLauncher.cs | 570 ++++++++++++++++++ .../Scripts/ScriptRuntimeContext.cs | 23 +- .../Scripts/ScriptSchedulerStatsReporter.cs | 4 +- .../Scripts/SiteScriptCompileCache.cs | 97 ++- .../Scripts/TriggerEvalGate.cs | 81 +++ .../SiteRuntimeOptions.cs | 53 +- .../SiteRuntimeOptionsValidator.cs | 23 +- .../ExecutionIdCorrelationTests.cs | 1 - .../ParentExecutionIdCorrelationTests.cs | 1 - .../IntegrationSurfaceTests.cs | 1 - .../Actors/AlarmActorTests.cs | 20 +- .../AlarmCascadeParentExecutionTests.cs | 1 - .../Actors/DeploymentWarmThenGateTests.cs | 230 +++++++ .../Actors/ExecutionActorTests.cs | 440 -------------- .../Actors/ScriptActorTests.cs | 37 +- .../Actors/ScriptDeadlineAtEnqueueTests.cs | 136 +++++ .../Actors/ScriptRunLauncherParityTests.cs | 469 ++++++++++++++ .../Actors/ScriptRunShedTests.cs | 199 ++++++ .../Actors/StuckScriptWatchdogTests.cs | 195 ++++++ .../Actors/TriggerEvalStarvationTests.cs | 196 ++++++ .../Scripts/AlarmsAccessorTests.cs | 1 - .../ExecutionCorrelationContextTests.cs | 1 - .../Scripts/ParentExecutionTreeTests.cs | 1 - .../Scripts/RecursionLimitSiteEventTests.cs | 1 - .../Scripts/ScopeAccessorTests.cs | 1 - .../Scripts/ScriptPoolSizingTests.cs | 103 ++++ .../Scripts/SiteScriptCompileCacheTests.cs | 86 ++- .../SiteRuntimeOptionsValidatorTests.cs | 60 ++ .../TestSupport/ScopeSpyServiceProvider.cs | 48 ++ 42 files changed, 3673 insertions(+), 1251 deletions(-) delete mode 100644 src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs delete mode 100644 src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs create mode 100644 src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs create mode 100644 src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerEvalGate.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs delete mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ExecutionActorTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunLauncherParityTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunShedTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/StuckScriptWatchdogTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/TriggerEvalStarvationTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptPoolSizingTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/TestSupport/ScopeSpyServiceProvider.cs diff --git a/CLAUDE.md b/CLAUDE.md index da8b3dd8..88b9ebff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie ### Architecture & Runtime - Instance modeled as Akka actor (Instance Actor) — single source of truth for runtime state. - Site Runtime actor hierarchy: Deployment Manager singleton → Instance Actors → Script Actors + Alarm Actors. -- Script Actors spawn short-lived Script Execution Actors on a dedicated blocking I/O dispatcher. +- Script/Alarm Actors launch script runs directly onto a dedicated blocking-I/O thread pool (`ScriptExecutionScheduler`) — the short-lived `ScriptExecutionActor`/`AlarmExecutionActor` children were **deleted** by WP3.1 (they had no `Receive` handler, no `PostStop`, and their `IActorRef` was never a message target; `ScriptRunLauncher` carries the run body verbatim). Same pool, same telemetry, same audit `ExecutionId`/`ParentExecutionId` threading, same "a stop does NOT cancel an in-flight run" semantics — minus a per-run actor cell. WP3.1 also: **trigger-expression evaluation left that pool** for the shared .NET thread pool behind `TriggerEvalGate` (arch-review finding #4 — blocked script bodies used to stall every Expression trigger on the node indefinitely); script and eval deadlines are armed **at enqueue** so queue wait burns the budget (a run dequeuing past its deadline is shed unrun); the pool is instance-scaled grow-only `clamp(max(ScriptExecutionThreadCount, ceil(instances/8)), 1, ScriptExecutionMaxThreadCount)`; the stuck-script watchdog now **detaches and replaces** a wedged worker (capped at pool size, gauge `DetachedScriptThreads`); `MaxConcurrentRunsPerScript` (4) sheds the newest run with a counter + rate-limited site event + an explicit error reply to Ask callers; `SiteScriptCompileCache` evicts an oldest-⅛ batch by approximate LRU instead of clearing wholesale; and deploy/startup compiles are warmed off-thread before the (ordering-critical, now cache-hit-only) synchronous deploy gate. Design memo: `docs/plans/2026-08-15-script-pool-split-design.md`. - Alarm Actors are separate peer subsystem from scripts (not inside Script Engine). - Shared scripts execute inline as compiled code (no separate actors). - Site-wide Akka stream for attribute value and alarm state changes with per-subscriber buffering. diff --git a/docs/requirements/Component-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 208ec16f..d9576841 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -41,9 +41,9 @@ flowchart TD AA2["Alarm Actor ('LowPressure')
— coordinator (computed)"] NAA1["Native Alarm Actor ('OpcUaServer1')
— read-only mirror, peer to Alarm Actor"] - SEA1["Script Execution Actor
— short-lived, per invocation"] - SEA2["Script Execution Actor
— short-lived, per invocation"] - AEA1["Alarm Execution Actor
— short-lived, per on-trigger invocation"] + SEA1["script run
— launched task, not an actor"] + SEA2["script run
— launched task, not an actor"] + AEA1["alarm on-trigger run
— launched task, not an actor"] IA2CHILD["… (Script / Alarm Actors)"] @@ -57,9 +57,9 @@ flowchart TD IA1 --> AA2 IA1 --> NAA1 - SA1 --> SEA1 - SA2 --> SEA2 - AA1 --> AEA1 + SA1 -.-> SEA1 + SA2 -.-> SEA2 + AA1 -.-> AEA1 IA2 -.-> IA2CHILD @@ -96,9 +96,11 @@ 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. -> **Per-instance compilation during staggered startup (P6, follow-up)**: the Roslyn compile of each instance's scripts still runs inside Instance Actor start during staggered startup; moving it off-thread is a deferred optimization (it affects failover time-to-recover only, not correctness). As of the round-2 hardening, a process-wide compile cache dedupes identical script bodies within a node's process lifetime (the deploy gate's compile is reused by Instance Actor start), shrinking the recompile cost; the *first* compile after process start still runs inside Instance Actor start, so the deferral stands. +> **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, and it clears wholesale on overflow, after which every script recompiles. Pinned by a reference-equality regression test rather than by watching memory. +> **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. + +> **Compile cache eviction is approximate LRU, not a wholesale clear (WP3.1)**: `SiteScriptCompileCache` is bounded at 1024 entries — deliberately smaller than the TemplateEngine verdict cache's 4096, because entries pin compiled assemblies rather than verdict strings. It used to `Clear()` **everything** on overflow. Instance scripts *and* trigger expressions share this cache, so on a site large enough to cross the bound every overflow discarded up to 1023 live compiled scripts and the next deploy or Instance-Actor start paid a full recompile storm **on actor threads**. Overflow now evicts only the oldest ⅛ (128 entries) by last-access stamp: one 1024-element scan instead of 1023 future recompiles, and hot entries survive. Recency is an `Interlocked` access sequence rather than a clock — deterministic for tests and immune to clock steps; hits update it lock-free, and only the (rare) eviction sweep takes a lock, double-checked so concurrent inserts do not stampede it. The sweep snapshots through `ConcurrentDictionary.ToArray()` and **not** LINQ over the dictionary: LINQ's `ToArray` picks the `ICollection.CopyTo` fast path, which throws when a concurrent insert lands between its Count read and its copy, and inserts are deliberately lock-free. > **Static options were only half the fix — the compile must also use the shared caching metadata resolver**: the static `ScriptOptions` carry only the *direct* API-surface references (5 assemblies here), and each `script.Compile()` still binds their **transitive closure** (~105 assemblies on a site node). Every transitively-referenced assembly is resolved through the options' `MetadataReferenceResolver`, and Roslyn's default one has no cross-compilation cache — it calls `MetadataReference.CreateFromFile` afresh on **every compile**, minting the same undisposed `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock` triple per assembly per compiled script, pinned for the process lifetime by the compile cache. That per-compile *re-resolution* — not the per-compile options construction — was the dominant term (measured live: 21 site scripts holding 6,640 metadata objects, counts bit-identical before and after the static-options fix). `ScriptCompilationService` therefore also attaches the process-wide `CachingScriptMetadataResolver.Instance` (owned by the Script Analysis component, see `Component-ScriptAnalysis.md`) via `ScriptOptions.WithMetadataResolver`, so each distinct assembly is materialized once per process. The decorator returns exactly what the undecorated resolver returned — only object identity is de-duplicated — so compile results and the trust gate are unchanged. @@ -179,10 +181,10 @@ The Instance Actor supervises all child Script and Alarm Actors with explicit st | Child Actor | Exception Type | Strategy | Rationale | |-------------|---------------|----------|-----------| -| Script Actor | Any exception | Resume | Script Actor is a coordinator — its state (trigger timers, last execution time) should survive child failures. Script Execution Actor failures are isolated. | +| Script Actor | Any exception | Resume | Script Actor is a coordinator — its state (trigger timers, last execution time) should survive run failures. | | Alarm Actor | Any exception | Resume | Alarm Actor holds alarm state. Resume preserves state and continues evaluation on next value update. | -| Script Execution Actor | Unhandled exception | Stop | Short-lived, per-invocation. Failure is logged; the Script Actor coordinator remains active for future triggers. | -| Alarm Execution Actor | Unhandled exception | Stop | Short-lived, per on-trigger invocation. Same as Script Execution Actor. | + +Script Actor and Alarm Actor have **no children of their own** since WP3.1 removed the per-run execution actors, so neither declares a supervision strategy any more. A run's own exceptions were always contained inside the run's try/catch (they never reached supervision); a failure of the *launch* is caught by the coordinator, which answers the caller and releases the run slot rather than letting the throw escalate to the Instance Actor. The Deployment Manager singleton supervises Instance Actors with a **OneForOneStrategy** — one Instance Actor's failure does not affect other instances. @@ -196,11 +198,11 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak - **Coordinator** for a single script definition on an instance. - Holds the compiled script code and trigger configuration. - Manages trigger evaluation (interval timer, value change detection, conditional evaluation). -- Spawns short-lived Script Execution Actors for each invocation. +- Launches script runs directly for each invocation (no per-run child actor since WP3.1). ### Trigger Management -- **Interval**: The Script Actor manages an internal timer. When the timer fires, it spawns a Script Execution Actor. -- **Value Change**: The Script Actor subscribes to attribute change notifications from its parent Instance Actor for the specific monitored attribute. When the attribute changes, it spawns a Script Execution Actor. +- **Interval**: The Script Actor manages an internal timer. When the timer fires, it launches a run. +- **Value Change**: The Script Actor subscribes to attribute change notifications from its parent Instance Actor for the specific monitored attribute. When the attribute changes, it launches a run. - **Conditional**: The Script Actor subscribes to attribute change notifications for the monitored attribute. On each update, it evaluates the condition (compares the attribute against a threshold). Firing depends on the **fire mode** (see below). - **Expression**: The Script Actor evaluates a compiled boolean expression against an attribute snapshot on each attribute change. Firing depends on the **fire mode** (see below). - **Fire mode (Conditional + Expression)**: @@ -211,20 +213,22 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak > **Per-attribute change routing (P2).** Rather than broadcasting every attribute change to all child Script/Alarm Actors, the Instance Actor routes each change only to the children that monitor that attribute (built from their trigger configs at spawn time); **Expression-trigger children, which read an attribute snapshot, receive all changes**. Each child still keeps its own trigger gate as defense-in-depth — the routing is an optimization, not the authority. ### Concurrent Execution -- Each invocation spawns a **new Script Execution Actor** as a child. -- Multiple Script Execution Actors can run concurrently (e.g., a trigger fires while a previous `Instance.CallScript` invocation is still running). -- The Script Actor coordinates but does not block on child completion. +- Each invocation launches a **new run** on the script-execution scheduler. +- Multiple runs can execute concurrently (e.g., a trigger fires while a previous `Instance.CallScript` invocation is still running), up to `MaxConcurrentRunsPerScript` — beyond which the newest is shed (see below). +- The Script Actor coordinates but does not block on run completion. -### Script Execution Actor -- **Short-lived** child actor created per invocation. -- Receives: compiled script code, input parameters, reference to the parent Instance Actor, current call depth. -- Executes the script in the Akka actor context. -- Has access to the full Script Runtime API (see below). -- Returns the script's return value (if defined) to the caller, then stops. -- The script body itself runs on the **dedicated `ScriptExecutionScheduler`** (a bounded set of dedicated threads), not the shared .NET thread pool, so blocking script I/O cannot starve the global pool or stall Akka dispatchers. The scheduler is **process-wide by default** (one pool per host, sized from `SiteRuntimeOptions.ScriptExecutionThreadCount`), but each script/alarm actor takes it through an **optional injection seam** rather than reaching for the static directly: the Host injects nothing and gets the shared pool, while tests (or a future multi-site host) can hand an actor its own instance. The shared accessor also recreates a disposed pool rather than returning it, so a disposed scheduler can never silently poison later executions. +### Script Run Launch (WP3.1 — replaces the Script Execution Actor) +- A run is launched **directly by the Script Actor** through `ScriptRunLauncher`. There is no per-run child actor. + - The former `ScriptExecutionActor` / `AlarmExecutionActor` were already inert shells: neither declared a `Receive` handler (they executed from their constructor), neither had a `PostStop`, state, or stash, and neither's `IActorRef` was ever a message target — the whole lifecycle lived inside a detached task the actor never observed. What they cost was an actor cell, mailbox, and name registration **per run**, plus a per-spawn expression-tree `Props.Create`. Removing them changed no semantics; the run body moved verbatim into the shared launcher. + - Everything the shells provided is preserved: exception and timeout containment, one DI scope per run disposed on every path, the site-event/health telemetry, the Ask reply, the completion notification (now the coordinator's own `Self`), and the audit `ExecutionId` / `ParentExecutionId` threading. A run still in flight when its Script Actor is stopped runs to completion and its completion message dead-letters, exactly as before — **stopping does NOT cancel in-flight runs**; redeploy/undeploy semantics are unchanged. + - One deliberate improvement: a failure of the *launch itself* (e.g. queueing onto a disposed scheduler) is caught by the Script Actor, which replies to the Ask caller and releases the run slot. The old per-run child's constructor throw was handled by a Stop supervision directive that sent no reply, leaving the caller to hang to its Ask timeout. +- The script body runs on the **dedicated `ScriptExecutionScheduler`** (a bounded set of dedicated threads), not the shared .NET thread pool, so blocking script I/O cannot starve the global pool or stall Akka dispatchers. The scheduler is **process-wide by default** (one pool per host), but each script/alarm actor takes it through an **optional injection seam** rather than reaching for the static directly: the Host injects nothing and gets the shared pool, while tests (or a future multi-site host) can hand an actor its own instance. The shared accessor also recreates a disposed pool rather than returning it, so a disposed scheduler can never silently poison later executions. +- **Pool sizing is instance-scaled and grow-only (WP3.1).** The pool was a fixed 8 threads regardless of load. It is now `clamp(max(ScriptExecutionThreadCount, ceil(enabledInstances / 8)), 1, ScriptExecutionMaxThreadCount)` — the existing `ScriptExecutionThreadCount` (default 8) becomes the **floor**, so configurations at or below 64 instances behave exactly as before, and the new `ScriptExecutionMaxThreadCount` (default 32) is the ceiling. Beyond the ceiling the per-script cap below is the real regulator. `DeploymentManagerActor.UpdateInstanceCounts` calls `EnsureCapacity` on every deploy / undeploy / enable / disable and once per staggered startup batch. Growth only: undeploying leaves idle threads, which cost nothing measurable and avoid drain/steal complexity. +- **The deadline is armed at enqueue, not at dequeue (WP3.1).** The run's timeout `CancellationTokenSource` is created on the actor thread *before* the body is queued, so queue wait consumes the script's own budget. A body that dequeues past its deadline **skips execution entirely** and takes the existing timeout path (site event, script-error counter, error reply, completion message): a saturated pool sheds stale work instead of running it late with a fresh full budget. +- **Concurrent runs per script are capped (WP3.1).** `MaxConcurrentRunsPerScript` (default 4) bounds runs in flight — queued or executing — for any one script or alarm on-trigger script. Over the cap the **newest** run is shed: the four already in flight are closest to their own deadlines and already charged against them, so nothing is ever reordered and no extra queue is needed (the scheduler's FIFO already is the queue). A shed increments `ISiteHealthCollector.IncrementScriptRunShed` (surfaced as `ScriptRunShedCount`), emits a `script`/`Warning` site event **rate-limited to one per script per minute** so a hot trigger cannot flood `site_events`, and — for an Ask-based `CallScript` — replies with an explicit error so a nested call or inbound-API route fails fast instead of hanging. ### Handling `Instance.CallScript` -- When an external caller (another Script Execution Actor, an Alarm Execution Actor, or a routed call from the Inbound API) sends a `CallScript` message to the Script Actor, it spawns a Script Execution Actor to handle the call. +- When an external caller (another script run, an alarm on-trigger run, or a routed call from the Inbound API) sends a `CallScript` message to the Script Actor, it launches a run to handle the call. - The caller uses the **Akka ask pattern** and receives the return value when the execution completes. --- @@ -244,7 +248,7 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak - **Range Violation**: Value is outside the allowed min/max range. - **Rate of Change**: Value change rate exceeds the defined threshold over a configurable time window. Direction filter (rising / falling / either) restricts which side of the rate triggers. - **HiLo**: Multi-setpoint level alarm with up to four configurable setpoints (LoLo, Lo, Hi, HiHi). Any subset may be configured. Each setpoint may carry its own priority that overrides the alarm-level priority for that band. -- **Expression** trigger evaluation runs on the bounded script-execution thread pool shared with script bodies (and Expression *script* triggers): scheduler saturation — e.g. stuck scripts occupying all threads — delays Expression alarm transitions site-wide until a thread frees. This is a deliberate trade-off (evaluation no longer blocks dispatcher threads; per-actor coalescing bounds queue growth) and the scheduler gauges + stuck-script watchdog make the saturated state visible. +- **Expression** trigger evaluation runs on the shared .NET thread pool behind a process-wide concurrency gate (`TriggerEvalGate`, sized by `TriggerEvalMaxConcurrency`, default `max(2, ProcessorCount)`) — **not** on the bounded script-execution pool. This is the WP3.1 fix for arch-review finding #4 (High): when evaluation shared that pool, N script bodies blocked in synchronous I/O stalled *every* Expression trigger on the node — scripts and alarms alike — for an unbounded time, and the evaluation's own 2 s timeout was constructed inside the queued body, so it did not start ticking until dequeue. An alarm that should have raised in milliseconds simply never raised, with neither a raise nor a timeout visible to the operator. Trigger expressions are non-blocking **by construction** (`TriggerExpressionGlobals` exposes only reads over an in-memory snapshot, and the script trust gate has already denied I/O, network, threading, and reflection), so the shared pool is where they belong; a second dedicated pool was considered and rejected as adding threads, gauges, and a second starvation surface for no isolation gain. The evaluation deadline (`TriggerEvalTimeoutSeconds`, default 2, previously hardcoded) is now armed **at enqueue**, so gate-wait time burns the same budget and a saturated gate yields a timely `false` rather than an unbounded stall. Per-actor coalescing (one evaluation in flight, one pending) is unchanged and caps waiters at one per Expression trigger, so the gate queue is bounded by trigger count. - For binary trigger types (ValueMatch / RangeViolation / RateOfChange), when the condition is met and the alarm is currently in **normal** state, the alarm transitions to **active**: - Updates the alarm state on the parent Instance Actor (which publishes to the Akka stream). - If an on-trigger script is defined, spawns an Alarm Execution Actor to execute it. @@ -256,9 +260,8 @@ When the Instance Actor is stopped (due to disable, delete, or redeployment), Ak - Held **in memory** only — not persisted to SQLite. State comprises `AlarmState` (Active / Normal) and `AlarmLevel` (None for binary triggers; the active band for HiLo). - On restart (or failover), alarm states are re-evaluated from incoming values. All alarms start in normal state with level None and transition based on incoming values. -### Alarm Execution Actor -- **Short-lived** child actor created when an on-trigger script needs to execute. -- Same pattern as Script Execution Actor — receives compiled code, executes, returns, and stops. +### Alarm On-Trigger Run +- Launched directly by the Alarm Actor when an on-trigger script needs to execute — the same `ScriptRunLauncher` path as an instance script run, with the firing alarm's name/level/priority/message exposed through the `Alarm` global. Bounded by the same `MaxConcurrentRunsPerScript` cap. - Has access to the Instance Actor for `GetAttribute`/`SetAttribute`. - **Can** call instance scripts via `Instance.CallScript()` — sends an ask message to the appropriate sibling Script Actor. - Instance scripts **cannot** call alarm on-trigger scripts — the call direction is one-way. @@ -275,7 +278,7 @@ change: |---|---| | `ScriptRuntimeContext.SetAttribute` (this run's `ExecutionId`) / `RouteToSetAttributesRequest.ParentExecutionId` (the inbound request's) | → `SetStaticAttributeCommand.SourceExecutionId` | | Instance Actor static-write path (`HandleSetStaticAttributeCore`) | → `AttributeValueChanged.SourceExecutionId` | -| Alarm Actor trigger evaluation → `SpawnAlarmExecution` | → `AlarmExecutionActor` → `ScriptRuntimeContext.ParentExecutionId` | +| Alarm Actor trigger evaluation → `SpawnAlarmExecution` | → `ScriptRunLauncher.LaunchAlarmScript` → `ScriptRuntimeContext.ParentExecutionId` | All four computed trigger types participate. `Expression` triggers evaluate a whole attribute snapshot **off the dispatcher**, so the firing change is no @@ -382,7 +385,7 @@ The enriched message flows Instance Actor → site-wide Akka stream → `SiteStr - Shared scripts are compiled at the site when received from central. - Compiled code is stored in memory and made available to all Script Actors. -- When a Script Execution Actor calls `Scripts.CallShared("scriptName", params)`, the shared script code executes **inline** in the Script Execution Actor's context — it is a direct method invocation, not an actor message. +- When a script run calls `Scripts.CallShared("scriptName", params)`, the shared script code executes **inline** in that run's context — it is a direct method invocation, not an actor message. - This avoids serialization bottlenecks since there is no shared script actor to contend for. - Shared scripts have access to the same runtime API as instance scripts (GetAttribute, SetAttribute, external systems, notifications, databases). - **Shared scripts always execute with Root scope (C5)**: `Attributes[...]` inside a shared script addresses **root** attributes even when the shared script is invoked from a composed-module script. Callers needing module-scoped access must pass the values in as parameters. @@ -417,14 +420,14 @@ All script types can be updated without restarting the cluster, but the mechanis ## Script Runtime API -Available to all Script Execution Actors and Alarm Execution Actors: +Available to all script runs and alarm on-trigger runs: ### Instance Attributes - `Instance.GetAttribute("name")` — Read an attribute value from the parent Instance Actor. - `Instance.SetAttribute("name", value)` — Write an attribute value. For data-connected attributes, writes to the DCL; for static attributes, updates in-memory and persists to local SQLite (survives restart/failover, reset on redeployment). ### Other Scripts -- `Instance.CallScript("scriptName", parameters)` — Send an ask message to a sibling Script Actor. The target Script Actor spawns a Script Execution Actor, executes, and returns the result. The call includes the current recursion depth. +- `Instance.CallScript("scriptName", parameters)` — Send an ask message to a sibling Script Actor. The target Script Actor launches a run, executes, and returns the result (or an explicit shed error if that script is already at its concurrent-run cap). The call includes the current recursion depth. - `Scripts.CallShared("scriptName", parameters)` — Execute shared script code inline (direct method invocation). The call includes the current recursion depth. ### External Systems @@ -474,7 +477,7 @@ Scripts execute **in-process** with constrained access. The following restrictio - **Allowed**: Access to the Script Runtime API (GetAttribute, SetAttribute, CallScript, CallShared, ExternalSystem, Notify, Database, Tracking, Alarms), standard C# language features, basic .NET types (collections, string manipulation, math, date/time). `System.Diagnostics.Stopwatch`, `Debug`, and `Activity` are permitted. - **Forbidden**: File system access (`System.IO`), process spawning (`System.Diagnostics.Process`), threading (`System.Threading` — except `Tasks`, `CancellationToken`, and `CancellationTokenSource`), reflection (`System.Reflection`), all raw network access (`System.Net` — must use `ExternalSystem.Call`), native interop (`System.Runtime.InteropServices`, `Microsoft.Win32`), assembly loading, unsafe code, `dynamic`, `Activator`. -- **Execution timeout**: Configurable per-script maximum execution time. Exceeding the timeout cancels the script **cooperatively** (S2/UA5): a script blocked in synchronous I/O or a tight CPU loop does not observe cancellation and continues to occupy its dedicated script-execution thread. A **watchdog logs the script by name** once its thread has not returned within a grace period after the timeout, and the script-execution scheduler's **queue depth / busy-thread / oldest-busy-age gauges** are reported through Health Monitoring so a saturated or stuck scheduler is visible. +- **Execution timeout**: Configurable per-script maximum execution time, **measured from enqueue** (WP3.1) — a run that spends its whole budget queued behind blocked scripts is shed at dequeue without executing, rather than being granted a fresh full budget for work whose triggering condition is already stale. Exceeding the timeout cancels the script **cooperatively** (S2/UA5): a script blocked in synchronous I/O or a tight CPU loop does not observe cancellation and continues to occupy its dedicated script-execution thread. A **watchdog names the script** once its thread has not returned within a grace period after the timeout — and, as of WP3.1, **detaches and replaces that worker thread**, so a wedged script costs the pool a thread only until the watchdog fires rather than for the rest of the process. The detached worker exits (instead of pulling more work) when its body finally returns, so capacity never silently doubles. Live detached workers are capped at the pool size — at the cap no replacement is started and an Error site event says so, because bounded starvation is preferable to unbounded thread growth when scripts wedge en masse. The scheduler's **queue depth / busy-thread / oldest-busy-age / detached-thread gauges** and the **per-interval shed count** are reported through Health Monitoring so a saturated, stuck, or shedding scheduler is visible. - **Memory**: Scripts share the host process memory. No per-script memory limit, but the execution timeout prevents runaway allocations. The forbidden-API policy is defined authoritatively in `ScriptTrustPolicy` (Script Analysis component, #25). `ScriptCompilationService.ValidateTrustModel` delegates to `ScriptTrustValidator.FindViolations` for the trust verdict; the Site Runtime also performs a `CSharpScript.Compile` against the real `ScriptGlobals` for execution. This is defence-in-depth static enforcement, not a true runtime sandbox. @@ -540,7 +543,7 @@ Per Akka.NET best practices, internal actor communication uses **Tell** (fire-an ### Script Compilation Errors - If script compilation fails when a deployment is received, the entire deployment for that instance is **rejected**. No partial state is applied. - The failure is reported back to central as a failed deployment. -- **Enforced by a site-side pre-compile validation gate (S3)**: before the Instance Actor is created or the config persisted, the Deployment Manager compiles **all** of the instance's scripts, alarm on-trigger scripts, and trigger expressions **synchronously, as a pure prefix step of the deploy handler on the singleton's actor thread** — deliberately not off-thread, because piping the verdict back to self would reorder concurrent deploys against delete/disable and break the redeploy-supersede mailbox-FIFO ordering (see the in-code rationale in `DeploymentManagerActor.HandleDeploy`); unchanged script bodies hit the process-wide compile cache, so the gate recompiles only genuinely new code. Any compile failure rejects the deployment with the collected compile errors. This makes the deployment result honest even if central's pre-deployment validation was bypassed or skew let a bad artifact through. +- **Enforced by a site-side pre-compile validation gate (S3)**: before the Instance Actor is created or the config persisted, the Deployment Manager compiles **all** of the instance's scripts, alarm on-trigger scripts, and trigger expressions **synchronously, as a pure prefix step of the deploy application on the singleton's actor thread** — the verdict is never derived off-thread, because piping a verdict back to self would reorder concurrent deploys against delete/disable and break the redeploy-supersede mailbox-FIFO ordering. WP3.1 made it **warm-then-gate**: the same compile is first run off-thread purely to populate the process-wide compile cache (`DeployCompileWarmed` pipes back), so the authoritative synchronous gate is then all cache hits and no longer holds the singleton for a Roslyn compile. Ordering is preserved by a **per-instance in-flight guard** — while a warm is in flight for instance X, further mutating commands for X (deploy / delete / disable / enable) are queued and re-dispatched in arrival order once the deploy applies; a second deploy with nothing queued behind it supersedes the pending one last-write-wins and its displaced deployer is answered `Failed`-superseded rather than left to Ask-timeout. Commands for **other** instances flow freely, which is the point, and is safe because cross-instance ordering was never guaranteed to callers. Unchanged script bodies hit the compile cache, so the gate recompiles only genuinely new code. Any compile failure rejects the deployment with the collected compile errors. This makes the deployment result honest even if central's pre-deployment validation was bypassed or skew let a bad artifact through. - Note: Pre-deployment validation at central should still catch compilation errors before they reach the site; the site gate is the authoritative backstop. --- diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs index 78d77ea0..ec43d5a1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs @@ -74,6 +74,25 @@ public record SiteHealthReport( /// public double? ScriptOldestBusyAgeSeconds { get; init; } + /// + /// WP3.1: script-execution worker threads the stuck-script watchdog has DETACHED and + /// replaced — a script wedged past its deadline plus grace in uninterruptible blocking + /// I/O, so the pool started a fresh thread and the wedged one will exit only when its + /// body finally returns. Point-in-time, refreshed by ScriptSchedulerStatsReporter. + /// Zero is the healthy state; a value that climbs and never drains means script bodies + /// are permanently consuming threads and the pool is being repeatedly rebuilt around them. + /// + public int DetachedScriptThreads { get; init; } + + /// + /// WP3.1: per-interval count of script and alarm on-trigger runs SHED because + /// MaxConcurrentRunsPerScript runs were already in flight for that script. Raw + /// per-interval count (drained on collect) like . A + /// sustained non-zero value means a trigger is firing faster than its script completes; + /// the shed itself is the designed back-pressure, not an error. + /// + public int ScriptRunShedCount { get; init; } + // LocalDb 2-node replication of the consolidated site database (Phase 1). // Additive init properties for the same reason as the scheduler gauges above: // the positional constructor stays untouched. Refreshed on the site by diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs index a1a19a4d..2495f6a4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs @@ -25,6 +25,20 @@ public interface ISiteHealthCollector /// void IncrementDeadLetter(); + /// + /// WP3.1: increments the per-interval count of script/alarm runs SHED because + /// MaxConcurrentRunsPerScript runs were already in flight for that script. A raw + /// per-interval count like the script/alarm error counters — a sustained non-zero value + /// means a trigger is firing faster than its script can complete. Every shed is counted + /// here even when its accompanying site event is rate-limited away. + /// Default interface implementation is a no-op so existing test fakes continue to + /// compile without per-fake updates. + /// + void IncrementScriptRunShed() + { + // Default no-op so test fakes do not need to be updated. + } + /// /// Increment the per-interval count of /// FallbackAuditWriter primary failures. Bridged from the @@ -173,7 +187,13 @@ public interface ISiteHealthCollector /// Script tasks waiting to run. /// Worker threads currently executing a script. /// Age (seconds) of the oldest in-flight script, or null when idle. - void SetScriptSchedulerStats(int queueDepth, int busyThreads, double? oldestBusyAgeSeconds) + /// + /// WP3.1: worker threads the stuck-script watchdog has detached and replaced because + /// their script wedged past its deadline plus grace, and which have not yet returned. + /// A non-zero, non-draining value means script bodies are permanently blocking threads. + /// + void SetScriptSchedulerStats( + int queueDepth, int busyThreads, double? oldestBusyAgeSeconds, int detachedThreads = 0) { // Default no-op so test fakes do not need to be updated. } diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs index 505a4bd1..e3341850 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs @@ -39,6 +39,12 @@ public class SiteHealthCollector : ISiteHealthCollector private int _scriptQueueDepth; private int _scriptBusyThreads; private long _scriptOldestBusyAgeBits = BitConverter.DoubleToInt64Bits(double.NaN); + // WP3.1: workers detached and replaced by the stuck-script watchdog and not yet exited + // (point-in-time, from the same reporter tick as the gauges above), and the per-interval + // count of runs shed by the per-script in-flight cap (reset on collect like the error + // counters, and restored by AddIntervalCounters when a report fails to send). + private int _scriptDetachedThreads; + private int _scriptRunShedCount; // WP2.6d: cumulative alarm-publish-queue drop count, refreshed by // SiteStreamAlarmDropReporter. Point-in-time (not reset on collect), like the // scheduler gauges above. @@ -162,12 +168,20 @@ public class SiteHealthCollector : ISiteHealthCollector } /// - public void SetScriptSchedulerStats(int queueDepth, int busyThreads, double? oldestBusyAgeSeconds) + public void SetScriptSchedulerStats( + int queueDepth, int busyThreads, double? oldestBusyAgeSeconds, int detachedThreads = 0) { Interlocked.Exchange(ref _scriptQueueDepth, queueDepth); Interlocked.Exchange(ref _scriptBusyThreads, busyThreads); Interlocked.Exchange(ref _scriptOldestBusyAgeBits, BitConverter.DoubleToInt64Bits(oldestBusyAgeSeconds ?? double.NaN)); + Interlocked.Exchange(ref _scriptDetachedThreads, detachedThreads); + } + + /// + public void IncrementScriptRunShed() + { + Interlocked.Increment(ref _scriptRunShedCount); } /// Reads the atomically-stored oldest-busy script age, mapping the NaN sentinel back to null. @@ -283,6 +297,11 @@ public class SiteHealthCollector : ISiteHealthCollector ScriptQueueDepth = Interlocked.CompareExchange(ref _scriptQueueDepth, 0, 0), ScriptBusyThreads = Interlocked.CompareExchange(ref _scriptBusyThreads, 0, 0), ScriptOldestBusyAgeSeconds = ReadScriptOldestBusyAgeSeconds(), + // WP3.1: point-in-time (like the three gauges above), so a CompareExchange read + // rather than an Exchange reset. + DetachedScriptThreads = Interlocked.CompareExchange(ref _scriptDetachedThreads, 0, 0), + // WP3.1: per-interval, so drained on collect like the error counters. + ScriptRunShedCount = Interlocked.Exchange(ref _scriptRunShedCount, 0), // Both fields come from the ONE snapshot read above. Null (the reporter has // not run) leaves both report fields null — "no data", not "disconnected // with an empty backlog". diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs index dc25baeb..e7ba30c2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmActor.cs @@ -25,10 +25,10 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// State (active/normal) is in memory only, NOT persisted. /// On restart: starts normal, re-evaluates from incoming values. /// -/// AlarmExecutionActor CAN call Instance.CallScript() (ask to sibling Script Actor). +/// An alarm on-trigger run CAN call Instance.CallScript() (ask to sibling Script Actor). /// Instance scripts CANNOT call alarm on-trigger scripts (no Instance.CallAlarmScript API). /// -/// Supervision: Resume on exception; AlarmExecutionActor stopped on exception. +/// Supervision: Resume on exception (from the Instance Actor). This actor has no children. /// public class AlarmActor : ReceiveActor { @@ -44,12 +44,39 @@ public class AlarmActor : ReceiveActor /// /// Script-execution scheduler seam (#18): the process-wide /// when null, or an injected instance so this - /// alarm's trigger-expression evaluation and spawned on-trigger scripts run on a - /// caller-owned pool. Resolved lazily at each use so the null (host) path is - /// unchanged. + /// alarm's launched on-trigger scripts run on a caller-owned pool. Resolved lazily at + /// each use so the null (host) path is unchanged. + /// + /// WP3.1: trigger-expression evaluation no longer uses this scheduler — see + /// . That split is the whole point of finding #4: an alarm whose + /// Expression trigger should raise in milliseconds must not queue behind blocking + /// script bodies. /// private readonly ScriptExecutionScheduler? _scheduler; + /// + /// WP3.1 (finding #4): the concurrency gate for trigger-expression evaluation, or null + /// for the process-wide . Evaluations run as plain + /// async work on the shared .NET thread pool behind this gate. + /// + private readonly TriggerEvalGate? _evalGate; + + /// + /// WP3.1: on-trigger runs launched but not yet completed. Incremented at launch, + /// decremented on , which every terminal path + /// emits — including the launch-path catch. Touched only on the actor thread. + /// + private int _runsInFlight; + + /// + /// WP3.1: when the last shed Warning site event was emitted for this alarm. Sheds are + /// always counted; the event is rate-limited to one per alarm per minute. + /// + private DateTimeOffset _lastShedEventUtc = DateTimeOffset.MinValue; + + /// Rate limit for the shed site event (the counter still counts every shed). + private static readonly TimeSpan ShedEventInterval = TimeSpan.FromMinutes(1); + /// /// The optional site operational-event log, resolved once from /// at construction and cached. The @@ -84,7 +111,7 @@ public class AlarmActor : ReceiveActor /// /// The on-trigger script's per-script execution timeout in seconds, /// or null to use the global default. Forwarded to each spawned - /// , which applies perScript ?? global + /// , which applies perScript ?? global /// (treating ≤ 0 as "use global"). The value comes from the referenced /// on-trigger script's . /// @@ -125,10 +152,10 @@ public class AlarmActor : ReceiveActor /// /// Audit Log #23 (ParentExecutionId tag-cascade): the /// parentExecutionId handed to the most recently spawned - /// — i.e. the execution whose attribute + /// on-trigger run — i.e. the execution whose attribute /// write fired this alarm, or null when the firing change came from /// the Data Connection Layer (external data has no spawning execution). - /// The spawned actor builds its own + /// The launched run builds its own /// internally, so this is exposed for regression coverage of the cascade /// contract (mirrors ). /// @@ -159,6 +186,7 @@ public class AlarmActor : ReceiveActor /// execution timeout in seconds (from its ), /// or null/non-positive to use the global default. /// Optional script-execution scheduler override (#18); null uses the process-wide shared scheduler. + /// Optional trigger-expression concurrency gate override (WP3.1); null uses the process-wide shared gate. public AlarmActor( string alarmName, string instanceName, @@ -175,7 +203,9 @@ public class AlarmActor : ReceiveActor // Per-script timeout for the on-trigger script (null = global). int? onTriggerExecutionTimeoutSeconds = null, // Script-execution scheduler seam (#18); null uses the process-wide shared scheduler. - ScriptExecutionScheduler? scheduler = null) + ScriptExecutionScheduler? scheduler = null, + // WP3.1 trigger-eval gate seam; null uses the process-wide shared gate. + TriggerEvalGate? evalGate = null) { _alarmName = alarmName; _instanceName = instanceName; @@ -186,6 +216,7 @@ public class AlarmActor : ReceiveActor _healthCollector = healthCollector; _serviceProvider = serviceProvider; _scheduler = scheduler; + _evalGate = evalGate; // Resolve the optional site event logger once and cache it, // rather than calling GetService on every alarm transition. _siteEventLogger = serviceProvider?.GetService(); @@ -217,9 +248,8 @@ public class AlarmActor : ReceiveActor // Handle attribute value changes Receive(HandleAttributeValueChanged); - // Handle alarm execution completion - Receive(_ => - _logger.LogDebug("Alarm {Alarm} execution completed on {Instance}", _alarmName, _instanceName)); + // Handle alarm execution completion (also releases the WP3.1 in-flight slot) + Receive(HandleAlarmExecutionCompleted); // Handle the off-dispatcher trigger-expression evaluation result (P1). Receive(HandleExpressionEvalResult); @@ -238,20 +268,10 @@ public class AlarmActor : ReceiveActor _alarmName, _instanceName, _triggerType); } - /// - protected override SupervisorStrategy SupervisorStrategy() - { - return new OneForOneStrategy( - maxNrOfRetries: -1, - withinTimeRange: TimeSpan.FromMinutes(1), - decider: Decider.From(ex => - { - _logger.LogWarning(ex, - "AlarmExecutionActor for {Alarm} on {Instance} failed, stopping", - _alarmName, _instanceName); - return Directive.Stop; - })); - } + // WP3.1: the Stop-on-failure SupervisorStrategy override is gone with the per-run + // AlarmExecutionActor child it supervised. This actor has no children left; a + // launch-path throw is caught in SpawnAlarmExecution so it can never escalate to + // InstanceActor, which continues to supervise this actor with Resume exactly as before. /// /// Evaluates alarm condition on attribute change. Alarm evaluation errors are logged, @@ -345,7 +365,7 @@ public class AlarmActor : ReceiveActor // Operational `alarm` event — raise. Severity by priority. LogAlarmEvent(RaiseSeverity(_priority), $"Alarm {_alarmName} activated (priority {_priority})"); - // Spawn AlarmExecutionActor if on-trigger script defined + // Launch the on-trigger run if an on-trigger script is defined if (_onTriggerCompiledScript != null) { SpawnAlarmExecution(AlarmLevel.None, _priority, string.Empty, sourceExecutionId); @@ -442,7 +462,7 @@ public class AlarmActor : ReceiveActor /// (resolved once at construction and cached /// in ). Never awaited so a logging failure /// cannot affect alarm evaluation (matching the established - /// ScriptActor/ScriptExecutionActor pattern). + /// ScriptActor / script-run pattern). /// private void LogAlarmEvent(string severity, string message) { @@ -589,31 +609,50 @@ public class AlarmActor : ReceiveActor // later change arriving while the evaluation is in flight cannot // mis-attribute the raise this evaluation produces. var sourceExecutionId = _latestSourceExecutionId; - Task.Factory.StartNew(async () => + var gate = _evalGate ?? TriggerEvalGate.Shared(_options); + // WP3.1 (finding #4): the deadline clock starts AT ENQUEUE, on the actor thread — + // gate-wait time burns the same budget, so a saturated gate yields a timely false + // instead of an unbounded stall. And the work runs on the shared .NET thread pool, + // NOT the blocking script pool, so it can never queue behind a blocked script body. + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(_options.TriggerEvalTimeoutSeconds)); + + Task.Run(async () => { try { - // Bound evaluation with a short timeout. The CancellationToken covers - // cooperative/async cases; a pathological CPU-bound expression is not - // fully interruptible — acceptable because trigger expressions are - // authored by trusted Design-role users and compile-checked pre-deploy. - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); - var state = await expression.RunAsync(new TriggerExpressionGlobals(snapshot), cancellationToken: cts.Token); - return state.ReturnValue is bool b && b; + await gate.WaitAsync(cts.Token).ConfigureAwait(false); + try + { + // The CancellationToken covers cooperative/async cases; a pathological + // CPU-bound expression is not fully interruptible — acceptable because + // trigger expressions are authored by trusted Design-role users and + // compile-checked pre-deploy. + var state = await expression.RunAsync( + new TriggerExpressionGlobals(snapshot), cancellationToken: cts.Token); + return state.ReturnValue is bool b && b; + } + finally + { + gate.Release(); + } } catch (Exception ex) { - // OperationCanceledException (timeout) falls through here too and is - // treated as false. _healthCollector (Interlocked) and _logger are - // thread-safe, so this catch is safe off the actor thread. + // OperationCanceledException (timeout, INCLUDING one that fired while still + // waiting on the gate) falls through here too and is treated as false. + // _healthCollector (Interlocked) and _logger are thread-safe, so this catch + // is safe off the actor thread. _healthCollector?.IncrementAlarmError(); _logger.LogError(ex, "Alarm {Alarm} trigger expression evaluation failed on {Instance}; treated as false", _alarmName, _instanceName); return false; } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, - _scheduler ?? ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, + finally + { + cts.Dispose(); + } + }).PipeTo(self, success: r => new ExpressionEvalResult(r, sourceExecutionId), failure: ex => new ExpressionEvalFailed(ex, sourceExecutionId)); } @@ -705,9 +744,34 @@ public class AlarmActor : ReceiveActor } /// - /// Spawns an AlarmExecutionActor to run the on-trigger script. + /// WP3.1: releases the in-flight slot for a completed on-trigger run. Every terminal + /// path emits exactly one (success / timeout / + /// failure / launch failure), so the counter tracks reality. + /// + private void HandleAlarmExecutionCompleted(AlarmExecutionCompleted msg) + { + if (_runsInFlight > 0) _runsInFlight--; + _logger.LogDebug( + "Alarm {Alarm} execution completed on {Instance}: success={Success}", + _alarmName, _instanceName, msg.Success); + } + + /// + /// WP3.1: on-trigger runs launched but not yet completed. Exposed for regression + /// coverage of the shed cap. + /// + internal int RunsInFlight => _runsInFlight; + + /// + /// Launches the on-trigger script run. /// Passes the firing alarm's level/priority/message so the script can /// branch on severity via the Alarm global. + /// + /// WP3.1: launched directly via rather than + /// through a short-lived AlarmExecutionActor child, and bounded by + /// — a raise arriving while + /// the cap is reached is shed (counted, rate-limited site event) rather than piling + /// another run onto a saturated pool. /// /// The firing alarm severity level. /// The firing alarm priority. @@ -727,34 +791,82 @@ public class AlarmActor : ReceiveActor { if (_onTriggerCompiledScript == null) return; - var executionId = $"{_alarmName}-alarm-exec-{_executionCounter++}"; + if (_runsInFlight >= _options.MaxConcurrentRunsPerScript) + { + ShedAlarmRun(); + return; + } + + var runId = _executionCounter++; // Record what the on-trigger run was parented to (null = root); read by - // the tag-cascade regression tests, which cannot see inside the child. + // the tag-cascade regression tests, which cannot observe the run directly. LastOnTriggerParentExecutionId = parentExecutionId; - // The on-trigger script body runs on the dedicated - // ScriptExecutionScheduler, not the shared .NET thread pool. - var props = Props.Create(() => new AlarmExecutionActor( - _alarmName, - _instanceName, - level, - priority, - message, - _onTriggerCompiledScript, - _instanceActor, - _sharedScriptLibrary, - _options, - _logger, - // Per-script timeout from the on-trigger script (null = global). - _onTriggerExecutionTimeoutSeconds, - // The firing execution's id — null for DCL-originated changes. - parentExecutionId, - // Scheduler seam (#18): share this alarm's scheduler override with the - // spawned on-trigger script body (null = process-wide shared). - _scheduler)); + // Incremented BEFORE the launch so the launch-path catch below (which always emits + // an AlarmExecutionCompleted) balances it on every path. + _runsInFlight++; - Context.ActorOf(props, executionId); + try + { + // The on-trigger script body runs on the dedicated + // ScriptExecutionScheduler, not the shared .NET thread pool. + ScriptRunLauncher.LaunchAlarmScript( + _alarmName, + _instanceName, + level, + priority, + message, + _onTriggerCompiledScript, + _instanceActor, + _sharedScriptLibrary, + _options, + // Completion target: this actor. Identical delivery to the old + // Context.Parent.Tell from the execution actor. + Self, + _logger, + runId, + // Per-script timeout from the on-trigger script (null = global). + _onTriggerExecutionTimeoutSeconds, + // The firing execution's id — null for DCL-originated changes. + parentExecutionId, + // Scheduler seam (#18): share this alarm's scheduler override with the + // launched on-trigger script body (null = process-wide shared). + _scheduler); + } + catch (Exception ex) + { + // WP3.1 supervision parity: mirrors the removed OneForOneStrategy's warn-and-stop + // for a per-run child that failed to construct. The alarm continues. + _logger.LogWarning(ex, + "Alarm on-trigger execution launch for {Alarm} on {Instance} failed, stopping", + _alarmName, _instanceName); + Self.Tell(new AlarmExecutionCompleted(_alarmName, false)); + } + } + + /// + /// WP3.1 shed policy for alarm on-trigger runs: refuses the newest run when + /// are already in flight. + /// There is no Ask caller on this path (an on-trigger run is never awaited), so the + /// shed surfaces as a counter plus a rate-limited Warning site event. + /// + private void ShedAlarmRun() + { + _healthCollector?.IncrementScriptRunShed(); + + var message = $"Alarm on-trigger script for '{_alarmName}' on instance '{_instanceName}': run shed — " + + $"{_runsInFlight} runs already in flight (cap {_options.MaxConcurrentRunsPerScript})."; + + _logger.LogWarning("{Message}", message); + + var now = DateTimeOffset.UtcNow; + if (now - _lastShedEventUtc >= ShedEventInterval) + { + _lastShedEventUtc = now; + _ = _siteEventLogger?.LogEventAsync( + "script", "Warning", _instanceName, $"AlarmActor:{_alarmName}", message); + } } private AlarmEvalConfig ParseEvalConfig(string? triggerConfigJson) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs deleted file mode 100644 index 0a298131..00000000 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/AlarmExecutionActor.cs +++ /dev/null @@ -1,167 +0,0 @@ -using Akka.Actor; -using Microsoft.CodeAnalysis.Scripting; -using Microsoft.Extensions.Logging; -using ZB.MOM.WW.ScadaBridge.Commons.Types; -using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; -using ZB.MOM.WW.ScadaBridge.Commons.Types.Scripts; -using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; - -namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; - -/// -/// Alarm Execution Actor -- short-lived child of Alarm Actor. -/// Same pattern as ScriptExecutionActor. -/// CAN call Instance.CallScript() (ask to sibling Script Actor). -/// Instance scripts CANNOT call alarm on-trigger scripts (no API for it). -/// Supervision: Stop on unhandled exception. -/// -public class AlarmExecutionActor : ReceiveActor -{ - /// Initializes a new and immediately schedules execution of the alarm on-trigger script. - /// The canonical name of the alarm that triggered. - /// The name of the owning instance. - /// The alarm severity level at the time of triggering. - /// The alarm priority value. - /// The alarm message to pass to the script. - /// The pre-compiled on-trigger script to execute. - /// Reference to the parent instance actor for attribute/script calls. - /// Shared script library providing common utilities. - /// Site runtime configuration options, including the execution timeout. - /// Logger for execution diagnostics. - /// The on-trigger script's per-script execution timeout in seconds. Null or non-positive falls back to the global . - /// - /// ParentExecutionId tag-cascade: the ExecutionId of - /// the execution whose attribute write fired this alarm, threaded into the - /// on-trigger script's as its - /// ParentExecutionId so the alarm-triggered run chains under its - /// firing execution. Null when the firing value came from the Data - /// Connection Layer (external data has no spawning execution) — that - /// on-trigger run is a tree root. - /// - public AlarmExecutionActor( - string alarmName, - string instanceName, - AlarmLevel level, - int priority, - string message, - Script compiledScript, - IActorRef instanceActor, - SharedScriptLibrary sharedScriptLibrary, - SiteRuntimeOptions options, - ILogger logger, - // Per-script execution timeout override (seconds) for the - // alarm on-trigger script. Null or non-positive falls back to the global. - int? executionTimeoutSeconds = null, - // The firing context's execution id (null today). - Guid? parentExecutionId = null, - // Script-execution scheduler seam (#18): the process-wide scheduler by - // default; null selects the shared default. - ScriptExecutionScheduler? scheduler = null) - { - var self = Self; - var parent = Context.Parent; - - ExecuteAlarmScript( - alarmName, instanceName, level, priority, message, - compiledScript, instanceActor, - sharedScriptLibrary, options, self, parent, logger, - executionTimeoutSeconds, parentExecutionId, scheduler); - } - - private static void ExecuteAlarmScript( - string alarmName, - string instanceName, - AlarmLevel level, - int priority, - string message, - Script compiledScript, - IActorRef instanceActor, - SharedScriptLibrary sharedScriptLibrary, - SiteRuntimeOptions options, - IActorRef self, - IActorRef parent, - ILogger logger, - int? executionTimeoutSeconds, - Guid? parentExecutionId, - ScriptExecutionScheduler? scheduler) - { - // Per-script timeout overrides the global default. A null or - // non-positive per-script value (≤ 0) falls back to the global. - var timeout = TimeSpan.FromSeconds( - executionTimeoutSeconds is { } perScript && perScript > 0 - ? perScript - : options.ScriptExecutionTimeoutSeconds); - - // Run the alarm on-trigger body on the dedicated - // script-execution scheduler, not the shared .NET thread pool. An injected - // scheduler (#18) overrides the process-wide default. - var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); - - _ = Task.Factory.StartNew(async () => - { - using var cts = new CancellationTokenSource(timeout); - try - { - // AlarmExecutionActor can call Instance.CallScript() - // via the ScriptRuntimeContext injected into globals - var context = new ScriptRuntimeContext( - instanceActor, - self, - sharedScriptLibrary, - currentCallDepth: 0, - options.MaxScriptCallDepth, - timeout, - instanceName, - logger, - // ParentExecutionId tag-cascade: the - // alarm on-trigger run mints its own fresh ExecutionId (the - // ctor's `?? NewGuid()` fallback) and records the firing - // execution's id as its ParentExecutionId — null (a root) - // only when the firing value came from the DCL. - parentExecutionId: parentExecutionId, - // WaitForAttribute (spec §4.4): thread the alarm on-trigger - // script's per-script execution-timeout token so a - // Attributes.WaitAsync inside an on-trigger script is bounded - // by the same script deadline. - scriptTimeoutToken: cts.Token); - - var globals = new ScriptGlobals - { - Instance = context, - Parameters = new ScriptParameters(), - CancellationToken = cts.Token, - Alarm = new AlarmContext - { - Name = alarmName, - Level = level, - Priority = priority, - Message = message - } - }; - - await compiledScript.RunAsync(globals, cts.Token); - - parent.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, true)); - } - catch (OperationCanceledException) - { - logger.LogWarning( - "Alarm on-trigger script for {Alarm} on {Instance} timed out", - alarmName, instanceName); - parent.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, false)); - } - catch (Exception ex) - { - // Failures logged, alarm continues - logger.LogError(ex, - "Alarm on-trigger script for {Alarm} on {Instance} failed", - alarmName, instanceName); - parent.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, false)); - } - finally - { - self.Tell(PoisonPill.Instance); - } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler).Unwrap(); - } -} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index 938de1fa..8495149f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -142,6 +142,23 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// private readonly HashSet _initFailedPendingRowRemoval = new(); + /// + /// WP3.1 warm-then-gate: instances whose deploy compile is being warmed off the actor + /// thread, keyed by instance name. Presence is the per-instance in-flight guard that keeps + /// same-instance command ordering intact while OTHER instances' commands flow freely. + /// Entries are added in and removed in + /// — one or the other always runs, because the + /// warm task pipes its result back unconditionally (it swallows its own exceptions). + /// + private readonly Dictionary _deployWarms = new(); + + /// + /// WP3.1: script-execution scheduler seam (#18) — the pool this actor grows as instances + /// deploy. Null selects the process-wide shared scheduler; tests inject their own so + /// sizing assertions do not perturb (or depend on) the process-wide singleton. + /// + private readonly ScriptExecutionScheduler? _scriptScheduler; + /// Akka timer scheduler injected by the framework via . public ITimerScheduler Timers { get; set; } = null!; @@ -172,6 +189,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// external-system changes. Optional/null in tests that do not exercise external-system /// caching. /// + /// + /// WP3.1: optional script-execution scheduler override (#18). This actor grows the pool + /// towards on every instance- + /// count change; null uses the process-wide shared scheduler. + /// public DeploymentManagerActor( SiteStorageService storage, ScriptCompilationService compilationService, @@ -186,8 +208,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers IDeploymentConfigFetcher? configFetcher = null, TimeSpan? startupLoadRetryInterval = null, Func>>? configLoader = null, - ExternalSystemDefinitionCache? externalSystemCache = null) + ExternalSystemDefinitionCache? externalSystemCache = null, + ScriptExecutionScheduler? scriptScheduler = null) { + _scriptScheduler = scriptScheduler; _storage = storage; _compilationService = compilationService; _deployCompileValidator = new DeployCompileValidator(compilationService); @@ -212,9 +236,16 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers // Lifecycle commands Receive(cmd => HandleDeploy(cmd, Sender)); - Receive(HandleDisable); - Receive(HandleEnable); - Receive(HandleDelete); + Receive(cmd => HandleDisable(cmd, Sender)); + Receive(cmd => HandleEnable(cmd, Sender)); + Receive(cmd => HandleDelete(cmd, Sender)); + + // WP3.1 warm-then-gate: the off-thread compile warm for a deploy has finished, so + // the (now all-cache-hit) synchronous compile gate can run on the actor thread. + Receive(HandleDeployCompileWarmed); + // WP3.1: a staggered startup batch's compiles have been pre-warmed off-thread; + // create the batch's Instance Actors now that their PreStart compiles are hits. + Receive(msg => CreateInstanceActorBatch(msg.Batch)); // Notify-and-fetch: central sends a small RefreshDeploymentCommand; // the active singleton fetches the flattened config over HTTP, then reuses the @@ -461,8 +492,15 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers } /// - /// Creates Instance Actors in batches with a configurable delay between batches - /// to prevent reconnection storms on failover. + /// WP3.1: pre-warms one staggered startup batch's script and trigger-expression compiles + /// off the actor thread, then pipes so + /// can create the batch's Instance Actors with + /// every PreStart compile already a cache hit. + /// + /// This closes the long-standing "per-instance compilation during staggered startup" + /// gap: on failover, each Instance Actor used to Roslyn-compile its own scripts inside + /// PreStart, serialising a site's whole recovery behind compilation. The cost is one + /// extra message per batch. /// private void HandleStartNextBatch(StartNextBatch msg) { @@ -471,6 +509,45 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers var startIdx = state.NextIndex; var endIdx = Math.Min(startIdx + batchSize, state.Configs.Count); + var validator = _deployCompileValidator; + var batchConfigs = new string[endIdx - startIdx]; + for (var i = startIdx; i < endIdx; i++) + batchConfigs[i - startIdx] = state.Configs[i].ConfigJson; + + Task.Run(() => + { + foreach (var configJson in batchConfigs) + { + try + { + // Verdict discarded: startup does not gate on compile failures (a failing + // script is logged by the Instance Actor and leaves the rest running). + // This call exists only to populate SiteScriptCompileCache. + validator.Validate(configJson); + } + catch + { + // Best-effort warm — a throw here just means that instance's PreStart + // compiles the hard way, exactly as it did before WP3.1. + } + } + + return new BatchCompileWarmed(msg); + }).PipeTo(Self); + } + + /// + /// Creates one batch's Instance Actors (after its compiles have been pre-warmed) and + /// schedules the next batch, with a configurable delay between batches to prevent + /// reconnection storms on failover. + /// + private void CreateInstanceActorBatch(StartNextBatch msg) + { + var state = msg.State; + var batchSize = _options.StartupBatchSize; + var startIdx = state.NextIndex; + var endIdx = Math.Min(startIdx + batchSize, state.Configs.Count); + _logger.LogDebug( "Creating Instance Actors batch [{Start}..{End}) of {Total}", startIdx, endIdx, state.Configs.Count); @@ -520,23 +597,164 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// apply still replies to the right actor. /// private void HandleDeploy(DeployInstanceCommand command, IActorRef replyTo) + { + var instanceName = command.InstanceUniqueName; + + // WP3.1 warm-then-gate. The S3 gate below must stay synchronous on the actor thread + // (mailbox FIFO is what makes redeploy-supersede and delete-during-redeploy correct), + // but the Roslyn compile it performs used to hold the whole singleton — every OTHER + // instance's commands stalled behind one instance's compile. So the compile is WARMED + // off-thread first and the synchronous gate then re-runs as pure cache hits. + // + // Ordering is preserved by an explicit per-instance in-flight guard: while a warm is + // in flight for instance X, further mutating commands for X are queued here rather + // than racing ahead. Commands for OTHER instances flow freely, which is the whole + // point — and safe, because cross-instance ordering was never guaranteed to callers. + if (_deployWarms.TryGetValue(instanceName, out var warming)) + { + // Nothing queued behind the pending deploy yet: plain last-write-wins, and the + // displaced deployer is told it was superseded so it never waits out its Ask — + // the same contract as the mid-termination redeploy buffer below. + if (warming.Buffered.Count == 0) + { + warming.ReplyTo.Tell(new DeploymentStatusResponse( + warming.Command.DeploymentId, instanceName, DeploymentStatus.Failed, + $"superseded by newer deployment {command.DeploymentId} before the site compile gate ran", + DateTimeOffset.UtcNow)); + warming.Command = command; + warming.ReplyTo = replyTo; + // Warm the NEW config; the in-flight warm's result is recognised as stale by + // the reference check in HandleDeployCompileWarmed and dropped. + StartDeployCompileWarm(command); + return; + } + + // A delete/disable/enable is already queued behind the pending deploy — + // superseding now would reorder it past this deploy, so queue instead. + warming.Buffered.Add(new BufferedInstanceCommand(command, replyTo)); + return; + } + + _deployWarms[instanceName] = new DeployWarmState(command, replyTo); + StartDeployCompileWarm(command); + } + + /// + /// Compiles the deployment's scripts off the actor thread purely to populate the + /// process-wide , then pipes + /// back so the authoritative gate can run on the actor + /// thread against a warm cache. The warm is best-effort: its verdict is discarded and any + /// exception swallowed, because re-derives the + /// verdict (and reproduces any failure) exactly as it did before WP3.1. + /// + private void StartDeployCompileWarm(DeployInstanceCommand command) + { + var validator = _deployCompileValidator; + var configJson = command.FlattenedConfigurationJson; + Task.Run(() => + { + try + { + validator.Validate(configJson); + } + catch (Exception ex) + { + _logger.LogDebug(ex, + "Deploy compile warm for {Instance} threw; the synchronous gate will re-run it", + command.InstanceUniqueName); + } + + return new DeployCompileWarmed(command); + }).PipeTo(Self); + } + + /// + /// Runs the authoritative compile gate for a warmed deployment on the actor thread, then + /// drains any commands queued for that instance during the warm — inline and in arrival + /// order, NOT by re-telling Self, which would put them behind messages that landed + /// in the mailbox during the warm and so reorder same-instance commands. + /// + private void HandleDeployCompileWarmed(DeployCompileWarmed msg) + { + var instanceName = msg.Command.InstanceUniqueName; + if (!_deployWarms.TryGetValue(instanceName, out var warming)) + return; + + // A newer deploy superseded this one while its warm was running; that supersede + // started its own warm, so this result is stale and must not apply a dead command. + if (!ReferenceEquals(warming.Command, msg.Command)) + return; + + _deployWarms.Remove(instanceName); + + RunDeployGateAndProceed(warming.Command, warming.ReplyTo); + + foreach (var queued in warming.Buffered) + DispatchBufferedCommand(queued); + } + + /// + /// Queues a mutating lifecycle command for an instance whose deploy is mid-compile-warm. + /// Returns when the command was queued (the caller must return + /// immediately), when no warm is in flight and the caller should + /// proceed normally. + /// + private bool TryBufferDuringDeployWarm(string instanceName, object command, IActorRef replyTo) + { + if (!_deployWarms.TryGetValue(instanceName, out var warming)) return false; + warming.Buffered.Add(new BufferedInstanceCommand(command, replyTo)); + return true; + } + + /// + /// Re-dispatches one command queued during a deploy compile warm, restoring the original + /// sender. A queued deploy legitimately opens a NEW warm window, in which case the + /// commands after it re-queue behind that one — ordering still holds. + /// + private void DispatchBufferedCommand(BufferedInstanceCommand queued) + { + switch (queued.Command) + { + case DeployInstanceCommand deploy: + HandleDeploy(deploy, queued.Sender); + break; + case DeleteInstanceCommand delete: + HandleDelete(delete, queued.Sender); + break; + case DisableInstanceCommand disable: + HandleDisable(disable, queued.Sender); + break; + case EnableInstanceCommand enable: + HandleEnable(enable, queued.Sender); + break; + default: + _logger.LogWarning( + "Unhandled buffered command type {Type} for a deploy compile warm — dropped", + queued.Command.GetType().Name); + break; + } + } + + /// + /// The authoritative site-side compile gate (S3) plus the deploy application. Reached + /// only from , i.e. after the same compile has + /// been warmed off-thread. + /// + private void RunDeployGateAndProceed(DeployInstanceCommand command, IActorRef replyTo) { // Site-side compile gate (S3): a compile failure must reject the // deployment with NO partial state applied (no Instance Actor, no - // persisted config) — the design spec's contract. Validation runs - // synchronously on the actor thread: it is a pure prefix step of the - // deploy handler, so the existing redeploy-supersede / delete-during- - // redeploy ordering (which depends on strict mailbox FIFO) is preserved - // exactly. It is NOT run off-thread — piping the verdict back to self - // reorders concurrent deploys relative to each other and to - // delete/disable commands, breaking that ordering. A deploy is an - // infrequent admin command, so briefly holding the singleton for a pure - // Roslyn compile is acceptable; the central deployer already Asks and - // waits for the DeploymentStatusResponse. Redeploys and multi-instance - // deploys of unchanged scripts hit the process-wide compile cache - // (SiteScriptCompileCache), so the synchronous gate recompiles only - // genuinely new code and the Instance Actor's PreStart reuses the gate's - // compile (N4). + // persisted config) — the design spec's contract. Validation still runs + // synchronously on the actor thread as a pure prefix step of the deploy + // application, so the existing redeploy-supersede / delete-during-redeploy + // ordering (which depends on strict mailbox FIFO) is preserved exactly. + // WP3.1 removed the COST rather than the ordering: the same compile has + // already been warmed off-thread (StartDeployCompileWarm), so this call is + // pure cache hits and no longer holds the singleton — while the per-instance + // warm guard keeps this instance's own command ordering intact. Redeploys and + // multi-instance deploys of unchanged scripts hit the process-wide compile + // cache (SiteScriptCompileCache) too, and the Instance Actor's PreStart reuses + // the gate's compile (N4). var errors = _deployCompileValidator.Validate(command.FlattenedConfigurationJson); if (errors.Count > 0) { @@ -942,10 +1160,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// /// Disables an instance: stops the actor and marks as disabled in SQLite. /// - private void HandleDisable(DisableInstanceCommand command) + private void HandleDisable(DisableInstanceCommand command, IActorRef replyTo) { var instanceName = command.InstanceUniqueName; + // WP3.1 warm-then-gate: a deploy for this instance is mid-compile-warm; queue this + // command so same-instance ordering is preserved (see TryBufferDuringDeployWarm). + if (TryBufferDuringDeployWarm(instanceName, command, replyTo)) return; + // A disable arriving mid-redeploy must cancel the buffered // redeploy. Otherwise HandleTerminated re-creates the Instance Actor and // re-stores its config with isEnabled: true when the predecessor terminates, @@ -975,7 +1197,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers UpdateInstanceCounts(); - var sender = Sender; + var sender = replyTo; _storage.SetInstanceEnabledAsync(instanceName, false).ContinueWith(t => { if (t.IsCompletedSuccessfully) @@ -1005,10 +1227,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// Enables an instance: marks as enabled in SQLite and re-creates the Instance Actor /// from the stored config. /// - private void HandleEnable(EnableInstanceCommand command) + private void HandleEnable(EnableInstanceCommand command, IActorRef replyTo) { var instanceName = command.InstanceUniqueName; - var sender = Sender; + + // WP3.1 warm-then-gate: see TryBufferDuringDeployWarm. + if (TryBufferDuringDeployWarm(instanceName, command, replyTo)) return; + + var sender = replyTo; Task.Run(async () => { @@ -1061,10 +1287,13 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// Deletes an instance: stops the actor and removes config from SQLite. /// Note: store-and-forward messages are NOT cleared per design decision. /// - private void HandleDelete(DeleteInstanceCommand command) + private void HandleDelete(DeleteInstanceCommand command, IActorRef replyTo) { var instanceName = command.InstanceUniqueName; + // WP3.1 warm-then-gate: see TryBufferDuringDeployWarm. + if (TryBufferDuringDeployWarm(instanceName, command, replyTo)) return; + // A delete arriving while a redeploy is still terminating must // be authoritative over the mid-redeploy bookkeeping. HandleDeploy already // removed the instance from _instanceActors and buffered a PendingRedeploy @@ -1105,7 +1334,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers _deployedInstanceNames.Remove(instanceName); UpdateInstanceCounts(); - var sender = Sender; + var sender = replyTo; _storage.RemoveDeployedConfigAsync(instanceName).ContinueWith(t => { if (t.IsCompletedSuccessfully) @@ -1135,7 +1364,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// Fire-and-forget a deployment operational event to the optional /// on a deploy/enable/disable/delete outcome. /// Resolved optionally and never awaited so a logging failure cannot affect the - /// deployment pipeline (matching the established ScriptActor/ScriptExecutionActor + /// deployment pipeline (matching the established ScriptActor / script-run /// pattern). /// /// Thread-safety: the disable () and delete @@ -2089,7 +2318,8 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers internal int InstanceActorCount => _instanceActors.Count; /// - /// Updates the health collector with current instance counts. + /// Updates the health collector with current instance counts, and (WP3.1) grows the + /// script-execution pool to match. /// Total deployed = _deployedInstanceNames.Count, enabled = running actors, disabled = difference. /// private void UpdateInstanceCounts() @@ -2098,6 +2328,22 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers deployed: _deployedInstanceNames.Count, enabled: _instanceActors.Count, disabled: _deployedInstanceNames.Count - _instanceActors.Count); + + // WP3.1: the blocking script pool scales with the number of running instances rather + // than sitting at a fixed 8 forever. This is the single call site because it already + // runs on every deploy / undeploy / enable / disable and once per staggered startup + // batch. Growth is idempotent and one-way — see ScriptExecutionScheduler.EnsureCapacity. + try + { + var target = ScriptExecutionScheduler.ComputeTargetThreads(_instanceActors.Count, _options); + (_scriptScheduler ?? ScriptExecutionScheduler.Shared(_options)).EnsureCapacity(target); + } + catch (Exception ex) + { + // Pool sizing is an optimisation, never a correctness requirement: a failure here + // must not fail the deploy/enable/disable that triggered it. + _logger.LogWarning(ex, "Failed to resize the script-execution pool; continuing at its current size."); + } } // ── Internal messages ── @@ -2131,6 +2377,43 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers internal record SharedScriptsLoaded( List EnabledConfigs, int CompiledCount, int TotalCount); + /// + /// WP3.1 warm-then-gate: piped back to self once a deployment's scripts have been + /// compiled off the actor thread into the process-wide compile cache. Carries the exact + /// command instance it warmed so a result superseded mid-warm is recognised by reference + /// and dropped. + /// + internal sealed record DeployCompileWarmed(DeployInstanceCommand Command); + + /// + /// WP3.1: piped back to self once a staggered startup batch's compiles have been + /// pre-warmed, carrying the original batch message so actor creation resumes unchanged. + /// + internal sealed record BatchCompileWarmed(StartNextBatch Batch); + + /// + /// A lifecycle command queued because its instance's deploy was mid-compile-warm, with + /// the sender to answer once it is re-dispatched. + /// + internal sealed record BufferedInstanceCommand(object Command, IActorRef Sender); + + /// + /// The in-flight state of one instance's deploy compile warm: the deploy that will be + /// applied when the warm lands (replaceable last-write-wins while nothing is queued behind + /// it), the deployer to answer, and the ordered commands queued during the warm. + /// + internal sealed class DeployWarmState(DeployInstanceCommand command, IActorRef replyTo) + { + /// The deploy to apply when the warm completes. + public DeployInstanceCommand Command { get; set; } = command; + + /// The deployer awaiting this deploy's . + public IActorRef ReplyTo { get; set; } = replyTo; + + /// Commands for this instance that arrived during the warm, in arrival order. + public List Buffered { get; } = []; + } + internal record StartNextBatch(BatchState State); internal record BatchState(List Configs, int NextIndex); internal record EnableResult( diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index 61e98da8..21f4e7d7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -374,7 +374,7 @@ public class InstanceActor : ReceiveActor /// Fire-and-forget an instance_lifecycle operational event to the /// optional . Resolved optionally and never /// awaited so a logging failure cannot affect the instance lifecycle - /// (matching the established ScriptActor/ScriptExecutionActor pattern). + /// (matching the established ScriptActor / script-run pattern). /// private void LogLifecycleEvent(string message) { @@ -1666,8 +1666,8 @@ public class InstanceActor : ReceiveActor { Script? onTriggerScript = null; // The on-trigger script's per-script execution timeout, - // captured from its ResolvedScript so the AlarmExecutionActor can - // apply perScript ?? global. Null when there is no on-trigger script. + // captured from its ResolvedScript so the launched on-trigger run + // can apply perScript ?? global. Null when there is no on-trigger script. int? onTriggerTimeoutSeconds = null; // Compile on-trigger script if defined diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/NativeAlarmActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/NativeAlarmActor.cs index 4577ce2c..ce74b01b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/NativeAlarmActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/NativeAlarmActor.cs @@ -444,7 +444,7 @@ public class NativeAlarmActor : ReceiveActor /// condition's severity); an inactive condition is a return-to-normal; an /// acknowledge transition is informational. Resolved optionally and never /// awaited so a logging failure cannot affect the mirror (matching the - /// established ScriptActor/ScriptExecutionActor pattern). + /// established ScriptActor / script-run pattern). /// private void LogAlarmEvent(NativeAlarmTransition t, AlarmConditionState condition) { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs index cd7e4a98..a8a6e71c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs @@ -15,8 +15,16 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// /// Script Actor — coordinator actor, child of Instance Actor. -/// Holds compiled script delegate, manages trigger configuration, and spawns -/// ScriptExecutionActor children per invocation. Does not block on child completion. +/// Holds compiled script delegate, manages trigger configuration, and launches script +/// runs per invocation. Does not block on run completion. +/// +/// WP3.1: runs are launched directly via rather than +/// through a short-lived ScriptExecutionActor child (which had no Receive +/// handler, no PostStop, and whose IActorRef was never a message target — pure +/// per-run actor-cell overhead). Concurrent runs are bounded by +/// ; over the cap the NEWEST +/// trigger is shed. Trigger-expression evaluation no longer runs on the blocking script pool +/// at all — see . /// /// Trigger types: /// - Interval: uses Akka timers to fire periodically @@ -43,20 +51,52 @@ public class ScriptActor : ReceiveActor, IWithTimers /// /// Script-execution scheduler seam (#18): the process-wide /// when null, or an injected instance so - /// this actor's trigger-expression evaluation and spawned script bodies run on a - /// caller-owned pool instead of the shared one. Resolved lazily at each use so the - /// null (host) path stays byte-for-byte identical to the previous static call. + /// this actor's launched script bodies run on a caller-owned pool instead of the + /// shared one. Resolved lazily at each use so the null (host) path stays + /// byte-for-byte identical to the previous static call. + /// + /// WP3.1: trigger-expression evaluation no longer uses this scheduler — see + /// . /// private readonly ScriptExecutionScheduler? _scheduler; + /// + /// WP3.1 (finding #4): the concurrency gate for trigger-expression evaluation, or null + /// for the process-wide . Evaluations run as plain + /// async work on the shared .NET thread pool behind this gate, NOT on + /// — that is what stops an Expression trigger from queueing + /// behind blocking script bodies. + /// + private readonly TriggerEvalGate? _evalGate; + + /// + /// WP3.1: runs launched but not yet completed (queued or executing) for this script. + /// Incremented at launch, decremented on , which + /// every terminal path emits — including the launch-path catch, so the counter cannot + /// leak. Touched only on the actor thread. + /// + private int _runsInFlight; + + /// + /// WP3.1: when the last shed Warning site event was emitted for this script. Sheds are + /// ALWAYS counted on the health collector; the site event is rate-limited to one per + /// script per minute so a hot trigger against a saturated cap cannot flood + /// site_events. + /// + private DateTimeOffset _lastShedEventUtc = DateTimeOffset.MinValue; + + /// Rate limit for the shed site event (the counter still counts every shed). + private static readonly TimeSpan ShedEventInterval = TimeSpan.FromMinutes(1); + private Script? _compiledScript; private ScriptTriggerConfig? _triggerConfig; private TimeSpan? _minTimeBetweenRuns; /// /// The per-script execution timeout in seconds, or null to use the - /// global default. Threaded down to each spawned , - /// which applies perScript ?? global (and treats ≤ 0 as "use global"). + /// global default. Threaded down to each launched run via + /// , which applies perScript ?? global + /// (and treats ≤ 0 as "use global"). /// private readonly int? _executionTimeoutSeconds; private DateTimeOffset _lastExecutionTime = DateTimeOffset.MinValue; @@ -111,6 +151,7 @@ public class ScriptActor : ReceiveActor, IWithTimers /// Optional health metrics collector. /// Optional DI service provider for script execution context services. /// Optional script-execution scheduler override (#18); null uses the process-wide shared scheduler. + /// Optional trigger-expression concurrency gate override (WP3.1); null uses the process-wide shared gate. public ScriptActor( string scriptName, string instanceName, @@ -124,7 +165,8 @@ public class ScriptActor : ReceiveActor, IWithTimers IReadOnlyDictionary? initialAttributes = null, ISiteHealthCollector? healthCollector = null, IServiceProvider? serviceProvider = null, - ScriptExecutionScheduler? scheduler = null) + ScriptExecutionScheduler? scheduler = null, + TriggerEvalGate? evalGate = null) { _scriptName = scriptName; _instanceName = instanceName; @@ -136,6 +178,7 @@ public class ScriptActor : ReceiveActor, IWithTimers _healthCollector = healthCollector; _serviceProvider = serviceProvider; _scheduler = scheduler; + _evalGate = evalGate; _minTimeBetweenRuns = scriptConfig.MinTimeBetweenRuns; _executionTimeoutSeconds = scriptConfig.ExecutionTimeoutSeconds; _scope = scriptConfig.Scope; @@ -201,24 +244,15 @@ public class ScriptActor : ReceiveActor, IWithTimers _scriptName, _instanceName); } - /// - protected override SupervisorStrategy SupervisorStrategy() - { - return new OneForOneStrategy( - maxNrOfRetries: -1, - withinTimeRange: TimeSpan.FromMinutes(1), - decider: Decider.From(ex => - { - _logger.LogWarning(ex, - "ScriptExecutionActor for {Script} on {Instance} failed, stopping", - _scriptName, _instanceName); - return Directive.Stop; - })); - } + // WP3.1: the Stop-on-failure SupervisorStrategy override is gone with the per-run + // ScriptExecutionActor child it supervised. This actor has no children left; a + // launch-path throw is caught in SpawnExecution (which replies to the Ask caller and + // releases the in-flight slot) so it can never escalate to InstanceActor, which + // continues to supervise this actor with Resume exactly as before. /// /// Handles CallScript ask from ScriptRuntimeContext or Instance Actor. - /// Spawns a ScriptExecutionActor and forwards the sender for reply. + /// Launches a run and captures the sender for the eventual reply. /// private void HandleScriptCallRequest(ScriptCallRequest request) { @@ -233,7 +267,7 @@ public class ScriptActor : ReceiveActor, IWithTimers } // (ParentExecutionId): carry any inbound-routed ParentExecutionId - // through to the ScriptExecutionActor so the routed script's + // through to the launched run so the routed script's // ScriptRuntimeContext can record its spawner. Null for normal // (tag-change / timer) runs and nested Script.Call invocations. SpawnExecution( @@ -287,12 +321,22 @@ public class ScriptActor : ReceiveActor, IWithTimers /// Starts an off-dispatcher evaluation of the compiled trigger expression (P1). /// The expression previously ran synchronously on the actor's dispatcher thread /// via RunAsync(...).GetAwaiter().GetResult(), blocking the dispatcher on - /// every attribute change. It now runs on the dedicated script-execution - /// scheduler against a point-in-time snapshot; the boolean result is piped back - /// to this actor as an so all edge state is - /// applied on the actor thread. Bursts coalesce: at most one evaluation is in - /// flight, at most one pending, so a storm of changes collapses to the latest - /// snapshot without unbounded task fan-out. + /// every attribute change. It runs off-thread against a point-in-time snapshot; + /// the boolean result is piped back to this actor as an + /// so all edge state is applied on the actor + /// thread. Bursts coalesce: at most one evaluation is in flight, at most one + /// pending, so a storm of changes collapses to the latest snapshot without + /// unbounded task fan-out. + /// + /// WP3.1 (finding #4) changed WHERE it runs and WHEN its clock starts. It used + /// to run on the dedicated — the same bounded + /// pool as blocking script bodies — so N blocked scripts stalled every Expression + /// trigger on the node indefinitely. It now runs as plain async work on the shared + /// .NET thread pool behind : trigger expressions are + /// non-blocking by construction, so they belong there. And the deadline + /// is armed HERE, on the actor thread, before + /// the work is queued — so gate-wait time burns the same budget and a saturated gate + /// yields a timely false instead of an unbounded stall. /// private void StartExpressionEvaluation() { @@ -303,29 +347,45 @@ public class ScriptActor : ReceiveActor, IWithTimers var snapshot = new Dictionary(_attributeSnapshot); // point-in-time copy, actor thread var expression = _compiledTriggerExpression; var self = Self; - Task.Factory.StartNew(async () => + var gate = _evalGate ?? TriggerEvalGate.Shared(_options); + // Clock starts AT ENQUEUE, not at dequeue. + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(_options.TriggerEvalTimeoutSeconds)); + + Task.Run(async () => { try { - // Bound evaluation with a short timeout. The CancellationToken covers - // cooperative/async cases; a pathological CPU-bound expression is not - // fully interruptible — acceptable because trigger expressions are - // authored by trusted Design-role users and compile-checked pre-deploy. - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); - var state = await expression.RunAsync(new TriggerExpressionGlobals(snapshot), cancellationToken: cts.Token); - return state.ReturnValue is bool b && b; + await gate.WaitAsync(cts.Token).ConfigureAwait(false); + try + { + // The CancellationToken covers cooperative/async cases; a pathological + // CPU-bound expression is not fully interruptible — acceptable because + // trigger expressions are authored by trusted Design-role users and + // compile-checked pre-deploy. + var state = await expression.RunAsync( + new TriggerExpressionGlobals(snapshot), cancellationToken: cts.Token); + return state.ReturnValue is bool b && b; + } + finally + { + gate.Release(); + } } catch (Exception ex) { - // OperationCanceledException (timeout) falls through here too and is - // treated as false. LogExpressionError touches only thread-safe - // members (_healthCollector Interlocked, _logger, DI-resolved - // singleton logger) so it is safe off the actor thread. + // OperationCanceledException (timeout, INCLUDING a timeout that fired while + // still waiting on the gate) falls through here too and is treated as false. + // LogExpressionError touches only thread-safe members (_healthCollector + // Interlocked, _logger, DI-resolved singleton logger) so it is safe off the + // actor thread. LogExpressionError(ex); return false; } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, - _scheduler ?? ScriptExecutionScheduler.Shared(_options)).Unwrap().PipeTo(self, + finally + { + cts.Dispose(); + } + }).PipeTo(self, success: r => new ExpressionEvalResult(r), failure: ex => new ExpressionEvalFailed(ex)); } @@ -421,7 +481,7 @@ public class ScriptActor : ReceiveActor, IWithTimers /// /// Records a trigger-expression evaluation failure to the site event log, - /// mirroring how ScriptExecutionActor reports script errors. + /// mirroring how a script run reports its own errors. /// private void LogExpressionError(Exception ex) { @@ -457,8 +517,15 @@ public class ScriptActor : ReceiveActor, IWithTimers } /// - /// Spawns a new ScriptExecutionActor child for this invocation. - /// Multiple concurrent executions are allowed. + /// Launches a run of this script. Multiple concurrent runs are allowed, up to + /// ; beyond that the newest + /// trigger is shed (see ). + /// + /// WP3.1: the run is launched directly on the script-execution scheduler via + /// — the actor's mailbox stays on the default dispatcher, + /// but the script body runs on the bounded set of dedicated threads, so blocking script + /// I/O is contained there and cannot starve the shared .NET thread pool. No per-run child + /// actor is created. /// private void SpawnExecution( IReadOnlyDictionary? parameters, @@ -467,46 +534,123 @@ public class ScriptActor : ReceiveActor, IWithTimers string correlationId, Guid? parentExecutionId = null) { - var executionId = $"{_scriptName}-exec-{_executionCounter++}"; + if (_runsInFlight >= _options.MaxConcurrentRunsPerScript) + { + ShedRun(replyTo, correlationId); + return; + } - // The actor's mailbox stays on the default dispatcher, but the - // script body itself runs on the dedicated ScriptExecutionScheduler (a bounded - // set of dedicated threads), so blocking script I/O is contained there and - // cannot starve the shared .NET thread pool. - var props = Props.Create(() => new ScriptExecutionActor( - _scriptName, - _instanceName, - _compiledScript!, - parameters, - callDepth, - _instanceActor, - _sharedScriptLibrary, - _options, - replyTo, - correlationId, - _logger, - _scope, - _healthCollector, - _serviceProvider, - // (ParentExecutionId): null for trigger-driven runs; - // an inbound-API-routed call supplies the inbound request's id. - parentExecutionId, - // Per-script timeout override (null = use global). - _executionTimeoutSeconds, - // Scheduler seam (#18): thread this actor's scheduler override down so - // spawned script bodies share the same pool (null = process-wide shared). - _scheduler)); + var runId = _executionCounter++; + // Incremented BEFORE the launch so the launch-path catch below (which always emits a + // ScriptExecutionCompleted) balances it on every path — the counter cannot leak. + _runsInFlight++; - Context.ActorOf(props, executionId); + try + { + ScriptRunLauncher.LaunchScript( + _scriptName, + _instanceName, + _compiledScript!, + parameters, + callDepth, + _instanceActor, + _sharedScriptLibrary, + _options, + replyTo, + correlationId, + // Completion target: this actor. Identical delivery to the old + // Context.Parent.Tell from the execution actor, and it doubles as the + // in-flight release. + Self, + // Notification Outbox: the site communication actor Notify.Status queries + // central through. Resolved here, on the actor thread, and handed to the + // launcher (which has no ActorContext of its own). + Context.System.ActorSelection("/user/site-communication"), + _logger, + _scope, + runId, + _healthCollector, + _serviceProvider, + // (ParentExecutionId): null for trigger-driven runs; + // an inbound-API-routed call supplies the inbound request's id. + parentExecutionId, + // Per-script timeout override (null = use global). + _executionTimeoutSeconds, + // Scheduler seam (#18): thread this actor's scheduler override down so + // launched script bodies share the same pool (null = process-wide shared). + _scheduler); + } + catch (Exception ex) + { + // WP3.1 supervision parity: the only failure the removed per-run child could + // surface was a constructor throw (e.g. queueing onto a disposed scheduler), + // which the old OneForOneStrategy logged and stopped — leaving an Ask caller to + // hang with no reply. Same log shape, same "coordinator unaffected" outcome, plus + // a reply so the caller fails fast instead of waiting out its Ask timeout. + var errorMsg = $"Script '{_scriptName}' on instance '{_instanceName}' could not be launched: {ex.Message}"; + _logger.LogWarning(ex, + "Script execution launch for {Script} on {Instance} failed, stopping", + _scriptName, _instanceName); + + if (!replyTo.IsNobody()) + replyTo.Tell(new ScriptCallResult(correlationId, false, null, errorMsg)); + + Self.Tell(new ScriptExecutionCompleted(_scriptName, false, errorMsg)); + } + } + + /// + /// WP3.1 shed policy: refuses the incoming run because + /// runs are already in flight. + /// The four already queued/running are kept — they are closest to their own deadlines and + /// already charged against them — so nothing is ever reordered. A trigger-driven run is + /// simply not launched; an Ask-based CallScript gets an explicit error so a nested + /// call or inbound-API route fails fast rather than hanging to its Ask timeout. + /// + private void ShedRun(IActorRef replyTo, string correlationId) + { + _healthCollector?.IncrementScriptRunShed(); + + var message = $"Script '{_scriptName}' on instance '{_instanceName}': run shed — " + + $"{_runsInFlight} runs already in flight (cap {_options.MaxConcurrentRunsPerScript})."; + + _logger.LogWarning("{Message}", message); + + // Rate-limited to one event per script per minute: the counter above still counts + // every shed, but a hot trigger against a saturated cap must not flood site_events. + var now = DateTimeOffset.UtcNow; + if (now - _lastShedEventUtc >= ShedEventInterval) + { + _lastShedEventUtc = now; + _ = _serviceProvider?.GetService()?.LogEventAsync( + "script", "Warning", _instanceName, $"ScriptActor:{_scriptName}", message); + } + + if (!replyTo.IsNobody()) + { + replyTo.Tell(new ScriptCallResult( + correlationId, false, null, + $"shed: {_runsInFlight} runs already in flight")); + } } private void HandleExecutionCompleted(ScriptExecutionCompleted msg) { + // WP3.1: release the in-flight slot. Every terminal path emits exactly one of these + // (success / timeout / failure / launch failure), so the counter tracks reality. + if (_runsInFlight > 0) _runsInFlight--; + _logger.LogDebug( "Script {Script} execution completed on {Instance}: success={Success}", _scriptName, _instanceName, msg.Success); } + /// + /// WP3.1: runs launched but not yet completed. Exposed for regression coverage of the + /// shed cap — the counter is otherwise invisible from outside the actor. + /// + internal int RunsInFlight => _runsInFlight; + // internal (not private) so the culture-invariance of the non-numeric fallback // can be unit-tested directly on the test thread — the live path evaluates on a // dispatcher thread whose CurrentCulture the test cannot deterministically set. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs deleted file mode 100644 index c568a913..00000000 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs +++ /dev/null @@ -1,341 +0,0 @@ -using Akka.Actor; -using Microsoft.CodeAnalysis.Scripting; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using ZB.MOM.WW.ScadaBridge.Commons.Interfaces; -using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; -using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; -using ZB.MOM.WW.ScadaBridge.Commons.Types; -using ZB.MOM.WW.ScadaBridge.HealthMonitoring; -using ZB.MOM.WW.ScadaBridge.SiteEventLogging; -using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; -using ZB.MOM.WW.ScadaBridge.StoreAndForward; - -namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; - -/// -/// Script Execution Actor -- short-lived child of Script Actor. -/// Receives compiled code, params, Instance Actor ref, and call depth. -/// Executes the script via Script Runtime API, returns result, then stops. -/// -/// The actor itself and its mailbox run on the default Akka dispatcher; only the -/// script body is dispatched off the actor thread, onto the dedicated -/// , -/// so blocking script I/O cannot starve the shared thread pool -/// or stall other Akka dispatchers. -/// -/// Script failures are logged but do not disable the script. -/// Supervision: Stop on unhandled exception (parent ScriptActor decides). -/// -public class ScriptExecutionActor : ReceiveActor -{ - /// - /// Initializes the actor and immediately begins script execution on construction. - /// - /// Name of the script being executed. - /// Name of the instance that owns the script. - /// Compiled Roslyn script to execute. - /// Optional named parameter values for the script. - /// Current call-nesting depth (used to enforce the max-depth limit). - /// Parent instance actor reference for attribute access. - /// Library of shared scripts available during execution. - /// Site runtime options applied during execution. - /// Actor reference that receives the script result. - /// Application-level correlation id threaded through the execution. - /// Logger for script execution events. - /// Script scope controlling which APIs are available. - /// Optional health collector for recording execution metrics. - /// Optional DI service provider for script execution services. - /// ExecutionId of the spawning inbound-API execution for audit correlation; null for normal runs. - /// Per-script execution timeout in seconds. Null or non-positive falls back to the global . - public ScriptExecutionActor( - string scriptName, - string instanceName, - Script compiledScript, - IReadOnlyDictionary? parameters, - int callDepth, - IActorRef instanceActor, - SharedScriptLibrary sharedScriptLibrary, - SiteRuntimeOptions options, - IActorRef replyTo, - string correlationId, - ILogger logger, - Commons.Types.Scripts.ScriptScope scope, - ISiteHealthCollector? healthCollector = null, - IServiceProvider? serviceProvider = null, - // The spawning execution's - // ExecutionId for an inbound-API-routed call. Null for normal - // (tag-change / timer) runs and nested Script.Call invocations. - Guid? parentExecutionId = null, - // Per-script execution timeout override (seconds). Null or - // non-positive falls back to the global ScriptExecutionTimeoutSeconds. - int? executionTimeoutSeconds = null, - // Script-execution scheduler seam (#18): the process-wide - // ScriptExecutionScheduler by default; a test (or a future multi-site - // host) can inject its own instance so script bodies never run on the - // shared process-wide pool. Null selects the shared default. - ScriptExecutionScheduler? scheduler = null) - { - // Immediately begin execution - var self = Self; - var parent = Context.Parent; - - ExecuteScript( - scriptName, instanceName, compiledScript, parameters, callDepth, - instanceActor, sharedScriptLibrary, options, replyTo, correlationId, - self, parent, logger, scope, healthCollector, serviceProvider, - parentExecutionId, executionTimeoutSeconds, scheduler); - } - - private static void ExecuteScript( - string scriptName, - string instanceName, - Script compiledScript, - IReadOnlyDictionary? parameters, - int callDepth, - IActorRef instanceActor, - SharedScriptLibrary sharedScriptLibrary, - SiteRuntimeOptions options, - IActorRef replyTo, - string correlationId, - IActorRef self, - IActorRef parent, - ILogger logger, - Commons.Types.Scripts.ScriptScope scope, - ISiteHealthCollector? healthCollector, - IServiceProvider? serviceProvider, - Guid? parentExecutionId, - int? executionTimeoutSeconds, - ScriptExecutionScheduler? scheduler) - { - // Per-script timeout overrides the global default. A null or - // non-positive per-script value (≤ 0) falls back to the global. - var timeout = TimeSpan.FromSeconds( - executionTimeoutSeconds is { } perScript && perScript > 0 - ? perScript - : options.ScriptExecutionTimeoutSeconds); - - // Run the script body on the dedicated script-execution - // scheduler, not the shared .NET thread pool, so blocking script I/O cannot - // starve the global pool and stall Akka dispatchers / HTTP handling. An - // injected scheduler (#18) overrides the process-wide default. - var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); - - // Notification Outbox: the site communication actor that Notify.Status queries - // central through. Resolved by actor path so the Notify helper does not need an - // IActorRef threaded all the way down from the host wiring. - var siteCommunicationActor = Context.System.ActorSelection("/user/site-communication"); - - // CTS must be created inside the async lambda so it outlives this method - _ = Task.Factory.StartNew(async () => - { - IServiceScope? serviceScope = null; - // ISiteEventLogger is a singleton; resolve from the root provider so - // it is available to the catch blocks regardless of scope state. - var siteEventLogger = serviceProvider?.GetService(); - using var cts = new CancellationTokenSource(timeout); - - // Stuck-script watchdog (S2). The CTS firing only REQUESTS cooperative - // cancellation; it does NOT free a thread blocked in synchronous I/O. - // When the timeout elapses, wait a grace period on the thread pool and, - // if the body still hasn't returned, name the script loudly — this is - // the only signal an operator gets that one of the bounded - // script-execution threads is gone. `completed` is flipped in the - // finally below; Register fires on cancellation only (normal completion - // disposes the CTS with no callback). The Task.Run/Task.Delay run on the - // thread pool, not the (possibly saturated) script scheduler — deliberate. - var completed = 0; - var graceMs = options.StuckScriptGraceMs; - cts.Token.Register(() => _ = Task.Run(async () => - { - await Task.Delay(graceMs); - if (Volatile.Read(ref completed) == 0) - { - var stuckMsg = $"Script '{scriptName}' on instance '{instanceName}' exceeded its " + - $"{timeout.TotalSeconds:F0}s timeout and is STILL EXECUTING — its dedicated " + - "script-execution thread is blocked (cooperative cancellation not observed)."; - logger.LogError(stuckMsg); - _ = siteEventLogger?.LogEventAsync("script", "Error", instanceName, - $"ScriptActor:{scriptName}", stuckMsg); - } - })); - - try - { - // Resolve integration services from DI (scoped lifetime) - IExternalSystemClient? externalSystemClient = null; - IDatabaseGateway? databaseGateway = null; - // Notification Outbox: the S&F engine is a singleton; the site identity - // provider supplies the site id stamped on enqueued notifications. - StoreAndForwardService? storeAndForward = null; - var siteId = string.Empty; - // The writer is a singleton (FallbackAuditWriter - // composes the SQLite hot-path + drop-oldest ring); null in tests / hosts - // that haven't called AddAuditLog, which the helper handles as a no-op. - IAuditWriter? auditWriter = null; - // Site-local tracking store - // backing Tracking.Status(id). Singleton; null in tests / hosts - // that haven't wired the store, which the helper handles by - // throwing on access. - IOperationTrackingStore? operationTrackingStore = null; - // Site-side cached-call - // telemetry forwarder. Singleton bound to the AuditLog - // composition root; null in tests / hosts that haven't called - // AddAuditLog, in which case the cached-call helpers degrade - // to the no-emission path (the underlying S&F handoff still - // happens and a TrackedOperationId is still returned). - ICachedCallTelemetryForwarder? cachedForwarder = null; - // SourceNode-stamping: the local node name - // resolved from INodeIdentityProvider — node-a/node-b on site - // hosts. Null in tests / hosts that haven't registered the - // provider, in which case NotificationSubmit.SourceNode and - // SiteCallOperational.SourceNode stay null and central - // persists the rows with SourceNode NULL. - string? sourceNode = null; - - if (serviceProvider != null) - { - serviceScope = serviceProvider.CreateScope(); - externalSystemClient = serviceScope.ServiceProvider.GetService(); - databaseGateway = serviceScope.ServiceProvider.GetService(); - storeAndForward = serviceScope.ServiceProvider.GetService(); - siteId = serviceScope.ServiceProvider.GetService()?.SiteId - ?? string.Empty; - auditWriter = serviceScope.ServiceProvider.GetService(); - operationTrackingStore = serviceScope.ServiceProvider.GetService(); - cachedForwarder = serviceScope.ServiceProvider.GetService(); - sourceNode = serviceScope.ServiceProvider.GetService()?.NodeName; - } - - var context = new ScriptRuntimeContext( - instanceActor, - self, - sharedScriptLibrary, - callDepth, - options.MaxScriptCallDepth, - timeout, - instanceName, - logger, - externalSystemClient, - databaseGateway, - storeAndForward, - siteCommunicationActor, - siteId, - // Notification Outbox (FU3): stamp the executing script onto outbound - // notifications using the Site Event Logging "Source" convention. - sourceScript: $"ScriptActor:{scriptName}", - // Emit one ApiOutbound/ApiCall row per - // ExternalSystem.Call. Writer is best-effort; failures are logged - // and swallowed inside the helper so the script's call path is - // never aborted by an audit failure. - auditWriter: auditWriter, - // Site-local tracking store - // backing Tracking.Status(id). Authoritative source of truth for - // cached-call status — read directly by the script API. - operationTrackingStore: operationTrackingStore, - // Cached-call telemetry - // forwarder for ExternalSystem.CachedCall / Database.CachedWrite - // CachedSubmit emission + the immediate-success terminal-row - // emission. Best-effort: null degrades the helpers to a - // no-emission path; the S&F handoff and TrackedOperationId - // return are unaffected. - cachedForwarder: cachedForwarder, - // The spawning execution's - // id for an inbound-API-routed call. The routed script still - // mints its own fresh ExecutionId — this records the spawner. - // Null for normal (tag-change / timer) runs. - parentExecutionId: parentExecutionId, - // SourceNode-stamping: the local node name - // (node-a/node-b on a site) — threaded down so Notify.Send - // and the four cached-call telemetry constructors can stamp - // it onto NotificationSubmit.SourceNode and - // SiteCallOperational.SourceNode respectively. - sourceNode: sourceNode, - // Thread the singleton site event logger so - // recursion-limit violations at CallScript/CallShared emit a - // script Error site event in addition to ILogger.LogError. - siteEventLogger: siteEventLogger, - // WaitForAttribute (spec §4.3/§4.4): thread the per-script - // execution-timeout token so Attributes.WaitAsync's Ask is - // bounded by the script's own ExecutionTimeoutSeconds — a - // shorter script deadline wins over the wait's own timeout. - scriptTimeoutToken: cts.Token); - - var globals = new ScriptGlobals - { - Instance = context, - Parameters = new ScriptParameters(parameters ?? new Dictionary()), - CancellationToken = cts.Token, - Scope = scope - }; - - // Operational `script` event — execution started. Fire-and-forget - // (the `_ =` discards the task) so the event log can never block or - // fault the script's own run; mirrors the existing Error-path emit. - _ = siteEventLogger?.LogEventAsync( - "script", "Info", instanceName, $"ScriptActor:{scriptName}", - $"Script '{scriptName}' on instance '{instanceName}' started"); - - var state = await compiledScript.RunAsync(globals, cts.Token); - - // Send result to requester if this was an Ask-based call - if (!replyTo.IsNobody()) - { - replyTo.Tell(new ScriptCallResult(correlationId, true, state.ReturnValue, null)); - } - - // Operational `script` event — execution completed successfully. - _ = siteEventLogger?.LogEventAsync( - "script", "Info", instanceName, $"ScriptActor:{scriptName}", - $"Script '{scriptName}' on instance '{instanceName}' completed"); - - // Notify parent of completion - parent.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, true, null)); - } - catch (OperationCanceledException) - { - healthCollector?.IncrementScriptError(); - var errorMsg = $"Script '{scriptName}' on instance '{instanceName}' timed out after {timeout.TotalSeconds}s"; - logger.LogWarning(errorMsg); - - // Failures recorded to site event log; script NOT disabled after failure. - _ = siteEventLogger?.LogEventAsync( - "script", "Error", instanceName, $"ScriptActor:{scriptName}", errorMsg); - - if (!replyTo.IsNobody()) - { - replyTo.Tell(new ScriptCallResult(correlationId, false, null, errorMsg)); - } - - parent.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, false, errorMsg)); - } - catch (Exception ex) - { - healthCollector?.IncrementScriptError(); - // Failures recorded to site event log; script NOT disabled after failure. - var errorMsg = $"Script '{scriptName}' on instance '{instanceName}' failed: {ex.Message}"; - logger.LogError(ex, "Script execution failed: {Script} on {Instance}", scriptName, instanceName); - - _ = siteEventLogger?.LogEventAsync( - "script", "Error", instanceName, $"ScriptActor:{scriptName}", errorMsg, ex.ToString()); - - if (!replyTo.IsNobody()) - { - replyTo.Tell(new ScriptCallResult(correlationId, false, null, errorMsg)); - } - - parent.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, false, errorMsg)); - } - finally - { - // Mark the body finished so the stuck-script watchdog (registered on - // cts.Token above) treats a timely cancellation as NOT stuck. - Interlocked.Exchange(ref completed, 1); - // Dispose the DI scope (and scoped services) after script execution completes - serviceScope?.Dispose(); - // Stop self after execution completes - self.Tell(PoisonPill.Instance); - } - }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler).Unwrap(); - } -} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs index 8c66777a..eebbaa34 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs @@ -3,30 +3,106 @@ using System.Collections.Concurrent; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; /// -/// A dedicated, bounded for running script +/// The outcome of a attempt. +/// +public enum WorkerDetachOutcome +{ + /// + /// The recorded slot/run pair no longer identifies a running task (the script finished, + /// or it never held a worker because it hopped threads on an await). Nothing was + /// detached and nothing needed to be — the worker is not lost. + /// + NotRunning, + + /// The worker was marked detached and a replacement thread was started. + Detached, + + /// + /// The number of live detached threads already equals the pool size, so no replacement was + /// started. Bounded starvation is preferable to unbounded thread growth when scripts wedge + /// en masse; the caller is expected to log this loudly. + /// + AtCap +} + +/// +/// A dedicated, grow-only for running script /// and alarm on-trigger bodies. /// /// Script bodies may perform synchronous blocking I/O (a database connection, a /// synchronous external-system call). Running them on the shared .NET /// lets a burst of blocking scripts starve the pool and stall -/// unrelated Akka dispatchers and HTTP request handling. This scheduler owns a fixed set +/// unrelated Akka dispatchers and HTTP request handling. This scheduler owns a set /// of dedicated threads, so script blocking is contained to those threads and cannot /// exhaust the global pool. /// +/// WP3.1 changed three things about that pool: +/// +/// It is no longer fixed-size. grows it towards +/// (instance-scaled, clamped between the configured +/// floor and ceiling). It is deliberately grow-only: undeploying instances +/// leaves idle threads, which cost nothing measurable and avoid drain/steal complexity. +/// A worker whose task has wedged in uninterruptible blocking I/O can be +/// detached and replaced, so a stuck script no longer +/// permanently costs the pool a thread. Live detached threads are capped at the pool size. +/// Trigger-expression evaluations no longer run here at all — they are non-blocking +/// by construction and now run on the shared thread pool behind +/// , so an alarm's Expression trigger can never queue behind +/// eight blocking script bodies (finding #4). +/// +/// /// The scheduler is process-wide (one set of threads for all instances) and is sized /// from the first time it is configured. /// public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable { + /// + /// Deployed instances per script-execution thread used by . + /// A named constant rather than an option: no deployment needs to tune the ratio + /// independently of the floor () + /// and the ceiling (). + /// + internal const int InstancesPerScriptThread = 8; + + /// Thread-name prefix; also the inlining guard in . + private const string ThreadNamePrefix = "script-execution-"; + private readonly BlockingCollection _queue = new(); - private readonly List _threads; + + /// + /// Immutable-on-read snapshot array of worker slots. Growth (EnsureCapacity, detach + /// replacement) publishes a NEW longer array under ; existing + /// slot objects are carried over by reference so a worker's index stays stable for its + /// whole life. Readers take one volatile read and then work on that snapshot. + /// + private volatile WorkerSlot[] _slots; + + /// Guards every mutation of , , and the detach bookkeeping. + private readonly object _growLock = new(); + + /// + /// Number of NON-detached workers — i.e. the pool's nominal size. Unchanged by a + /// detach (each detach starts a replacement), grown only by . + /// + private int _configuredCount; + + /// Detached workers that have not yet finished their wedged task and exited. + private int _detachedLive; + + /// Monotonic run identity; see . + private long _runStampSeed; + private int _disposed; - // Per-worker "busy since" timestamp (Environment.TickCount64 ms) while a task - // is executing on that worker, 0 when idle. Written by the worker thread, - // read (lock-free) by the observability gauges below. S2/UA5: makes a - // saturated or stuck script-execution pool visible on the site health report. - private readonly long[] _busySinceTicks; + /// + /// The slot index of the script-execution worker running the current thread, or null on + /// any other thread. Captured by the first synchronous segment of a script body so the + /// stuck-script watchdog can identify — and replace — the exact worker a wedged script + /// is holding. A script that hopped threads on an await is no longer on its worker, + /// which is correct: it holds no thread and must not cause a detach. + /// + [ThreadStatic] + internal static int? CurrentWorkerSlot; private static volatile ScriptExecutionScheduler? _shared; private static readonly object SharedLock = new(); @@ -34,8 +110,9 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable /// /// The process-wide script-execution scheduler, used as the default when no scheduler /// is injected. Lazily created on first use with the thread count from - /// ; the first caller wins, - /// subsequent calls reuse the existing instance. + /// (the floor — the Deployment + /// Manager grows it from there as instances deploy); the first caller wins, subsequent + /// calls reuse the existing instance. /// /// If the cached instance has been disposed it is recreated rather than handed back: /// a disposed scheduler can execute no work, so returning it would silently poison @@ -59,27 +136,40 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable } } + /// + /// Pure sizing function for the blocking script-execution pool: the configured floor, + /// raised to one thread per enabled instances, + /// clamped to the configured ceiling. Static and side-effect-free so the policy is + /// unit-testable without starting a single thread. + /// + /// Number of currently-running (enabled) Instance Actors. + /// Site runtime options supplying the floor and ceiling. + /// The target worker-thread count, always at least 1. + public static int ComputeTargetThreads(int enabledInstances, SiteRuntimeOptions options) + { + var floor = Math.Max(1, options.ScriptExecutionThreadCount); + // A ceiling below the floor is rejected by SiteRuntimeOptionsValidator; clamp here + // too so a directly-constructed options object can never invert the range. + var ceiling = Math.Max(floor, options.ScriptExecutionMaxThreadCount); + var scaled = enabledInstances <= 0 + ? 0 + : (int)Math.Ceiling(enabledInstances / (double)InstancesPerScriptThread); + return Math.Clamp(Math.Max(floor, scaled), 1, ceiling); + } + /// /// Creates a scheduler backed by dedicated threads. /// - /// Number of dedicated worker threads to create. + /// Initial number of dedicated worker threads to create. public ScriptExecutionScheduler(int threadCount) { if (threadCount < 1) threadCount = 1; - _busySinceTicks = new long[threadCount]; - _threads = new List(threadCount); - for (var i = 0; i < threadCount; i++) + _slots = []; + lock (_growLock) { - var index = i; // capture per-worker slot index - var thread = new Thread(() => WorkerLoop(index)) - { - IsBackground = true, - Name = $"script-execution-{i}" - }; - _threads.Add(thread); - thread.Start(); + GrowTo(threadCount); } } @@ -88,19 +178,28 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable public bool IsDisposed => Volatile.Read(ref _disposed) != 0; /// - public override int MaximumConcurrencyLevel => _threads.Count; + public override int MaximumConcurrencyLevel => Volatile.Read(ref _configuredCount); /// Number of tasks waiting in the queue (not counting those currently executing). public int QueueDepth => _queue.Count; - /// Number of worker threads currently executing a task. + /// + /// Workers whose task wedged past its deadline plus the stuck-script grace, which have been + /// detached and replaced but have not yet returned. Surfaced on the site health report as + /// DetachedScriptThreads: a non-zero, non-draining value means script bodies are + /// permanently blocking threads. + /// + public int DetachedThreadCount => Volatile.Read(ref _detachedLive); + + /// Number of worker threads currently executing a task (detached workers included — they really are busy). public int BusyThreadCount { get { + var slots = _slots; var count = 0; - for (var i = 0; i < _busySinceTicks.Length; i++) - if (Volatile.Read(ref _busySinceTicks[i]) != 0) count++; + foreach (var slot in slots) + if (Volatile.Read(ref slot.BusySinceTicks) != 0) count++; return count; } } @@ -115,10 +214,11 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable get { var now = Environment.TickCount64; + var slots = _slots; long oldestSince = 0; - for (var i = 0; i < _busySinceTicks.Length; i++) + foreach (var slot in slots) { - var since = Volatile.Read(ref _busySinceTicks[i]); + var since = Volatile.Read(ref slot.BusySinceTicks); if (since != 0 && (oldestSince == 0 || since < oldestSince)) oldestSince = since; } @@ -126,20 +226,138 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable } } + /// + /// Grows the pool to non-detached workers. Idempotent and + /// never shrinking: a target at or below the current size is a no-op. Called from + /// DeploymentManagerActor.UpdateInstanceCounts on every deploy/undeploy/enable/ + /// disable and per staggered startup batch. + /// + /// Desired number of non-detached worker threads. + /// The pool's non-detached worker count after the call. + public int EnsureCapacity(int target) + { + if (IsDisposed) return Volatile.Read(ref _configuredCount); + if (target <= Volatile.Read(ref _configuredCount)) return Volatile.Read(ref _configuredCount); + + lock (_growLock) + { + if (IsDisposed || target <= _configuredCount) return _configuredCount; + GrowTo(target - _configuredCount); + return _configuredCount; + } + } + + /// + /// The identity stamp of the task currently running on , or 0 when + /// that worker is idle. Captured alongside the slot index at script-body start and handed + /// back to , which detaches only if the SAME task is still + /// running. A monotonic counter rather than a timestamp: two runs on one worker inside the + /// same tick would otherwise be indistinguishable. + /// + /// The worker slot index. + /// The current run stamp, or 0 when the slot is idle or out of range. + internal long CurrentRunStamp(int slot) + { + var slots = _slots; + return slot >= 0 && slot < slots.Length ? Volatile.Read(ref slots[slot].RunStamp) : 0L; + } + + /// + /// Detaches the worker at — if it is still running the task + /// identified by — and starts a replacement thread, so + /// a script wedged in uninterruptible blocking I/O no longer costs the pool a thread + /// permanently. The detached worker exits (instead of pulling more work) as soon as its + /// wedged task finally returns, so capacity never silently doubles-and-drains. + /// + /// The worker slot recorded when the script body started. + /// The run stamp recorded at the same moment. + /// What was done; see . + internal WorkerDetachOutcome TryDetachWorker(int slot, long observedRunStamp) + { + if (observedRunStamp == 0 || IsDisposed) return WorkerDetachOutcome.NotRunning; + + lock (_growLock) + { + if (IsDisposed) return WorkerDetachOutcome.NotRunning; + + var slots = _slots; + if (slot < 0 || slot >= slots.Length) return WorkerDetachOutcome.NotRunning; + + var worker = slots[slot]; + // Same task still running on that exact worker? If the stamp moved (or went to 0) + // the script finished or never held this thread — there is nothing lost to replace. + if (Volatile.Read(ref worker.RunStamp) != observedRunStamp) return WorkerDetachOutcome.NotRunning; + if (Volatile.Read(ref worker.Detached) != 0) return WorkerDetachOutcome.NotRunning; + + // Bound: never hold more than 2x threads (N wedged + N live). + if (_detachedLive >= _configuredCount) return WorkerDetachOutcome.AtCap; + + Volatile.Write(ref worker.Detached, 1); + _detachedLive++; + _configuredCount--; // the detached worker no longer counts towards the pool … + GrowTo(1); // … and GrowTo puts the count back by starting its replacement. + return WorkerDetachOutcome.Detached; + } + } + + /// Appends new worker slots + threads. Caller holds . + private void GrowTo(int count) + { + var existing = _slots; + var grown = new WorkerSlot[existing.Length + count]; + Array.Copy(existing, grown, existing.Length); + + for (var i = 0; i < count; i++) + { + var index = existing.Length + i; + grown[index] = new WorkerSlot(); + } + + // Publish the array BEFORE starting the threads: a worker's very first action is to + // index its own slot, which must already be visible on the published snapshot. + _slots = grown; + _configuredCount += count; + + for (var i = 0; i < count; i++) + { + var index = existing.Length + i; + var thread = new Thread(() => WorkerLoop(index)) + { + IsBackground = true, + Name = ThreadNamePrefix + index + }; + grown[index].Thread = thread; + thread.Start(); + } + } + private void WorkerLoop(int index) { + CurrentWorkerSlot = index; + var slot = _slots[index]; try { foreach (var task in _queue.GetConsumingEnumerable()) { - Volatile.Write(ref _busySinceTicks[index], Environment.TickCount64); + Volatile.Write(ref slot.RunStamp, Interlocked.Increment(ref _runStampSeed)); + Volatile.Write(ref slot.BusySinceTicks, Environment.TickCount64); try { TryExecuteTask(task); } finally { - Volatile.Write(ref _busySinceTicks[index], 0); + Volatile.Write(ref slot.BusySinceTicks, 0); + Volatile.Write(ref slot.RunStamp, 0); + } + + // Detached while this task ran: a replacement worker is already live, so + // exit rather than pulling more work — otherwise capacity would silently + // double once the wedged script finally returned. + if (Volatile.Read(ref slot.Detached) != 0) + { + Interlocked.Decrement(ref _detachedLive); + return; } } } @@ -147,6 +365,10 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable { // Scheduler disposed — worker exits. } + finally + { + CurrentWorkerSlot = null; + } } /// @@ -157,7 +379,7 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable { // Only inline if we are already on one of this scheduler's worker threads, // so script work never escapes onto a thread-pool thread. - if (Thread.CurrentThread.Name?.StartsWith("script-execution-", StringComparison.Ordinal) != true) + if (Thread.CurrentThread.Name?.StartsWith(ThreadNamePrefix, StringComparison.Ordinal) != true) return false; return TryExecuteTask(task); @@ -173,8 +395,35 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable return; _queue.CompleteAdding(); - foreach (var thread in _threads) - thread.Join(TimeSpan.FromSeconds(5)); + foreach (var slot in _slots) + slot.Thread?.Join(TimeSpan.FromSeconds(5)); _queue.Dispose(); } + + /// + /// Per-worker bookkeeping. One instance per slot index, created once and carried by + /// reference across every growth so a worker's state survives + /// the array being replaced. + /// + private sealed class WorkerSlot + { + /// + /// at which the worker picked up its current + /// task, 0 when idle. Written by the worker, read lock-free by the gauges. + /// + public long BusySinceTicks; + + /// + /// Monotonic identity of the task currently running on this worker, 0 when idle. + /// Distinguishes two runs that start inside the same clock tick, which + /// alone cannot. + /// + public long RunStamp; + + /// Non-zero once the worker has been detached; it exits after its current task. + public int Detached; + + /// The worker thread, for to join. + public Thread? Thread; + } } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs new file mode 100644 index 00000000..4efc16cf --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs @@ -0,0 +1,570 @@ +using Akka.Actor; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Types; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Scripts; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteEventLogging; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.StoreAndForward; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +/// +/// WP3.1: launches one script or alarm on-trigger run directly from its owning coordinator +/// ( / ), replacing the short-lived +/// ScriptExecutionActor and AlarmExecutionActor. +/// +/// Those actors were already inert shells: neither declared a single Receive +/// handler (they executed from their constructor), neither had a PostStop, state, or +/// stash, and their IActorRef was never a message target — the whole lifecycle lived +/// inside a detached the actor never observed. What they cost was a real +/// actor cell, mailbox, and name registration per run, plus a per-spawn expression-tree +/// Props.Create. Removing them changes no semantics; the run body below is the former +/// ExecuteScript/ExecuteAlarmScript body, unified. +/// +/// Two behaviours DID change, both deliberately: +/// +/// The deadline is now armed by the CALLER, +/// before the body is queued to the , so queue wait +/// consumes the script's own budget. A body that dequeues past its deadline skips execution +/// entirely and takes the timeout path — a saturated pool sheds stale work instead of +/// running it late. +/// The stuck-script watchdog now DETACHES and replaces the worker thread a wedged +/// script is holding () instead of +/// only naming it, so the pool recovers its capacity. +/// +/// +/// Ordering, supervision, telemetry, DI scoping, Ask replies, completion messages, and +/// the audit ExecutionId/ParentExecutionId threading are all unchanged. The +/// launch call itself can throw only if the target scheduler is unusable (e.g. disposed); the +/// caller wraps it so that failure replies to the Ask caller and decrements the in-flight +/// counter rather than escalating to the coordinator's supervisor. +/// +internal static class ScriptRunLauncher +{ + /// + /// Launches an instance script run. Returns as soon as the body is queued; the run reports + /// completion to . + /// + /// Name of the script being executed. + /// Name of the instance that owns the script. + /// Compiled Roslyn script to execute. + /// Optional named parameter values for the script. + /// Current call-nesting depth (used to enforce the max-depth limit). + /// Instance actor reference for attribute access. + /// Library of shared scripts available during execution. + /// Site runtime options applied during execution. + /// Actor reference that receives the script result; Nobody for fire-and-forget. + /// Application-level correlation id threaded through the execution. + /// The owning , which receives the completion message. + /// Site communication actor (resolved on the actor thread) for Notify.Status. + /// Logger for script execution events. + /// Script scope controlling which APIs are available. + /// Per-script run counter, used only in log messages (replaces the former per-run actor name). + /// Optional health collector for recording execution metrics. + /// Optional DI service provider for script execution services. + /// ExecutionId of the spawning execution for audit correlation; null for root runs. + /// Per-script execution timeout in seconds. Null or non-positive falls back to the global value. + /// Script-execution scheduler seam (#18); null selects the process-wide shared instance. + public static void LaunchScript( + string scriptName, + string instanceName, + Script compiledScript, + IReadOnlyDictionary? parameters, + int callDepth, + IActorRef instanceActor, + SharedScriptLibrary sharedScriptLibrary, + SiteRuntimeOptions options, + IActorRef replyTo, + string correlationId, + IActorRef completionTarget, + ICanTell? siteCommunicationActor, + ILogger logger, + ScriptScope scope, + long runId, + ISiteHealthCollector? healthCollector, + IServiceProvider? serviceProvider, + Guid? parentExecutionId, + int? executionTimeoutSeconds, + ScriptExecutionScheduler? scheduler) + { + var timeout = ResolveTimeout(executionTimeoutSeconds, options); + var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); + + // Armed HERE, on the caller's (actor) thread, not inside the queued body: queue wait + // must consume the script's own budget, otherwise a saturated pool silently grants + // every backlogged run a fresh full timeout. + var cts = new CancellationTokenSource(timeout); + try + { + _ = Task.Factory.StartNew( + () => RunScriptAsync( + scriptName, instanceName, compiledScript, parameters, callDepth, + instanceActor, sharedScriptLibrary, options, replyTo, correlationId, + completionTarget, siteCommunicationActor, logger, scope, runId, + healthCollector, serviceProvider, parentExecutionId, timeout, + executionScheduler, cts), + CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler) + .Unwrap(); + } + catch + { + // The body never got queued, so nothing will ever dispose the CTS. + cts.Dispose(); + throw; + } + } + + /// + /// Launches an alarm on-trigger run. Same contract as , with the + /// firing alarm's level/priority/message exposed to the body through the Alarm global. + /// + /// The canonical name of the alarm that triggered. + /// The name of the owning instance. + /// The alarm severity level at the time of triggering. + /// The alarm priority value. + /// The alarm message to pass to the script. + /// The pre-compiled on-trigger script to execute. + /// Reference to the instance actor for attribute/script calls. + /// Shared script library providing common utilities. + /// Site runtime configuration options, including the execution timeout. + /// The owning , which receives the completion message. + /// Logger for execution diagnostics. + /// Per-alarm run counter, used only in log messages. + /// The on-trigger script's per-script timeout in seconds. Null or non-positive falls back to the global value. + /// + /// ParentExecutionId tag-cascade: the ExecutionId of the execution whose attribute + /// write fired this alarm. Null when the firing value came from the Data Connection Layer + /// (external data has no spawning execution) — that on-trigger run is a tree ROOT. + /// + /// Script-execution scheduler seam (#18); null selects the process-wide shared instance. + public static void LaunchAlarmScript( + string alarmName, + string instanceName, + AlarmLevel level, + int priority, + string message, + Script compiledScript, + IActorRef instanceActor, + SharedScriptLibrary sharedScriptLibrary, + SiteRuntimeOptions options, + IActorRef completionTarget, + ILogger logger, + long runId, + int? executionTimeoutSeconds, + Guid? parentExecutionId, + ScriptExecutionScheduler? scheduler) + { + var timeout = ResolveTimeout(executionTimeoutSeconds, options); + var executionScheduler = scheduler ?? ScriptExecutionScheduler.Shared(options); + + // Enqueue-anchored deadline; see LaunchScript. + var cts = new CancellationTokenSource(timeout); + try + { + _ = Task.Factory.StartNew( + () => RunAlarmScriptAsync( + alarmName, instanceName, level, priority, message, compiledScript, + instanceActor, sharedScriptLibrary, options, completionTarget, logger, + runId, parentExecutionId, timeout, cts), + CancellationToken.None, TaskCreationOptions.DenyChildAttach, executionScheduler) + .Unwrap(); + } + catch + { + cts.Dispose(); + throw; + } + } + + /// + /// Per-script timeout overrides the global default. A null or non-positive per-script + /// value (≤ 0) falls back to the global. + /// + private static TimeSpan ResolveTimeout(int? executionTimeoutSeconds, SiteRuntimeOptions options) + => TimeSpan.FromSeconds( + executionTimeoutSeconds is { } perScript && perScript > 0 + ? perScript + : options.ScriptExecutionTimeoutSeconds); + + private static async Task RunScriptAsync( + string scriptName, + string instanceName, + Script compiledScript, + IReadOnlyDictionary? parameters, + int callDepth, + IActorRef instanceActor, + SharedScriptLibrary sharedScriptLibrary, + SiteRuntimeOptions options, + IActorRef replyTo, + string correlationId, + IActorRef completionTarget, + ICanTell? siteCommunicationActor, + ILogger logger, + ScriptScope scope, + long runId, + ISiteHealthCollector? healthCollector, + IServiceProvider? serviceProvider, + Guid? parentExecutionId, + TimeSpan timeout, + ScriptExecutionScheduler executionScheduler, + CancellationTokenSource cts) + { + IServiceScope? serviceScope = null; + // ISiteEventLogger is a singleton; resolve from the root provider so + // it is available to the catch blocks regardless of scope state. + var siteEventLogger = serviceProvider?.GetService(); + + // WP3.1: identify the worker this body is holding, so the watchdog can detach and + // replace it if the body wedges. Read in the FIRST synchronous segment — a script + // that later hops threads on an await no longer holds this worker, and the run-stamp + // guard in TryDetachWorker will (correctly) refuse to detach it. + var workerSlot = ScriptExecutionScheduler.CurrentWorkerSlot; + var workerRunStamp = workerSlot is { } slot ? executionScheduler.CurrentRunStamp(slot) : 0L; + + var completed = 0; + ArmStuckScriptWatchdog( + cts, options, logger, siteEventLogger, executionScheduler, + workerSlot, workerRunStamp, timeout, + () => Volatile.Read(ref completed) != 0, + $"Script '{scriptName}' on instance '{instanceName}'", + instanceName, $"ScriptActor:{scriptName}"); + + try + { + // WP3.1 shed-at-dequeue: the deadline was armed at ENQUEUE, so an already-cancelled + // token here means this run spent its entire budget queueing. Running it now would + // burn a scarce script thread on work that is already stale — throw straight into + // the existing timeout path instead, BEFORE the "started" event and before the body. + cts.Token.ThrowIfCancellationRequested(); + + // Resolve integration services from DI (scoped lifetime) + IExternalSystemClient? externalSystemClient = null; + IDatabaseGateway? databaseGateway = null; + // Notification Outbox: the S&F engine is a singleton; the site identity + // provider supplies the site id stamped on enqueued notifications. + StoreAndForwardService? storeAndForward = null; + var siteId = string.Empty; + // The writer is a singleton (FallbackAuditWriter + // composes the SQLite hot-path + drop-oldest ring); null in tests / hosts + // that haven't called AddAuditLog, which the helper handles as a no-op. + IAuditWriter? auditWriter = null; + // Site-local tracking store + // backing Tracking.Status(id). Singleton; null in tests / hosts + // that haven't wired the store, which the helper handles by + // throwing on access. + IOperationTrackingStore? operationTrackingStore = null; + // Site-side cached-call + // telemetry forwarder. Singleton bound to the AuditLog + // composition root; null in tests / hosts that haven't called + // AddAuditLog, in which case the cached-call helpers degrade + // to the no-emission path (the underlying S&F handoff still + // happens and a TrackedOperationId is still returned). + ICachedCallTelemetryForwarder? cachedForwarder = null; + // SourceNode-stamping: the local node name + // resolved from INodeIdentityProvider — node-a/node-b on site + // hosts. Null in tests / hosts that haven't registered the + // provider, in which case NotificationSubmit.SourceNode and + // SiteCallOperational.SourceNode stay null and central + // persists the rows with SourceNode NULL. + string? sourceNode = null; + + if (serviceProvider != null) + { + serviceScope = serviceProvider.CreateScope(); + externalSystemClient = serviceScope.ServiceProvider.GetService(); + databaseGateway = serviceScope.ServiceProvider.GetService(); + storeAndForward = serviceScope.ServiceProvider.GetService(); + siteId = serviceScope.ServiceProvider.GetService()?.SiteId + ?? string.Empty; + auditWriter = serviceScope.ServiceProvider.GetService(); + operationTrackingStore = serviceScope.ServiceProvider.GetService(); + cachedForwarder = serviceScope.ServiceProvider.GetService(); + sourceNode = serviceScope.ServiceProvider.GetService()?.NodeName; + } + + var context = new ScriptRuntimeContext( + instanceActor, + sharedScriptLibrary, + callDepth, + options.MaxScriptCallDepth, + timeout, + instanceName, + logger, + externalSystemClient, + databaseGateway, + storeAndForward, + siteCommunicationActor, + siteId, + // Notification Outbox (FU3): stamp the executing script onto outbound + // notifications using the Site Event Logging "Source" convention. + sourceScript: $"ScriptActor:{scriptName}", + // Emit one ApiOutbound/ApiCall row per + // ExternalSystem.Call. Writer is best-effort; failures are logged + // and swallowed inside the helper so the script's call path is + // never aborted by an audit failure. + auditWriter: auditWriter, + // Site-local tracking store + // backing Tracking.Status(id). Authoritative source of truth for + // cached-call status — read directly by the script API. + operationTrackingStore: operationTrackingStore, + // Cached-call telemetry + // forwarder for ExternalSystem.CachedCall / Database.CachedWrite + // CachedSubmit emission + the immediate-success terminal-row + // emission. Best-effort: null degrades the helpers to a + // no-emission path; the S&F handoff and TrackedOperationId + // return are unaffected. + cachedForwarder: cachedForwarder, + // The spawning execution's + // id for an inbound-API-routed call. The routed script still + // mints its own fresh ExecutionId — this records the spawner. + // Null for normal (tag-change / timer) runs. + parentExecutionId: parentExecutionId, + // SourceNode-stamping: the local node name + // (node-a/node-b on a site) — threaded down so Notify.Send + // and the four cached-call telemetry constructors can stamp + // it onto NotificationSubmit.SourceNode and + // SiteCallOperational.SourceNode respectively. + sourceNode: sourceNode, + // Thread the singleton site event logger so + // recursion-limit violations at CallScript/CallShared emit a + // script Error site event in addition to ILogger.LogError. + siteEventLogger: siteEventLogger, + // WaitForAttribute (spec §4.3/§4.4): thread the per-script + // execution-timeout token so Attributes.WaitAsync's Ask is + // bounded by the script's own ExecutionTimeoutSeconds — a + // shorter script deadline wins over the wait's own timeout. + scriptTimeoutToken: cts.Token); + + var globals = new ScriptGlobals + { + Instance = context, + Parameters = new ScriptParameters(parameters ?? new Dictionary()), + CancellationToken = cts.Token, + Scope = scope + }; + + // Operational `script` event — execution started. Fire-and-forget + // (the `_ =` discards the task) so the event log can never block or + // fault the script's own run; mirrors the existing Error-path emit. + _ = siteEventLogger?.LogEventAsync( + "script", "Info", instanceName, $"ScriptActor:{scriptName}", + $"Script '{scriptName}' on instance '{instanceName}' started"); + + var state = await compiledScript.RunAsync(globals, cts.Token); + + // Send result to requester if this was an Ask-based call + if (!replyTo.IsNobody()) + { + replyTo.Tell(new ScriptCallResult(correlationId, true, state.ReturnValue, null)); + } + + // Operational `script` event — execution completed successfully. + _ = siteEventLogger?.LogEventAsync( + "script", "Info", instanceName, $"ScriptActor:{scriptName}", + $"Script '{scriptName}' on instance '{instanceName}' completed"); + + // Notify the owning ScriptActor of completion (also releases its in-flight slot). + completionTarget.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, true, null)); + } + catch (OperationCanceledException) + { + healthCollector?.IncrementScriptError(); + var errorMsg = $"Script '{scriptName}' on instance '{instanceName}' timed out after {timeout.TotalSeconds}s"; + logger.LogWarning("{Message} (run {RunId})", errorMsg, runId); + + // Failures recorded to site event log; script NOT disabled after failure. + _ = siteEventLogger?.LogEventAsync( + "script", "Error", instanceName, $"ScriptActor:{scriptName}", errorMsg); + + if (!replyTo.IsNobody()) + { + replyTo.Tell(new ScriptCallResult(correlationId, false, null, errorMsg)); + } + + completionTarget.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, false, errorMsg)); + } + catch (Exception ex) + { + healthCollector?.IncrementScriptError(); + // Failures recorded to site event log; script NOT disabled after failure. + var errorMsg = $"Script '{scriptName}' on instance '{instanceName}' failed: {ex.Message}"; + logger.LogError(ex, "Script execution failed: {Script} on {Instance} (run {RunId})", + scriptName, instanceName, runId); + + _ = siteEventLogger?.LogEventAsync( + "script", "Error", instanceName, $"ScriptActor:{scriptName}", errorMsg, ex.ToString()); + + if (!replyTo.IsNobody()) + { + replyTo.Tell(new ScriptCallResult(correlationId, false, null, errorMsg)); + } + + completionTarget.Tell(new ScriptActor.ScriptExecutionCompleted(scriptName, false, errorMsg)); + } + finally + { + // Mark the body finished so the stuck-script watchdog (registered on + // cts.Token above) treats a timely cancellation as NOT stuck. + Interlocked.Exchange(ref completed, 1); + // Dispose the DI scope (and scoped services) after script execution completes + serviceScope?.Dispose(); + cts.Dispose(); + } + } + + private static async Task RunAlarmScriptAsync( + string alarmName, + string instanceName, + AlarmLevel level, + int priority, + string message, + Script compiledScript, + IActorRef instanceActor, + SharedScriptLibrary sharedScriptLibrary, + SiteRuntimeOptions options, + IActorRef completionTarget, + ILogger logger, + long runId, + Guid? parentExecutionId, + TimeSpan timeout, + CancellationTokenSource cts) + { + try + { + // Enqueue-anchored deadline: an already-cancelled token means this on-trigger run + // spent its whole budget queueing behind other script bodies. Shed it rather than + // run it late — the alarm it would react to is already stale. + cts.Token.ThrowIfCancellationRequested(); + + // An alarm on-trigger run can call Instance.CallScript() + // via the ScriptRuntimeContext injected into globals + var context = new ScriptRuntimeContext( + instanceActor, + sharedScriptLibrary, + currentCallDepth: 0, + options.MaxScriptCallDepth, + timeout, + instanceName, + logger, + // ParentExecutionId tag-cascade: the + // alarm on-trigger run mints its own fresh ExecutionId (the + // ctor's `?? NewGuid()` fallback) and records the firing + // execution's id as its ParentExecutionId — null (a root) + // only when the firing value came from the DCL. + parentExecutionId: parentExecutionId, + // WaitForAttribute (spec §4.4): thread the alarm on-trigger + // script's per-script execution-timeout token so a + // Attributes.WaitAsync inside an on-trigger script is bounded + // by the same script deadline. + scriptTimeoutToken: cts.Token); + + var globals = new ScriptGlobals + { + Instance = context, + Parameters = new ScriptParameters(), + CancellationToken = cts.Token, + Alarm = new AlarmContext + { + Name = alarmName, + Level = level, + Priority = priority, + Message = message + } + }; + + await compiledScript.RunAsync(globals, cts.Token); + + completionTarget.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, true)); + } + catch (OperationCanceledException) + { + logger.LogWarning( + "Alarm on-trigger script for {Alarm} on {Instance} timed out (run {RunId})", + alarmName, instanceName, runId); + completionTarget.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, false)); + } + catch (Exception ex) + { + // Failures logged, alarm continues + logger.LogError(ex, + "Alarm on-trigger script for {Alarm} on {Instance} failed (run {RunId})", + alarmName, instanceName, runId); + completionTarget.Tell(new AlarmActor.AlarmExecutionCompleted(alarmName, false)); + } + finally + { + cts.Dispose(); + } + } + + /// + /// Arms the stuck-script watchdog (S2, extended by WP3.1). + /// + /// The CTS firing only REQUESTS cooperative cancellation; it does NOT free a thread + /// blocked in synchronous I/O. When the deadline elapses, this waits + /// and, if the body still has not + /// returned, (a) names the script loudly on the site event log and (b) DETACHES the worker + /// thread it is holding and starts a replacement — so a wedged script costs the pool one + /// thread only until the watchdog fires, not forever. At the detach cap it logs an Error + /// instead of growing threads without bound. + /// + /// The Task.Run/Task.Delay run on the shared thread pool, never on the + /// (possibly saturated) script scheduler — deliberate, and the whole point of the watchdog. + /// Register fires on cancellation only; a normally-completing run disposes its CTS + /// with no callback. A run cancelled while still QUEUED flips its completed flag in the + /// body's finally long before the grace elapses, so it is correctly not reported. + /// + private static void ArmStuckScriptWatchdog( + CancellationTokenSource cts, + SiteRuntimeOptions options, + ILogger logger, + ISiteEventLogger? siteEventLogger, + ScriptExecutionScheduler scheduler, + int? workerSlot, + long workerRunStamp, + TimeSpan timeout, + Func isCompleted, + string subject, + string instanceName, + string source) + { + var graceMs = options.StuckScriptGraceMs; + cts.Token.Register(() => _ = Task.Run(async () => + { + await Task.Delay(graceMs); + if (isCompleted()) return; + + var stuckMsg = $"{subject} exceeded its {timeout.TotalSeconds:F0}s timeout and is STILL EXECUTING — " + + "its dedicated script-execution thread is blocked (cooperative cancellation not observed)."; + + var outcome = workerSlot is { } slot + ? scheduler.TryDetachWorker(slot, workerRunStamp) + : WorkerDetachOutcome.NotRunning; + + switch (outcome) + { + case WorkerDetachOutcome.Detached: + stuckMsg += " The worker thread has been DETACHED and replaced; it will exit when the body returns."; + break; + case WorkerDetachOutcome.AtCap: + stuckMsg += $" The script-execution pool already holds {scheduler.DetachedThreadCount} detached " + + "stuck threads (at cap); NOT replacing this one."; + break; + case WorkerDetachOutcome.NotRunning: + // The body is not on a worker thread (it hopped threads on an await), so no + // dedicated thread is lost — report it, but there is nothing to replace. + break; + } + + logger.LogError(stuckMsg); + _ = siteEventLogger?.LogEventAsync("script", "Error", instanceName, source, stuckMsg); + })); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 06ea16c8..fa1b6a9c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -40,7 +40,6 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; public class ScriptRuntimeContext { private readonly IActorRef _instanceActor; - private readonly IActorRef _self; private readonly SharedScriptLibrary _sharedScriptLibrary; private readonly int _currentCallDepth; private readonly int _maxCallDepth; @@ -50,7 +49,7 @@ public class ScriptRuntimeContext /// /// WaitForAttribute (spec §4.3): the per-script execution-timeout token from - /// the owning ScriptExecutionActor/AlarmExecutionActor + /// the owning script or alarm on-trigger run /// (cts.Token). Bounds the Attributes.WaitAsync Ask so a script /// that hits its own ExecutionTimeoutSeconds abandons the wait. Defaults /// to for contexts that do not thread one @@ -170,7 +169,6 @@ public class ScriptRuntimeContext /// execution, external system calls, database access, and notification delivery. /// /// Reference to the Instance Actor managing this instance's state. - /// Reference to the executing script actor. /// Library containing shared scripts available to all instances. /// Current recursion depth of script calls. /// Maximum allowed recursion depth before an error is thrown. @@ -217,7 +215,6 @@ public class ScriptRuntimeContext /// public ScriptRuntimeContext( IActorRef instanceActor, - IActorRef self, SharedScriptLibrary sharedScriptLibrary, int currentCallDepth, int maxCallDepth, @@ -240,7 +237,6 @@ public class ScriptRuntimeContext CancellationToken scriptTimeoutToken = default) { _instanceActor = instanceActor; - _self = self; _sharedScriptLibrary = sharedScriptLibrary; _currentCallDepth = currentCallDepth; _maxCallDepth = maxCallDepth; @@ -257,7 +253,7 @@ public class ScriptRuntimeContext _operationTrackingStore = operationTrackingStore; _cachedForwarder = cachedForwarder; // SourceNode-stamping: the local node name read from - // INodeIdentityProvider at the ScriptExecutionActor; null when no + // INodeIdentityProvider at the launching script run; null when no // provider was wired so the downstream callsites pass null through // verbatim — leaving central SourceNode as NULL. _sourceNode = sourceNode; @@ -265,7 +261,7 @@ public class ScriptRuntimeContext // (ParentExecutionId): stored verbatim — no `?? NewGuid()` // fallback. A non-routed run legitimately has no parent and stays null. _parentExecutionId = parentExecutionId; - // Optional — null when not wired (tests / AlarmExecutionActor). + // Optional — null when not wired (tests / alarm on-trigger runs). _siteEventLogger = siteEventLogger; // WaitForAttribute (spec §4.3): default(CancellationToken) == None when // not threaded in — the WaitAsync Ask is then bounded only by its own timeout. @@ -302,7 +298,6 @@ public class ScriptRuntimeContext { return new ScriptRuntimeContext( _instanceActor, - _self, _sharedScriptLibrary, childCallDepth, _maxCallDepth, @@ -333,7 +328,7 @@ public class ScriptRuntimeContext /// /// Fire-and-forget emission of a script Error site event /// for a recursion-limit violation. Mirrors the call shape used by - /// ScriptExecutionActor's catch blocks. A fault from + /// the run's own catch blocks. A fault from /// the site-event logger is observed-and-dropped (best-effort) via /// ContinueWith(OnlyOnFaulted) — it never blocks or faults the /// _logger.LogError + throw path that follows. A null logger is a no-op. @@ -1135,7 +1130,7 @@ public class ScriptRuntimeContext SourceSite: _siteId, // SourceNode-stamping: the local node name // (node-a/node-b) — threaded through INodeIdentityProvider - // at the ScriptExecutionActor; null when no provider was + // at the launching script run; null when no provider was // wired so central persists SiteCalls.SourceNode as NULL. SourceNode: _sourceNode, Status: "Submitted", @@ -1254,7 +1249,7 @@ public class ScriptRuntimeContext SourceSite: _siteId, // SourceNode-stamping: the local node name // (node-a/node-b) — threaded through INodeIdentityProvider - // at the ScriptExecutionActor; null when no provider was + // at the launching script run; null when no provider was // wired so central persists SiteCalls.SourceNode as NULL. SourceNode: _sourceNode, Status: "Attempted", @@ -1322,7 +1317,7 @@ public class ScriptRuntimeContext SourceSite: _siteId, // SourceNode-stamping: the local node name // (node-a/node-b) — threaded through INodeIdentityProvider - // at the ScriptExecutionActor; null when no provider was + // at the launching script run; null when no provider was // wired so central persists SiteCalls.SourceNode as NULL. SourceNode: _sourceNode, Status: operationalTerminalStatus, @@ -1913,7 +1908,7 @@ public class ScriptRuntimeContext SourceSite: _siteId, // SourceNode-stamping: the local node name // (node-a/node-b) — threaded through INodeIdentityProvider - // at the ScriptExecutionActor; null when no provider was + // at the launching script run; null when no provider was // wired so central persists SiteCalls.SourceNode as NULL. SourceNode: _sourceNode, Status: "Submitted", @@ -2260,7 +2255,7 @@ public class ScriptRuntimeContext OriginParentExecutionId: _parentExecutionId, // SourceNode-stamping: the cluster node name on which this // notification was emitted (node-a/node-b). Stamped from the local - // INodeIdentityProvider via ScriptExecutionActor. Rides inside the + // INodeIdentityProvider via the launching script run. Rides inside the // serialized payload through the S&F buffer to central, where // NotificationOutboxActor persists it on the Notifications row. SourceNode: _sourceNode); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs index 16cf2bd6..943d81b8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptSchedulerStatsReporter.cs @@ -80,7 +80,9 @@ public sealed class ScriptSchedulerStatsReporter : BackgroundService _collector.SetScriptSchedulerStats( scheduler.QueueDepth, scheduler.BusyThreadCount, - scheduler.OldestBusyAge?.TotalSeconds); + scheduler.OldestBusyAge?.TotalSeconds, + // WP3.1: workers detached and replaced by the stuck-script watchdog. + scheduler.DetachedThreadCount); } catch (Exception ex) { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs index 0b025af8..768a6e9d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/SiteScriptCompileCache.cs @@ -26,19 +26,41 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; /// /// /// Bounded at entries — deliberately smaller than the verdict cache's -/// 4096 because entries pin compiled assemblies, not just verdict strings. On overflow the cache -/// is cleared wholesale (results are recomputable, so a coarse reset avoids eviction bookkeeping). -/// // are exposed for tests and diagnostics. +/// 4096 because entries pin compiled assemblies, not just verdict strings. /// +/// +/// WP3.1 — approximate LRU replaced the overflow cliff. The cache used to +/// Clear() wholesale on overflow. Instance scripts AND trigger expressions share this +/// cache, so on a site large enough to cross 1024 entries every overflow discarded up to 1023 +/// live compiled scripts and the next deploy or Instance-Actor start paid a full recompile +/// storm — on actor threads. Now an insert at the bound evicts only the oldest +/// ⅛ of entries by last-access stamp: overflow costs one +/// 1024-element scan instead of 1023 future recompiles, and hot entries survive. +/// +/// Recency is an access sequence, not a clock: it is +/// deterministic for tests and immune to clock steps. Hits update the stamp lock-free; the +/// only lock is the (rare) eviction sweep, double-checked so concurrent inserts do not +/// stampede it. /// internal static class SiteScriptCompileCache { - /// Upper bound on cached entries; the cache is cleared wholesale on overflow. + /// Upper bound on cached entries; crossing it evicts the oldest batch. internal const int MaxEntries = 1024; - private static readonly ConcurrentDictionary Cache = new(); + /// + /// Fraction of the cache evicted in one sweep (⅛ = 128 entries at + /// ). Batching amortises the O(n) scan across many inserts, so + /// steady-state churn does not re-scan on every single miss. + /// + private const int EvictionBatchDivisor = 8; + + private static readonly ConcurrentDictionary Cache = new(); + private static readonly object EvictionLock = new(); private static long _hits; + /// Monotonic access sequence; the recency stamp written onto entries. + private static long _accessSequence; + /// Number of cache hits observed since the last . public static long Hits => Interlocked.Read(ref _hits); @@ -48,8 +70,8 @@ internal static class SiteScriptCompileCache /// /// Returns the cached compile result for against /// , or computes it via and caches - /// it on a miss. A hit increments . Both success and failure results are - /// cached — the error text is name-free by construction. + /// it on a miss. A hit increments and refreshes the entry's recency. + /// Both success and failure results are cached — the error text is name-free by construction. /// /// The script source code to look up (hashed to form the cache key). /// The Roslyn globals surface the script compiles against; part of the key so identical source under different globals stays distinct. @@ -59,19 +81,19 @@ internal static class SiteScriptCompileCache { var key = globalsType.FullName + ":" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code))); - if (Cache.TryGetValue(key, out var result)) + if (Cache.TryGetValue(key, out var entry)) { Interlocked.Increment(ref _hits); - return result; + Touch(entry); + return entry.Result; } - result = factory(); + var result = factory(); - // Coarse bound: on overflow drop everything rather than track evictions. if (Cache.Count >= MaxEntries) - Cache.Clear(); + EvictOldestBatch(); - Cache[key] = result; + Cache[key] = new CacheEntry(result) { LastAccess = NextStamp() }; return result; } @@ -81,4 +103,53 @@ internal static class SiteScriptCompileCache Cache.Clear(); Interlocked.Exchange(ref _hits, 0); } + + /// Refreshes an entry's recency stamp. Lock-free — a lost race only costs accuracy, never correctness. + private static void Touch(CacheEntry entry) => Volatile.Write(ref entry.LastAccess, NextStamp()); + + private static long NextStamp() => Interlocked.Increment(ref _accessSequence); + + /// + /// Evicts the oldest / entries + /// by recency stamp. Double-checked under so several concurrent + /// inserts crossing the bound together perform ONE sweep rather than one each. + /// + private static void EvictOldestBatch() + { + lock (EvictionLock) + { + if (Cache.Count < MaxEntries) return; // another thread already swept + + var batch = Math.Max(1, MaxEntries / EvictionBatchDivisor); + + // ConcurrentDictionary.ToArray() takes an ATOMIC snapshot. Enumerating (or + // LINQ-ing) the dictionary directly does not: LINQ's ToArray picks the + // ICollection>.CopyTo fast path, which reads Count and then + // copies, and throws ArgumentException when a concurrent insert lands between + // the two. The eviction lock only excludes other EVICTORS — inserts run + // lock-free by design — so the snapshot must be the thread-safe one. + var victims = Cache.ToArray() + .OrderBy(kvp => Volatile.Read(ref kvp.Value.LastAccess)) + .Take(batch) + .Select(kvp => kvp.Key) + .ToArray(); + + foreach (var victim in victims) + Cache.TryRemove(victim, out _); + } + } + + /// + /// A cached compile result plus its recency stamp. A class (not a struct) so + /// can update recency in place without replacing the dictionary + /// value — the hot path stays a single volatile write. + /// + private sealed class CacheEntry(ScriptCompilationResult result) + { + /// The memoised compile result (success or failure). + public ScriptCompilationResult Result { get; } = result; + + /// Access sequence number of the most recent hit; older = evicted first. + public long LastAccess; + } } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerEvalGate.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerEvalGate.cs new file mode 100644 index 00000000..997effd5 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/TriggerEvalGate.cs @@ -0,0 +1,81 @@ +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +/// +/// WP3.1 (finding #4): the process-wide concurrency gate for trigger-expression evaluation. +/// +/// Before WP3.1, ScriptActor and AlarmActor evaluated their compiled +/// Expression triggers on the — the same bounded pool +/// of dedicated threads that runs script bodies. Eight scripts blocked in synchronous I/O +/// therefore stalled EVERY Expression trigger on the node behind them, for an unbounded time, +/// and the evaluation's own 2 s timeout did not even start until it was dequeued. An alarm +/// that should have raised in milliseconds simply never raised. +/// +/// Trigger expressions are non-blocking by construction: +/// exposes only reads over an in-memory snapshot +/// dictionary, and the script trust gate has already denied I/O, network, threading, and +/// reflection long before the expression can deploy. They are short CPU-bound work — exactly +/// what the shared .NET thread pool is for. So they run there, and this semaphore is the only +/// thing bounding their fan-out. A second dedicated pool was considered and rejected: it would +/// add threads, gauges, and a second starvation surface for no isolation gain. +/// +/// Per-trigger coalescing in the actors (one evaluation in flight, one pending) already +/// caps waiters at one per Expression trigger, so this gate's queue is bounded by trigger +/// count. +/// +/// Mirrors 's lazy-singleton plus injectable-seam +/// shape so tests can drive a gate of size 1 deterministically. +/// +public sealed class TriggerEvalGate : IDisposable +{ + private readonly SemaphoreSlim _gate; + + private static volatile TriggerEvalGate? _shared; + private static readonly object SharedLock = new(); + + /// Creates a gate admitting concurrent evaluations. + /// Maximum concurrent trigger-expression evaluations; values below 1 are clamped to 1. + public TriggerEvalGate(int maxConcurrency) + { + MaxConcurrency = Math.Max(1, maxConcurrency); + _gate = new SemaphoreSlim(MaxConcurrency, MaxConcurrency); + } + + /// The configured concurrency limit. + public int MaxConcurrency { get; } + + /// Free permits right now; 0 means the gate is saturated and new evaluations will queue. + public int AvailablePermits => _gate.CurrentCount; + + /// + /// The process-wide gate, used when no gate is injected. Lazily created from + /// ; the first caller wins. + /// + /// Site runtime options supplying the concurrency limit. + /// The shared gate instance. + public static TriggerEvalGate Shared(SiteRuntimeOptions options) + { + var existing = _shared; + if (existing is not null) return existing; + + lock (SharedLock) + { + return _shared ??= new TriggerEvalGate(options.TriggerEvalMaxConcurrency); + } + } + + /// + /// Waits for a permit. The caller passes the evaluation's own deadline token, which was + /// armed at ENQUEUE — so time spent queueing here burns the same budget the evaluation + /// itself would, and a saturated gate produces a timely cancellation (treated as + /// by both actors) instead of an unbounded stall. + /// + /// The evaluation's deadline token. + /// A task that completes when a permit is acquired. + public Task WaitAsync(CancellationToken cancellationToken) => _gate.WaitAsync(cancellationToken); + + /// Returns a permit. Must be called exactly once per successful . + public void Release() => _gate.Release(); + + /// + public void Dispose() => _gate.Dispose(); +} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs index ba2506f3..2fe65424 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptions.cs @@ -38,13 +38,64 @@ public class SiteRuntimeOptions public int StreamBufferSize { get; set; } = 1000; /// - /// Number of dedicated threads in the script-execution scheduler. + /// FLOOR for the number of dedicated threads in the script-execution scheduler. /// Script and alarm on-trigger bodies run on these threads instead of the shared /// .NET thread pool, so blocking script I/O cannot starve the global pool. + /// + /// WP3.1: this was a fixed size and is now the lower bound of a grow-only, + /// instance-scaled pool — see + /// . Existing + /// configurations keep exactly the previous behaviour at or below + /// ScriptExecutionThreadCount * InstancesPerScriptThread deployed instances. + /// /// Default: 8. /// public int ScriptExecutionThreadCount { get; set; } = 8; + /// + /// WP3.1: CEILING for the instance-scaled script-execution pool. The pool grows + /// towards ceil(enabledInstances / 8) threads but never past this value; beyond + /// it, is the real regulator. A 1 MB-stack + /// dedicated thread is cheap, so 32 (≈ 256 instances at the /8 ratio) is a generous + /// default. Must be greater than or equal to . + /// Default: 32. + /// + public int ScriptExecutionMaxThreadCount { get; set; } = 32; + + /// + /// WP3.1 (finding #4): maximum number of trigger-expression evaluations allowed to run + /// concurrently across the whole process. Trigger expressions are non-blocking by + /// construction ( exposes only reads over an + /// in-memory snapshot, and the script trust gate has already denied I/O, network, and + /// threading), so they run as plain async work on the shared .NET thread pool behind this + /// gate rather than on the blocking script-execution pool. That is what keeps an alarm's + /// Expression trigger from queueing behind blocking script bodies. + /// Default: max(2, Environment.ProcessorCount). + /// + public int TriggerEvalMaxConcurrency { get; set; } = Math.Max(2, Environment.ProcessorCount); + + /// + /// WP3.1: timeout (seconds) for a single trigger-expression evaluation. Previously + /// hardcoded at 2 s in both ScriptActor and AlarmActor. The deadline is now + /// armed when the evaluation is ENQUEUED, not when it is dequeued, so time spent waiting + /// on burns the same budget — a saturated gate + /// produces a timely "false" instead of an unbounded stall. + /// Default: 2. + /// + public int TriggerEvalTimeoutSeconds { get; set; } = 2; + + /// + /// WP3.1: maximum number of concurrent runs (queued or executing) for any single script + /// or alarm on-trigger script. A trigger that arrives while the cap is reached is SHED — + /// the newest run is refused, the four already in flight (which are closest to their own + /// deadlines) are kept, and no reordering occurs. A shed increments + /// ISiteHealthCollector.IncrementScriptRunShed, emits a rate-limited Warning site + /// event, and — for an Ask-based CallScript — replies with an explicit error rather + /// than letting the caller hang to its Ask timeout. + /// Default: 4. + /// + public int MaxConcurrentRunsPerScript { get; set; } = 4; + /// /// Max mirrored native alarms retained per source binding before older entries are dropped (logged). /// Default: 1000. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs index 6515c648..f5e0f0a6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/SiteRuntimeOptionsValidator.cs @@ -39,7 +39,28 @@ public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase 0, $"ScadaBridge:SiteRuntime:ScriptExecutionThreadCount must be greater than 0 " + - $"(was {options.ScriptExecutionThreadCount}); it sizes the dedicated script-execution scheduler."); + $"(was {options.ScriptExecutionThreadCount}); it is the FLOOR for the dedicated script-execution scheduler."); + + builder.RequireThat(options.ScriptExecutionMaxThreadCount >= options.ScriptExecutionThreadCount, + $"ScadaBridge:SiteRuntime:ScriptExecutionMaxThreadCount must be >= ScriptExecutionThreadCount " + + $"(was {options.ScriptExecutionMaxThreadCount} vs {options.ScriptExecutionThreadCount}); it is the " + + "CEILING for the instance-scaled script-execution pool and a ceiling below the floor would " + + "silently shrink the configured pool."); + + builder.RequireThat(options.TriggerEvalMaxConcurrency > 0, + $"ScadaBridge:SiteRuntime:TriggerEvalMaxConcurrency must be greater than 0 " + + $"(was {options.TriggerEvalMaxConcurrency}); it gates concurrent trigger-expression evaluations " + + "and a zero gate would park every Expression trigger on the node forever."); + + builder.RequireThat(options.TriggerEvalTimeoutSeconds > 0, + $"ScadaBridge:SiteRuntime:TriggerEvalTimeoutSeconds must be greater than 0 " + + $"(was {options.TriggerEvalTimeoutSeconds}); it bounds a single trigger-expression evaluation " + + "(measured from enqueue, so it also bounds gate-wait time)."); + + builder.RequireThat(options.MaxConcurrentRunsPerScript > 0, + $"ScadaBridge:SiteRuntime:MaxConcurrentRunsPerScript must be greater than 0 " + + $"(was {options.MaxConcurrentRunsPerScript}); it caps concurrent runs per script and a zero cap " + + "would shed every trigger."); builder.RequireThat(options.MirroredAlarmCapPerSource > 0, $"ScadaBridge:SiteRuntime:MirroredAlarmCapPerSource must be greater than 0 " + diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs index 4a333061..02ab43b3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs @@ -254,7 +254,6 @@ public class ExecutionIdCorrelationTests : TestKit, IClassFixture.Instance); return new ScriptRuntimeContext( - ActorRefs.Nobody, ActorRefs.Nobody, sharedScriptLibrary, currentCallDepth: 0, diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ParentExecutionIdCorrelationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ParentExecutionIdCorrelationTests.cs index ac72f09f..a1b826cc 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ParentExecutionIdCorrelationTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ParentExecutionIdCorrelationTests.cs @@ -594,7 +594,6 @@ public class ParentExecutionIdCorrelationTests : TestKit, IClassFixture.Instance); return new SiteRuntime.Scripts.ScriptRuntimeContext( - actorRef, actorRef, sharedLibrary, currentCallDepth: 0, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs index 3d5d2050..66341fd6 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs @@ -1028,14 +1028,22 @@ public class AlarmActorTests : TestKit, IDisposable } [Fact] - public void ExpressionAlarm_EvaluatesOnSchedulerThread_AndActivates() + public void ExpressionAlarm_EvaluatesOffTheBlockingScriptPool_AndActivates() { - // TRUE only when evaluated on a script-execution thread. Before P1 the - // expression ran on the actor dispatcher (name is NOT "script-execution-*") - // → false → alarm never activates. After P1 it runs on the script scheduler. + // WP3.1 retarget of the former P1 assertion, whose sense is deliberately INVERTED. + // + // P1 moved the evaluation off the actor's dispatcher and onto the dedicated + // script-execution scheduler, and this test asserted exactly that. WP3.1 (finding #4) + // proved that destination wrong for alarms in particular: sharing the blocking pool + // meant an alarm that should raise in milliseconds queued behind blocked script bodies + // for an unbounded time, and its 2 s evaluation timeout did not even start ticking + // until it was dequeued. Evaluations now run on the shared .NET thread pool behind + // TriggerEvalGate. + // + // TRUE only when evaluated OFF a script-execution thread. var expr = CompileRawTriggerExpression( - "System.Threading.Thread.CurrentThread.Name != null && " + - "System.Threading.Thread.CurrentThread.Name.StartsWith(\"script-execution-\")"); + "System.Threading.Thread.CurrentThread.Name == null || " + + "!System.Threading.Thread.CurrentThread.Name.StartsWith(\"script-execution-\")"); var alarmConfig = new ResolvedAlarm { CanonicalName = "ExprAlarm", diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmCascadeParentExecutionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmCascadeParentExecutionTests.cs index 29339fcb..1ab679a1 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmCascadeParentExecutionTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmCascadeParentExecutionTests.cs @@ -217,7 +217,6 @@ public class AlarmCascadeParentExecutionTests : TestKit, IDisposable var executionId = Guid.NewGuid(); var context = new ScriptRuntimeContext( probe.Ref, - ActorRefs.Nobody, _sharedLibrary, currentCallDepth: 0, maxCallDepth: 10, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs new file mode 100644 index 00000000..d8cdaa8d --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs @@ -0,0 +1,230 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +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.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.TestSupport; +using System.Text.Json; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 test group 9 — warm-then-gate deploys and startup batch pre-warm. +/// +/// The site-side compile gate (S3) must stay synchronous on the Deployment Manager's +/// thread, because redeploy-supersede and delete-during-redeploy both depend on strict mailbox +/// FIFO. But the Roslyn compile it performs used to hold the singleton for the whole +/// compilation, stalling every OTHER instance's commands behind one instance's scripts. WP3.1 +/// warms the compile off-thread first and then re-runs the gate as pure cache hits, with a +/// per-instance in-flight guard preserving same-instance ordering. +/// +/// These tests pin the ordering contract, not the timing: a command for the SAME instance +/// arriving during a warm must be queued and applied after the deploy, a superseded deploy must +/// answer its deployer instead of leaving it to Ask-timeout, and commands for DIFFERENT +/// instances must not block each other. +/// +/// Shares the SiteScriptCompileCache collection because the batch pre-warm test +/// asserts on that process-wide cache's hit counter. +/// +[Collection("SiteScriptCompileCache")] +public class DeploymentWarmThenGateTests : TestKit, IDisposable +{ + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly TestLocalDb _localDb; + + public DeploymentWarmThenGateTests() + { + _localDb = TestLocalDb.CreateTemp("dm-warm-gate-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() + { + Shutdown(); + var path = _localDb.Path; + _localDb.Dispose(); + TestLocalDb.DeleteFiles(path); + } + + private IActorRef CreateDeploymentManager(ISiteHealthCollector? healthCollector = null) => + ActorOf(Props.Create(() => new DeploymentManagerActor( + _storage, _compilationService, _sharedScriptLibrary, null, + new SiteRuntimeOptions(), NullLogger.Instance, null, + healthCollector))); + + /// + /// Captures the deployed-instance count the Deployment Manager reports. The count is + /// mutated only on the actor thread — HandleDeploy adds the instance name, + /// HandleDelete removes it — so it is an exact, storage-race-free record of the + /// order in which the two commands were APPLIED. + /// + private sealed class DeployedCountCollector : ISiteHealthCollector + { + public int LastDeployedCount { get; private set; } + public void IncrementScriptError() { } + public void IncrementAlarmError() { } + public void IncrementDeadLetter() { } + public void IncrementSiteAuditWriteFailures() { } + public void IncrementAuditRedactionFailure() { } + public void UpdateSiteAuditBacklog(Commons.Types.SiteAuditBacklogSnapshot snapshot) { } + public void UpdateConnectionHealth(string connectionName, ConnectionHealth health) { } + public void RemoveConnection(string connectionName) { } + public void UpdateTagResolution(string connectionName, int totalSubscribed, int successfullyResolved) { } + public void UpdateConnectionEndpoint(string connectionName, string endpoint) { } + public void UpdateTagQuality(string connectionName, int good, int bad, int uncertain) { } + public void SetStoreAndForwardDepths(IReadOnlyDictionary depths) { } + public void SetInstanceCounts(int deployed, int enabled, int disabled) => LastDeployedCount = deployed; + public void SetParkedMessageCount(int count) { } + public void SetNodeHostname(string hostname) { } + public void SetClusterNodes(IReadOnlyList nodes) { } + public void SetActiveNode(bool isActive) { } + public bool IsActiveNode => true; + public Commons.Messages.Health.SiteHealthReport CollectReport(string siteId) + => throw new NotSupportedException(); + } + + private static string ConfigJson(string instanceName, string? scriptCode = null) => + JsonSerializer.Serialize(new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "TestAttr", Value = "1", DataType = "Int32" } + ], + Scripts = scriptCode is null + ? [] + : [new ResolvedScript { CanonicalName = "Worker", Code = scriptCode, TriggerType = "Call" }] + }); + + [Fact] + public async Task DeleteArrivingDuringTheCompileWarm_IsQueuedAndAppliedAfterTheDeploy() + { + var health = new DeployedCountCollector(); + var dm = CreateDeploymentManager(health); + await Task.Delay(500); // empty startup + Assert.Equal(0, health.LastDeployedCount); + + var deployProbe = CreateTestProbe(); + var deleteProbe = CreateTestProbe(); + + // Back-to-back on the mailbox: the delete lands while the deploy's compile warm is + // still in flight, so it must be queued rather than racing ahead of the deploy. + dm.Tell(new DeployInstanceCommand( + "dep-1", "WarmPump", "h1", ConfigJson("WarmPump", "return 1;"), "admin", DateTimeOffset.UtcNow), + deployProbe.Ref); + dm.Tell(new DeleteInstanceCommand("del-1", "WarmPump", DateTimeOffset.UtcNow), deleteProbe.Ref); + + var deploy = deployProbe.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.Equal(DeploymentStatus.Success, deploy.Status); + + var delete = deleteProbe.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.True(delete.Success); + + // Terminal in-memory state: the deploy applied FIRST (adding the instance) and the + // delete applied SECOND (removing it), leaving the count at 0. Had the delete raced + // ahead of the warm it would have removed nothing and the deploy would have left the + // count at 1. This is the ordering signal rather than the SQLite row, because the + // deploy's store and the delete's remove are independent background tasks whose + // completion order the actor has never guaranteed (true before WP3.1 as well). + AwaitAssert(() => Assert.Equal(0, health.LastDeployedCount), TimeSpan.FromSeconds(10)); + } + + [Fact] + public async Task SecondDeployDuringTheWarm_SupersedesTheFirst_AndAnswersItsDeployer() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + var first = CreateTestProbe(); + var second = CreateTestProbe(); + + dm.Tell(new DeployInstanceCommand( + "dep-a", "SupersedePump", "h1", ConfigJson("SupersedePump", "return 1;"), "admin", DateTimeOffset.UtcNow), + first.Ref); + dm.Tell(new DeployInstanceCommand( + "dep-b", "SupersedePump", "h2", ConfigJson("SupersedePump", "return 2;"), "admin", DateTimeOffset.UtcNow), + second.Ref); + + // The displaced deployer is answered rather than left to time out its Ask. + var superseded = first.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.Equal("dep-a", superseded.DeploymentId); + Assert.Equal(DeploymentStatus.Failed, superseded.Status); + Assert.Contains("superseded", superseded.ErrorMessage!, StringComparison.OrdinalIgnoreCase); + + var winner = second.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.Equal("dep-b", winner.DeploymentId); + Assert.Equal(DeploymentStatus.Success, winner.Status); + + // Exactly one row, carrying the winning revision hash. + var configs = await _storage.GetAllDeployedConfigsAsync(); + var row = Assert.Single(configs, c => c.InstanceUniqueName == "SupersedePump"); + Assert.Equal("h2", row.RevisionHash); + } + + [Fact] + public async Task DeploysForDifferentInstances_DoNotBlockEachOther() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + var probeX = CreateTestProbe(); + var probeY = CreateTestProbe(); + + dm.Tell(new DeployInstanceCommand( + "dep-x", "PumpX", "hx", ConfigJson("PumpX", "return 1;"), "admin", DateTimeOffset.UtcNow), + probeX.Ref); + dm.Tell(new DeployInstanceCommand( + "dep-y", "PumpY", "hy", ConfigJson("PumpY", "return 2;"), "admin", DateTimeOffset.UtcNow), + probeY.Ref); + + // Both apply; the per-instance warm guard scopes to the instance, so a warm for X + // never queues a command for Y. + Assert.Equal(DeploymentStatus.Success, + probeX.ExpectMsg(TimeSpan.FromSeconds(15)).Status); + Assert.Equal(DeploymentStatus.Success, + probeY.ExpectMsg(TimeSpan.FromSeconds(15)).Status); + + var configs = await _storage.GetAllDeployedConfigsAsync(); + Assert.Contains(configs, c => c.InstanceUniqueName == "PumpX"); + Assert.Contains(configs, c => c.InstanceUniqueName == "PumpY"); + } + + [Fact] + public async Task StaggeredStartup_PreWarmsEachBatchSoInstanceActorPreStartCompilesAreCacheHits() + { + // Two instances sharing one script body. The batch pre-warm compiles it once; the + // second config's warm and BOTH Instance Actors' PreStart compiles are then hits. + // Before WP3.1 every Instance Actor Roslyn-compiled its own scripts inside PreStart, + // serialising a site's whole failover recovery behind compilation. + const string sharedCode = "return 41 + 1;"; + await _storage.StoreDeployedConfigAsync( + "BatchOne", ConfigJson("BatchOne", sharedCode), "d1", "h1", true); + await _storage.StoreDeployedConfigAsync( + "BatchTwo", ConfigJson("BatchTwo", sharedCode), "d2", "h2", true); + + SiteScriptCompileCache.Clear(); + Assert.Equal(0, SiteScriptCompileCache.Hits); + + CreateDeploymentManager(); + + AwaitAssert(() => + { + // One compile, then repeated hits: the second pre-warm plus both PreStarts. + Assert.True(SiteScriptCompileCache.Hits >= 3, + $"expected the pre-warmed body to be served from cache, saw {SiteScriptCompileCache.Hits} hits"); + }, TimeSpan.FromSeconds(20)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ExecutionActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ExecutionActorTests.cs deleted file mode 100644 index 0b1c4b5b..00000000 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ExecutionActorTests.cs +++ /dev/null @@ -1,440 +0,0 @@ -using Akka.Actor; -using Akka.TestKit.Xunit2; -using Microsoft.CodeAnalysis.CSharp.Scripting; -using Microsoft.CodeAnalysis.Scripting; -using Microsoft.Extensions.Logging.Abstractions; -using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; -using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; -using ZB.MOM.WW.ScadaBridge.Commons.Types.Scripts; -using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; -using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; -using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; - -namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; - -/// -/// Regression coverage for SiteRuntime-016 — the short-lived execution actors -/// (, ) were -/// previously untested. Covers success, exception, timeout, Ask-reply, and the -/// PoisonPill self-stop after completion. -/// -public class ExecutionActorTests : TestKit, IDisposable -{ - private readonly SharedScriptLibrary _sharedLibrary; - private readonly ScriptCompilationService _compilationService; - - public ExecutionActorTests() - { - _compilationService = new ScriptCompilationService( - NullLogger.Instance); - _sharedLibrary = new SharedScriptLibrary( - _compilationService, NullLogger.Instance); - } - - void IDisposable.Dispose() => Shutdown(); - - private static Script CompileScript(string code) - { - var scriptOptions = ScriptOptions.Default - .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly) - .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); - var script = CSharpScript.Create(code, scriptOptions, typeof(ScriptGlobals)); - script.Compile(); - return script; - } - - private static SiteRuntimeOptions Options(int timeoutSeconds = 30) - => new() { MaxScriptCallDepth = 10, ScriptExecutionTimeoutSeconds = timeoutSeconds }; - - // ── ScriptExecutionActor ── - - [Fact] - public void ScriptExecutionActor_Success_RepliesWithResultAndStops() - { - var compiled = CompileScript("return 7 * 6;"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Answer", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(), - replyTo.Ref, "corr-1", NullLogger.Instance, - ScriptScope.Root, null, null))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.True(result.Success); - Assert.Equal("corr-1", result.CorrelationId); - Assert.Equal(42, result.ReturnValue); - - // The actor must PoisonPill itself once execution completes. - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - // ── M1.8: site event log `script` started/completed ──────────────────── - - [Fact] - public void ScriptExecutionActor_Success_EmitsScriptStartedAndCompletedInfoEvents() - { - var compiled = CompileScript("return 7 * 6;"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - var siteLog = new FakeSiteEventLogger(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Answer", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(), - replyTo.Ref, "corr-evt-1", NullLogger.Instance, - ScriptScope.Root, null, new SingleServiceProvider(siteLog)))); - - Watch(exec); - replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - - AwaitAssert(() => - { - var rows = siteLog.OfType("script"); - // started + completed, both Info, in order. - Assert.Equal(2, rows.Count); - Assert.All(rows, r => - { - Assert.Equal("Info", r.Severity); - Assert.Equal("Inst1", r.InstanceId); - Assert.Equal("ScriptActor:Answer", r.Source); - }); - Assert.Contains("started", rows[0].Message, StringComparison.OrdinalIgnoreCase); - Assert.Contains("completed", rows[1].Message, StringComparison.OrdinalIgnoreCase); - }, TimeSpan.FromSeconds(2)); - } - - [Fact] - public void ScriptExecutionActor_Failure_EmitsStartedInfoThenErrorEvent() - { - var compiled = CompileScript("throw new InvalidOperationException(\"boom\");"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - var siteLog = new FakeSiteEventLogger(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Bad", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(), - replyTo.Ref, "corr-evt-2", NullLogger.Instance, - ScriptScope.Root, null, new SingleServiceProvider(siteLog)))); - - Watch(exec); - replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - - AwaitAssert(() => - { - var rows = siteLog.OfType("script"); - // started (Info) + failed (Error) — no completed. - Assert.Equal(2, rows.Count); - Assert.Equal("Info", rows[0].Severity); - Assert.Contains("started", rows[0].Message, StringComparison.OrdinalIgnoreCase); - Assert.Equal("Error", rows[1].Severity); - }, TimeSpan.FromSeconds(2)); - } - - [Fact] - public void ScriptExecutionActor_ScriptThrows_RepliesFailureAndStops() - { - var compiled = CompileScript("throw new InvalidOperationException(\"boom\");"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Bad", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(), - replyTo.Ref, "corr-2", NullLogger.Instance, - ScriptScope.Root, null, null))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.False(result.Success); - Assert.Equal("corr-2", result.CorrelationId); - Assert.Contains("boom", result.ErrorMessage); - - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void ScriptExecutionActor_Timeout_RepliesFailureAndStops() - { - // A long busy loop that observes the cancellation token so the - // 1-second timeout fires cooperatively. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Slow", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 1), - replyTo.Ref, "corr-3", NullLogger.Instance, - ScriptScope.Root, null, null))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.False(result.Success); - Assert.Contains("timed out", result.ErrorMessage); - - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void ScriptExecutionActor_PerScriptTimeout_OverridesLongerGlobal() - { - // M2.5 (#9): a short per-script timeout (1s) must win over a long global - // (300s), so the busy loop is cancelled at the per-script value. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Slow", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 300), - replyTo.Ref, "corr-perscript", NullLogger.Instance, - ScriptScope.Root, null, null, null, - /* executionTimeoutSeconds */ 1))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.False(result.Success); - Assert.Contains("timed out", result.ErrorMessage); - - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void ScriptExecutionActor_NullPerScriptTimeout_FallsBackToGlobal() - { - // M2.5 (#9): a null per-script timeout falls back to the global (1s here), - // so the busy loop is still cancelled at the global value. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Slow", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 1), - replyTo.Ref, "corr-fallback", NullLogger.Instance, - ScriptScope.Root, null, null, null, - /* executionTimeoutSeconds */ null))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.False(result.Success); - Assert.Contains("timed out", result.ErrorMessage); - - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void ScriptExecutionActor_NonPositivePerScriptTimeout_FallsBackToGlobal() - { - // M2.5 (#9): a non-positive per-script value (<= 0) is treated as "use - // global", so the busy loop is cancelled at the global (1s) value. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "Slow", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 1), - replyTo.Ref, "corr-clamp", NullLogger.Instance, - ScriptScope.Root, null, null, null, - /* executionTimeoutSeconds */ 0))); - - Watch(exec); - - var result = replyTo.ExpectMsg(TimeSpan.FromSeconds(10)); - Assert.False(result.Success); - Assert.Contains("timed out", result.ErrorMessage); - - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void ScriptExecutionActor_NoReplyTo_StillStopsAfterCompletion() - { - var compiled = CompileScript("return 1;"); - var instanceActor = CreateTestProbe(); - - // ActorRefs.Nobody as replyTo — fire-and-forget execution. - var exec = ActorOf(Props.Create(() => new ScriptExecutionActor( - "FireForget", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, Options(), - ActorRefs.Nobody, "corr-4", NullLogger.Instance, - ScriptScope.Root, null, null))); - - Watch(exec); - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - // ── AlarmExecutionActor ── - - [Fact] - public void AlarmExecutionActor_Success_StopsAfterCompletion() - { - var compiled = CompileScript("return 0;"); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new AlarmExecutionActor( - "HiTemp", "Inst1", AlarmLevel.High, 5, "High temperature", - compiled, instanceActor.Ref, _sharedLibrary, Options(), - NullLogger.Instance))); - - Watch(exec); - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void AlarmExecutionActor_ScriptThrows_StillStops() - { - var compiled = CompileScript("throw new System.Exception(\"alarm-boom\");"); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new AlarmExecutionActor( - "HiTemp", "Inst1", AlarmLevel.High, 5, "High temperature", - compiled, instanceActor.Ref, _sharedLibrary, Options(), - NullLogger.Instance))); - - Watch(exec); - // Even on a throwing on-trigger body, the actor must self-stop. - ExpectTerminated(exec, TimeSpan.FromSeconds(5)); - } - - [Fact] - public void AlarmExecutionActor_PerScriptTimeout_OverridesLongerGlobal() - { - // M2.5 (#9): the alarm on-trigger script's per-script timeout (1s) wins - // over a long global (300s). The busy loop is cancelled and the actor - // self-stops (the timeout is logged, alarm continues). - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new AlarmExecutionActor( - "HiTemp", "Inst1", AlarmLevel.High, 5, "High temperature", - compiled, instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 300), - NullLogger.Instance, /* executionTimeoutSeconds */ 1))); - - Watch(exec); - // If the per-script timeout were ignored it would block ~300s and this - // ExpectTerminated would fail; with the override it stops within ~1s. - ExpectTerminated(exec, TimeSpan.FromSeconds(10)); - } - - [Fact] - public void AlarmExecutionActor_NullPerScriptTimeout_FallsBackToGlobal() - { - // M2.5 (#9): a null per-script timeout falls back to the global (1s here), - // so the busy loop is cancelled at the global value and the actor self-stops. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new AlarmExecutionActor( - "HiTemp", "Inst1", AlarmLevel.High, 5, "High temperature", - compiled, instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 1), - NullLogger.Instance, /* executionTimeoutSeconds */ null))); - - Watch(exec); - // Global timeout (1s) must fire even when per-script is null. - ExpectTerminated(exec, TimeSpan.FromSeconds(10)); - } - - [Fact] - public void AlarmExecutionActor_NonPositivePerScriptTimeout_FallsBackToGlobal() - { - // M2.5 (#9): a non-positive per-script value (<= 0) is treated as "use - // global", so the busy loop is cancelled at the global (1s) value. - var compiled = CompileScript( - "while (true) { await System.Threading.Tasks.Task.Delay(50, CancellationToken); }"); - var instanceActor = CreateTestProbe(); - - var exec = ActorOf(Props.Create(() => new AlarmExecutionActor( - "HiTemp", "Inst1", AlarmLevel.High, 5, "High temperature", - compiled, instanceActor.Ref, _sharedLibrary, Options(timeoutSeconds: 1), - NullLogger.Instance, /* executionTimeoutSeconds */ 0))); - - Watch(exec); - // Non-positive per-script timeout must be ignored; global (1s) must fire. - ExpectTerminated(exec, TimeSpan.FromSeconds(10)); - } - - // ── S2: stuck-script watchdog names the script holding a scheduler thread ── - - /// - /// Compiles a raw script that can reach the test-assembly - /// (so a script body can block on a gate). Bypasses the trust validator, like - /// . - /// - private static Script CompileRaw(string code) - { - var scriptOptions = ScriptOptions.Default - .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, typeof(StuckTestHooks).Assembly) - .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); - var script = CSharpScript.Create(code, scriptOptions, typeof(ScriptGlobals)); - script.Compile(); - return script; - } - - [Fact] - public void TimedOutScript_WithBlockedThread_EmitsStuckThreadSiteEvent() - { - // A script that blocks synchronously on a gate NEVER observes cooperative - // cancellation, so the CTS firing at the 1s timeout does not free its - // scheduler thread. After the 200ms grace the watchdog must name it loudly. - StuckTestHooks.Gate = new SemaphoreSlim(0); - var compiled = CompileRaw( - "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.StuckTestHooks.Gate.Wait(); return null;"); - var replyTo = CreateTestProbe(); - var instanceActor = CreateTestProbe(); - var siteLog = new FakeSiteEventLogger(); - var options = new SiteRuntimeOptions { ScriptExecutionTimeoutSeconds = 1, StuckScriptGraceMs = 200 }; - - ActorOf(Props.Create(() => new ScriptExecutionActor( - "StuckScript", "Inst1", compiled, null, 0, - instanceActor.Ref, _sharedLibrary, options, - replyTo.Ref, "corr-stuck", NullLogger.Instance, - ScriptScope.Root, null, new SingleServiceProvider(siteLog)))); - - try - { - AwaitAssert(() => - { - var rows = siteLog.OfType("script"); - Assert.Contains(rows, r => - r.Severity == "Error" && - r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase) && - r.Message.Contains("StuckScript")); - }, TimeSpan.FromSeconds(10)); - } - finally - { - // Free the blocked scheduler thread so the test run stays clean. - StuckTestHooks.Gate.Release(); - } - } -} - -/// -/// Test hook the stuck-script watchdog test uses to block a script-execution -/// thread deterministically: the compiled body waits on , which -/// the test releases once it has observed the stuck-thread site event. -/// -public static class StuckTestHooks -{ - /// Gate a blocking test script waits on; reset per test. - public static SemaphoreSlim Gate = new(0); -} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs index 44bc846d..92a09b88 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs @@ -403,13 +403,15 @@ public class ScriptActorTests : TestKit, IDisposable "ExprFault", "Expression", "{\"expression\":\"true\",\"mode\":\"OnTrue\"}", null, expr); try { - actor.Tell(Change("A", "1")); // eval starts on the scheduler and BLOCKS → _evalInFlight = true + actor.Tell(Change("A", "1")); // eval starts off-thread and BLOCKS → _evalInFlight = true AwaitAssert(() => Assert.Equal(1, EvalGate.Entries), TimeSpan.FromSeconds(10)); - // #18 seam: the blocked evaluation is running on THIS class's injected - // scheduler — not the process-wide singleton — so a worker it strands can - // never starve another test class. - Assert.Equal(1, _scheduler.BusyThreadCount); + // WP3.1: the blocked evaluation no longer occupies a script-execution worker at + // all — it runs on the shared thread pool behind TriggerEvalGate, which is the + // whole point of finding #4. The blocking pool must be completely idle here; the + // pre-WP3.1 assertion was the opposite (BusyThreadCount == 1). + Assert.Equal(0, _scheduler.BusyThreadCount); + Assert.Equal(0, _scheduler.QueueDepth); actor.Tell(Change("A", "2")); // coalesces → _evalPending = true @@ -432,15 +434,24 @@ public class ScriptActorTests : TestKit, IDisposable } [Fact] - public void ExpressionTrigger_EvaluatesOnScriptSchedulerThread_AndStillFires() + public void ExpressionTrigger_EvaluatesOffTheBlockingScriptPool_AndStillFires() { - // The expression is TRUE only when evaluated on a script-execution thread. - // Before P1 it ran synchronously on the actor's dispatcher thread (name is - // NOT "script-execution-*") → false → no fire. After P1 it runs on the - // script scheduler → true → fire. + // WP3.1 retarget of the former P1 assertion, whose sense is deliberately INVERTED. + // + // P1 moved trigger-expression evaluation off the actor's dispatcher thread and onto + // the dedicated script-execution scheduler, and this test asserted exactly that + // ("evaluated on a script-execution-* thread"). WP3.1 (finding #4) proved that + // destination wrong: sharing the blocking pool meant N blocked script bodies stalled + // every Expression trigger on the node indefinitely. Evaluations are non-blocking by + // construction, so they now run as plain async work on the shared .NET thread pool + // behind TriggerEvalGate. + // + // The expression is therefore TRUE only when evaluated OFF a script-execution thread — + // still off the dispatcher (the P1 property, covered by the coalescing/PipeTo tests), + // and now provably off the blocking pool too. var expr = CompileRawTriggerExpression( - "System.Threading.Thread.CurrentThread.Name != null && " + - "System.Threading.Thread.CurrentThread.Name.StartsWith(\"script-execution-\")"); + "System.Threading.Thread.CurrentThread.Name == null || " + + "!System.Threading.Thread.CurrentThread.Name.StartsWith(\"script-execution-\")"); var (actor, instance) = CreateTriggeredActor( "ExprThread", "Expression", @@ -449,7 +460,7 @@ public class ScriptActorTests : TestKit, IDisposable expr); actor.Tell(Change("Any", "1")); - instance.ExpectMsg(TimeSpan.FromSeconds(10)); // fired ⇒ evaluated off-dispatcher + instance.ExpectMsg(TimeSpan.FromSeconds(10)); // fired ⇒ evaluated off the blocking pool } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs new file mode 100644 index 00000000..8e708540 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs @@ -0,0 +1,136 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 test group 3 — a script's execution deadline is armed when the run is ENQUEUED, not +/// when it is dequeued. +/// +/// Before WP3.1 the deadline was constructed inside +/// the queued body, so a run that spent ten minutes waiting behind blocked scripts still got a +/// fresh full 30 s budget when it finally started — and then ran work whose triggering +/// condition was long stale. Now queue wait consumes the run's own budget, and a body that +/// dequeues past its deadline is SHED at dequeue: it never executes, and takes the existing +/// timeout path (site event, script-error counter, error reply, completion message). +/// +public class ScriptDeadlineAtEnqueueTests : TestKit, IDisposable +{ + private readonly SharedScriptLibrary _sharedLibrary; + + public ScriptDeadlineAtEnqueueTests() + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + DeadlineHooks.Gate = new SemaphoreSlim(0); + DeadlineHooks.SecondScriptRan = false; + } + + void IDisposable.Dispose() + { + DeadlineHooks.Gate.Release(8); + Shutdown(); + } + + private static Script CompileRaw(string code) + { + var options = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, + typeof(DeadlineHooks).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(code, options, typeof(ScriptGlobals)); + script.Compile(); + return script; + } + + private IActorRef BuildScriptActor( + string name, Script compiled, SiteRuntimeOptions options, + ScriptExecutionScheduler scheduler, IServiceProvider? serviceProvider) + { + var instance = CreateTestProbe().Ref; + var config = new ResolvedScript { CanonicalName = name, TriggerType = "Call" }; + return ActorOf(Props.Create(() => new ScriptActor( + name, "Inst1", instance, compiled, config, _sharedLibrary, options, + NullLogger.Instance, null, null, null, serviceProvider, scheduler, null))); + } + + [Fact] + public void ARunThatDequeuesPastItsDeadline_IsShedWithoutExecutingItsBody() + { + using var scheduler = new ScriptExecutionScheduler(1); + var siteLog = new FakeSiteEventLogger(); + + // The single worker is held by a first script for longer than the second script's + // entire timeout. + var blocker = BuildScriptActor( + "Blocker", + CompileRaw("ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.DeadlineHooks.Gate.Wait(); return null;"), + new SiteRuntimeOptions { ScriptExecutionTimeoutSeconds = 120, StuckScriptGraceMs = 120_000 }, + scheduler, null); + blocker.Tell(new ScriptCallRequest("Blocker", null, 0, "corr-blocker"), ActorRefs.NoSender); + AwaitAssert(() => Assert.Equal(1, scheduler.BusyThreadCount), TimeSpan.FromSeconds(15)); + + // A 1 s script queued behind it. Its grace is generous so that if the watchdog DID + // fire it would have ample opportunity to emit — the assertion below is that it does not. + var lateOptions = new SiteRuntimeOptions + { + ScriptExecutionTimeoutSeconds = 1, + StuckScriptGraceMs = 1000 + }; + var late = BuildScriptActor( + "Late", + CompileRaw( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.DeadlineHooks.SecondScriptRan = true; return 1;"), + lateOptions, scheduler, new SingleServiceProvider(siteLog)); + + var caller = CreateTestProbe(); + late.Tell(new ScriptCallRequest("Late", null, 0, "corr-late"), caller.Ref); + + // Let its whole budget elapse while it is still queued, then free the worker. + Thread.Sleep(TimeSpan.FromSeconds(2)); + DeadlineHooks.Gate.Release(); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.False(result.Success); + Assert.Contains("timed out", result.ErrorMessage); + + // The body never ran: stale work is shed at dequeue rather than executed late. + Assert.False(DeadlineHooks.SecondScriptRan, + "the queued body executed even though its deadline had already passed"); + + AwaitAssert(() => + { + var rows = siteLog.OfType("script"); + // Timeout path only — no "started" Info event, because the body was skipped. + Assert.Contains(rows, r => r.Severity == "Error" && r.Message.Contains("timed out")); + Assert.DoesNotContain(rows, r => r.Message.Contains("started", StringComparison.OrdinalIgnoreCase)); + }, TimeSpan.FromSeconds(5)); + + // And the watchdog did NOT report it as a stuck thread: it was cancelled while queued, + // so it never held a worker. + Thread.Sleep(TimeSpan.FromSeconds(2)); // well past the 1 s grace + Assert.DoesNotContain(siteLog.OfType("script"), + r => r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase)); + Assert.Equal(0, scheduler.DetachedThreadCount); + } +} + +/// Test hooks for the enqueue-anchored deadline test. +public static class DeadlineHooks +{ + /// Gate the blocking first script waits on. + public static SemaphoreSlim Gate = new(0); + + /// Set by the second script's body — must stay false when the run is shed at dequeue. + public static bool SecondScriptRan; +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunLauncherParityTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunLauncherParityTests.cs new file mode 100644 index 00000000..830d4017 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunLauncherParityTests.cs @@ -0,0 +1,469 @@ +using Akka.Actor; +using Akka.Event; +using Akka.TestKit; +using Akka.TestKit.Xunit2; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Scripts; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 supervision-parity set (design memo §4, seven pins) — the reworked successor to +/// ExecutionActorTests. +/// +/// WP3.1 eliminated the short-lived ScriptExecutionActor and +/// AlarmExecutionActor: neither had a Receive handler, a PostStop, or any +/// state, and neither's IActorRef was ever a message target — the entire lifecycle lived +/// in a detached task. Runs are now launched directly by the coordinator via +/// . These tests pin every behaviour the removed actors +/// provided onto its replacement: exception and timeout containment, the Ask reply, the +/// completion notification, one DI scope per run, the audit ParentExecutionId threading, and +/// the supervision outcome (coordinator unaffected — no stop, no restart). +/// +public class ScriptRunLauncherParityTests : TestKit, IDisposable +{ + private readonly SharedScriptLibrary _sharedLibrary; + private readonly ScriptCompilationService _compilationService; + + /// Own pool per test class (#18 seam), so a wedged body cannot strand the process-wide one. + private readonly ScriptExecutionScheduler _scheduler = new(4); + + public ScriptRunLauncherParityTests() + { + _compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + } + + void IDisposable.Dispose() + { + Shutdown(); + _scheduler.Dispose(); + } + + // ── helpers ────────────────────────────────────────────────────────────────── + + private static Script CompileScript(string code) + { + var scriptOptions = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, + typeof(RunLauncherHooks).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(code, scriptOptions, typeof(ScriptGlobals)); + script.Compile(); + return script; + } + + private static SiteRuntimeOptions Options(int timeoutSeconds = 30, int graceMs = 30000) + => new() + { + MaxScriptCallDepth = 10, + ScriptExecutionTimeoutSeconds = timeoutSeconds, + StuckScriptGraceMs = graceMs + }; + + private static ResolvedScript CallScript(int? timeoutSeconds = null) => new() + { + CanonicalName = "Runner", + TriggerType = "Call", + ExecutionTimeoutSeconds = timeoutSeconds + }; + + private TestActorRef BuildScriptActor( + Script? compiled, + SiteRuntimeOptions options, + IServiceProvider? serviceProvider = null, + ISiteHealthCollector? healthCollector = null, + ScriptExecutionScheduler? scheduler = null, + int? perScriptTimeoutSeconds = null, + IActorRef? instanceActor = null) + { + var instance = instanceActor ?? CreateTestProbe().Ref; + return ActorOfAsTestActorRef( + Props.Create(() => new ScriptActor( + "Runner", "Inst1", instance, compiled, CallScript(perScriptTimeoutSeconds), + _sharedLibrary, options, NullLogger.Instance, + null, null, healthCollector, serviceProvider, scheduler ?? _scheduler, null)), + "script-" + Guid.NewGuid().ToString("N")); + } + + // ── Pin 1: throwing body — coordinator survives, everything is reported ─────── + + [Fact] + public void ThrowingScriptBody_LeavesScriptActorAliveAndReportsEverything() + { + var siteLog = new FakeSiteEventLogger(); + var health = new SiteHealthCollector(); + var actor = BuildScriptActor( + CompileScript("throw new InvalidOperationException(\"boom\");"), + Options(), + new SingleServiceProvider(siteLog), + health); + + Watch(actor); + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-throw"), caller.Ref); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.False(result.Success); + Assert.Equal("corr-throw", result.CorrelationId); + Assert.Contains("boom", result.ErrorMessage); + + AwaitAssert(() => + { + // Error site event + script-error counter, exactly as the execution actor emitted. + Assert.Contains(siteLog.OfType("script"), + r => r.Severity == "Error" && r.Message.Contains("failed", StringComparison.OrdinalIgnoreCase)); + // The in-flight slot is released, so the script can run again. + Assert.Equal(0, actor.UnderlyingActor.RunsInFlight); + }, TimeSpan.FromSeconds(5)); + + Assert.Equal(1, health.CollectReport("site-1").ScriptErrorCount); + + // The coordinator is neither stopped nor restarted — a throwing body was always + // contained inside the run's own try/catch, and still is. Watch() above means a stop + // would deliver Terminated to the TestActor; a still-answering call proves it is live. + ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-still-alive"), caller.Ref); + Assert.Equal("corr-still-alive", + caller.ExpectMsg(TimeSpan.FromSeconds(10)).CorrelationId); + } + + // ── Pin 2: launch-path throw — improved over the old silent hang ────────────── + + /// + /// The only failure the removed per-run child could surface was a constructor throw — + /// e.g. queueing onto a disposed . The old + /// OneForOneStrategy logged it and stopped the child, leaving the Ask caller to + /// hang with no reply at all. WP3.1 folds that into a launch-path catch that replies, an + /// intentional improvement pinned here so it is explicit rather than accidental. + /// + [Fact] + public void LaunchPathThrow_RepliesToTheCallerAndLeavesTheCoordinatorAlive() + { + var dead = new ScriptExecutionScheduler(1); + dead.Dispose(); + + var actor = BuildScriptActor( + CompileScript("return 1;"), Options(), scheduler: dead); + + Watch(actor); + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-launch"), caller.Ref); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.False(result.Success); + Assert.Equal("corr-launch", result.CorrelationId); + Assert.Contains("could not be launched", result.ErrorMessage); + + // Watch() above means a stop would deliver Terminated to the TestActor; none arrives, + // so the coordinator neither died nor restarted on a launch failure. + ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + + // The in-flight counter is incremented before the launch, so the catch MUST balance + // it — otherwise a run of launch failures would permanently wedge the script at cap. + AwaitAssert(() => Assert.Equal(0, actor.UnderlyingActor.RunsInFlight), TimeSpan.FromSeconds(5)); + } + + // ── Pin 3: exactly one DI scope per run, disposed on every path ─────────────── + + [Theory] + [InlineData("return 1;", 30)] // success + [InlineData("throw new InvalidOperationException(\"boom\");", 30)] // failure + [InlineData("while (true) { await Task.Delay(25, CancellationToken); }", 1)] // timeout + public void EachRun_CreatesOneDiScope_AndDisposesItExactlyOnce(string code, int timeoutSeconds) + { + var spy = new ScopeSpyServiceProvider(); + var actor = BuildScriptActor( + CompileScript(code), Options(timeoutSeconds), spy); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-scope"), caller.Ref); + caller.ExpectMsg(TimeSpan.FromSeconds(15)); + + AwaitAssert(() => + { + Assert.Equal(1, spy.ScopesCreated); + Assert.Equal(1, spy.ScopesDisposed); + }, TimeSpan.FromSeconds(5)); + } + + // ── Pin 4: audit correlation threading survives the actor removal ───────────── + + /// + /// A routed must still reach the run's + /// — this is the inbound-API leg of the audit execution + /// tree, and it used to be threaded through the execution actor's constructor. + /// + [Fact] + public void RoutedParentExecutionId_ReachesTheRunsScriptRuntimeContext() + { + RunLauncherHooks.CapturedContext = null; + var parent = Guid.NewGuid(); + + var actor = BuildScriptActor( + CompileScript( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.RunLauncherHooks.CapturedContext = Instance; return 1;"), + Options()); + + var caller = CreateTestProbe(); + actor.Tell( + new ScriptCallRequest("Runner", null, 0, "corr-parent", ParentExecutionId: parent), + caller.Ref); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.True(result.Success, result.ErrorMessage); + + AwaitAssert(() => + { + Assert.NotNull(RunLauncherHooks.CapturedContext); + Assert.Equal(parent, RunLauncherHooks.CapturedContext!.ParentExecutionId); + // The routed run still mints its OWN ExecutionId — the parent is a pointer, not a copy. + Assert.NotEqual(parent, RunLauncherHooks.CapturedContext.ExecutionId); + }, TimeSpan.FromSeconds(5)); + } + + // ── Pin 5: timeout resolution parity (perScript ?? global, <= 0 => global) ──── + + [Theory] + [InlineData(300, 1)] // per-script override wins over a much longer global + [InlineData(1, null)] // null per-script falls back to the global + [InlineData(1, 0)] // non-positive per-script is treated as "use global" + public void TimeoutResolution_MatchesTheRemovedExecutionActor(int globalSeconds, int? perScriptSeconds) + { + var siteLog = new FakeSiteEventLogger(); + var actor = BuildScriptActor( + CompileScript("while (true) { await Task.Delay(25, CancellationToken); }"), + Options(globalSeconds), + new SingleServiceProvider(siteLog), + perScriptTimeoutSeconds: perScriptSeconds); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-timeout"), caller.Ref); + + // If the effective timeout were the 300 s global (case 1) or ignored (cases 2/3) this + // would not answer inside the window. + var result = caller.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.False(result.Success); + Assert.Contains("timed out", result.ErrorMessage); + + AwaitAssert( + () => Assert.Contains(siteLog.OfType("script"), + r => r.Severity == "Error" && r.Message.Contains("timed out")), + TimeSpan.FromSeconds(5)); + } + + // ── Pin 6: stop-during-run parity ──────────────────────────────────────────── + + /// + /// Stopping a ScriptActor mid-run must NOT cancel the in-flight run (redeploy/undeploy + /// semantics: running scripts are allowed to finish). The run completes normally and its + /// completion message dead-letters, exactly as the old per-run child's + /// parent.Tell did once the subtree was stopped — dead letters are a health metric, + /// not an error. + /// + [Fact] + public void StoppingTheScriptActorMidRun_LetsTheRunFinishAndDeadLettersItsCompletion() + { + RunLauncherHooks.Gate = new SemaphoreSlim(0); + RunLauncherHooks.Finished = new ManualResetEventSlim(false); + RunLauncherHooks.ObservedCancellation = null; + + var actor = BuildScriptActor( + CompileScript( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.RunLauncherHooks.Gate.Wait();" + + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.RunLauncherHooks.ObservedCancellation = CancellationToken.IsCancellationRequested;" + + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.RunLauncherHooks.Finished.Set();" + + "return 1;"), + Options()); + + var deadLetters = CreateTestProbe(); + Sys.EventStream.Subscribe(deadLetters.Ref, typeof(DeadLetter)); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-stop"), caller.Ref); + + // Wait until the body is actually on a worker thread, then stop the coordinator. + AwaitAssert(() => Assert.Equal(1, _scheduler.BusyThreadCount), TimeSpan.FromSeconds(10)); + Watch(actor); + Sys.Stop(actor); + ExpectTerminated(actor, TimeSpan.FromSeconds(10)); + + RunLauncherHooks.Gate.Release(); + + // The run ran to completion and was never cancelled by the stop. + Assert.True(RunLauncherHooks.Finished.Wait(TimeSpan.FromSeconds(10))); + Assert.False(RunLauncherHooks.ObservedCancellation); + + // The Ask caller still gets its result (the reply target is not the stopped actor)… + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.True(result.Success, result.ErrorMessage); + + // …and the completion notification aimed at the now-stopped coordinator dead-letters. + deadLetters.FishForMessage( + d => d.Message is ScriptActor.ScriptExecutionCompleted, + TimeSpan.FromSeconds(10)); + } + + // ── Pin 7: alarm side ──────────────────────────────────────────────────────── + + /// + /// An alarm on-trigger run still receives the Alarm globals (name/level/priority/ + /// message) and still reports AlarmExecutionCompleted back to its AlarmActor — + /// observable here through the in-flight counter returning to zero, which only the + /// completion message can do. + /// + [Fact] + public void AlarmOnTriggerRun_GetsAlarmGlobals_AndCompletesBackToTheAlarmActor() + { + RunLauncherHooks.CapturedAlarm = null; + + var onTrigger = CompileScript( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.RunLauncherHooks.CapturedAlarm = Alarm; return null;"); + + var instanceProbe = CreateTestProbe(); + var alarm = ActorOfAsTestActorRef( + Props.Create(() => new AlarmActor( + "TempBand", "Inst1", instanceProbe.Ref, + new ResolvedAlarm + { + CanonicalName = "TempBand", + TriggerType = "HiLo", + TriggerConfiguration = "{\"attributeName\":\"Temp\",\"hi\":80,\"hiHi\":95,\"hiMessage\":\"too hot\"}", + PriorityLevel = 42 + }, + onTrigger, _sharedLibrary, Options(), NullLogger.Instance, + null, null, null, null, null, _scheduler, null)), + "alarm-" + Guid.NewGuid().ToString("N")); + + alarm.Tell(new Commons.Messages.Streaming.AttributeValueChanged( + "Inst1", "Temp", "Temp", 90.0, "Good", DateTimeOffset.UtcNow)); + + instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + + AwaitAssert(() => + { + Assert.NotNull(RunLauncherHooks.CapturedAlarm); + Assert.Equal("TempBand", RunLauncherHooks.CapturedAlarm!.Name); + Assert.Equal(AlarmLevel.High, RunLauncherHooks.CapturedAlarm.Level); + Assert.Equal("too hot", RunLauncherHooks.CapturedAlarm.Message); + // Only AlarmExecutionCompleted releases the slot. + Assert.Equal(0, alarm.UnderlyingActor.RunsInFlight); + }, TimeSpan.FromSeconds(10)); + } + + // ── Retargeted from ExecutionActorTests: success path + operational events ──── + + [Fact] + public void SuccessfulRun_RepliesWithTheReturnValue() + { + var actor = BuildScriptActor(CompileScript("return 7 * 6;"), Options()); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-ok"), caller.Ref); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.True(result.Success, result.ErrorMessage); + Assert.Equal("corr-ok", result.CorrelationId); + Assert.Equal(42, result.ReturnValue); + } + + [Fact] + public void SuccessfulRun_EmitsStartedThenCompletedInfoEvents() + { + var siteLog = new FakeSiteEventLogger(); + var actor = BuildScriptActor( + CompileScript("return 7 * 6;"), Options(), new SingleServiceProvider(siteLog)); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-evt"), caller.Ref); + caller.ExpectMsg(TimeSpan.FromSeconds(10)); + + AwaitAssert(() => + { + var rows = siteLog.OfType("script"); + Assert.Equal(2, rows.Count); + Assert.All(rows, r => + { + Assert.Equal("Info", r.Severity); + Assert.Equal("Inst1", r.InstanceId); + Assert.Equal("ScriptActor:Runner", r.Source); + }); + Assert.Contains("started", rows[0].Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("completed", rows[1].Message, StringComparison.OrdinalIgnoreCase); + }, TimeSpan.FromSeconds(5)); + } + + [Fact] + public void FailingRun_EmitsStartedInfoThenErrorEvent() + { + var siteLog = new FakeSiteEventLogger(); + var actor = BuildScriptActor( + CompileScript("throw new InvalidOperationException(\"boom\");"), + Options(), new SingleServiceProvider(siteLog)); + + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-evt-err"), caller.Ref); + caller.ExpectMsg(TimeSpan.FromSeconds(10)); + + AwaitAssert(() => + { + var rows = siteLog.OfType("script"); + Assert.Equal(2, rows.Count); + Assert.Equal("Info", rows[0].Severity); + Assert.Contains("started", rows[0].Message, StringComparison.OrdinalIgnoreCase); + Assert.Equal("Error", rows[1].Severity); + }, TimeSpan.FromSeconds(5)); + } + + [Fact] + public void FireAndForgetRun_NeedsNoReplyTarget() + { + var siteLog = new FakeSiteEventLogger(); + var actor = BuildScriptActor( + CompileScript("return 1;"), Options(), new SingleServiceProvider(siteLog)); + + // Trigger-driven spawns pass ActorRefs.NoSender as replyTo; drive that path via an + // interval-free Call script by telling the actor to run with no sender. + actor.Tell(new ScriptCallRequest("Runner", null, 0, "corr-nobody"), ActorRefs.NoSender); + + AwaitAssert( + () => Assert.Contains(siteLog.OfType("script"), + r => r.Message.Contains("completed", StringComparison.OrdinalIgnoreCase)), + TimeSpan.FromSeconds(10)); + } +} + +/// +/// Hooks a compiled test script can reach from inside a script body: a gate to block a +/// worker thread deterministically, and capture slots for the run's +/// and (both of which the run +/// otherwise exposes to nobody). +/// +public static class RunLauncherHooks +{ + /// Gate a blocking test script waits on; reset per test. + public static SemaphoreSlim Gate = new(0); + + /// Set by a test script once its body has run to completion. + public static ManualResetEventSlim Finished = new(false); + + /// Whether the script observed a cancellation request at the end of its body. + public static bool? ObservedCancellation; + + /// The runtime context handed to the last captured run. + public static ScriptRuntimeContext? CapturedContext; + + /// The Alarm global handed to the last captured on-trigger run. + public static AlarmContext? CapturedAlarm; +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunShedTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunShedTests.cs new file mode 100644 index 00000000..18024441 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunShedTests.cs @@ -0,0 +1,199 @@ +using Akka.Actor; +using Akka.TestKit; +using Akka.TestKit.Xunit2; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 test group 6 — the per-script in-flight cap and its shed policy. +/// +/// Before WP3.1 every trigger spawned another run unconditionally: a trigger firing +/// faster than its script completes produced unbounded fan-out onto a bounded thread pool. +/// The cap (, default 4) sheds the +/// NEWEST run instead. Keeping the four already queued/running — which are closest to their +/// own deadlines and already charged against them — is the policy that never reorders runs +/// and needs no queue at all: the scheduler's FIFO already IS the queue. +/// +/// A shed is always counted on the health collector, emits a site event rate-limited to +/// one per script per minute (so a hot trigger cannot flood site_events), and — for an +/// Ask-based CallScript — replies with an explicit error rather than letting a nested +/// call or inbound-API route hang to its Ask timeout. +/// +public class ScriptRunShedTests : TestKit, IDisposable +{ + private readonly SharedScriptLibrary _sharedLibrary; + private readonly ScriptExecutionScheduler _scheduler = new(8); + + public ScriptRunShedTests() + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + ShedHooks.Gate = new SemaphoreSlim(0); + } + + void IDisposable.Dispose() + { + ShedHooks.Gate.Release(64); + Shutdown(); + _scheduler.Dispose(); + } + + private static Script BlockingScript() => CompileRaw( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.ShedHooks.Gate.Wait(); return null;"); + + private static Script CompileRaw(string code) + { + var options = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, + typeof(ShedHooks).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(code, options, typeof(ScriptGlobals)); + script.Compile(); + return script; + } + + private static SiteRuntimeOptions Options() => new() + { + MaxConcurrentRunsPerScript = 4, + // Long enough that nothing times out inside the test window — the cap, not the + // deadline, must be what refuses the fifth run. + ScriptExecutionTimeoutSeconds = 120, + StuckScriptGraceMs = 120_000 + }; + + [Fact] + public void FifthConcurrentRun_IsShed_Counted_EventedOnce_AndAnsweredWithAnError() + { + var siteLog = new FakeSiteEventLogger(); + var health = new SiteHealthCollector(); + var instance = CreateTestProbe().Ref; + var options = Options(); + + var actor = ActorOfAsTestActorRef( + Props.Create(() => new ScriptActor( + "Hot", "Inst1", instance, BlockingScript(), + new ResolvedScript { CanonicalName = "Hot", TriggerType = "Call" }, + _sharedLibrary, options, NullLogger.Instance, + null, null, health, new SingleServiceProvider(siteLog), _scheduler, null)), + "shed-" + Guid.NewGuid().ToString("N")); + + // Fill the cap: four runs, all blocked in their bodies. + for (var i = 0; i < 4; i++) + actor.Tell(new ScriptCallRequest("Hot", null, 0, $"corr-{i}"), ActorRefs.NoSender); + + AwaitAssert(() => + { + Assert.Equal(4, actor.UnderlyingActor.RunsInFlight); + Assert.Equal(4, _scheduler.BusyThreadCount); + }, TimeSpan.FromSeconds(15)); + + // Fifth: shed. The Ask caller is answered explicitly instead of hanging. + var caller = CreateTestProbe(); + actor.Tell(new ScriptCallRequest("Hot", null, 0, "corr-shed-1"), caller.Ref); + + var result = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.False(result.Success); + Assert.Equal("corr-shed-1", result.CorrelationId); + Assert.Contains("shed", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); + Assert.Contains("in flight", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); + + // Still exactly four in flight — the shed run was never launched. + Assert.Equal(4, actor.UnderlyingActor.RunsInFlight); + + // Sixth: counted again, but the Warning site event is rate-limited to one per minute. + actor.Tell(new ScriptCallRequest("Hot", null, 0, "corr-shed-2"), caller.Ref); + var second = caller.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.False(second.Success); + + AwaitAssert(() => + { + var shedEvents = siteLog.OfType("script") + .Where(r => r.Severity == "Warning" && r.Message.Contains("shed")) + .ToArray(); + Assert.Single(shedEvents); + Assert.Equal("ScriptActor:Hot", shedEvents[0].Source); + Assert.Equal("Inst1", shedEvents[0].InstanceId); + }, TimeSpan.FromSeconds(5)); + + // Both sheds were counted on the health report even though only one was evented. + Assert.Equal(2, health.CollectReport("site-1").ScriptRunShedCount); + + // One completion frees a slot, and the next trigger launches again. + ShedHooks.Gate.Release(); + AwaitAssert(() => Assert.Equal(3, actor.UnderlyingActor.RunsInFlight), TimeSpan.FromSeconds(15)); + + actor.Tell(new ScriptCallRequest("Hot", null, 0, "corr-after"), ActorRefs.NoSender); + AwaitAssert(() => Assert.Equal(4, actor.UnderlyingActor.RunsInFlight), TimeSpan.FromSeconds(15)); + } + + [Fact] + public void AlarmOnTriggerRuns_ShedAtTheSameCap_WithAnAlarmScopedSiteEvent() + { + var siteLog = new FakeSiteEventLogger(); + var health = new SiteHealthCollector(); + var instanceProbe = CreateTestProbe(); + var options = Options(); + + var alarm = ActorOfAsTestActorRef( + Props.Create(() => new AlarmActor( + "Flapper", "Inst1", instanceProbe.Ref, + new ResolvedAlarm + { + CanonicalName = "Flapper", + TriggerType = "ValueMatch", + TriggerConfiguration = "{\"attributeName\":\"Status\",\"matchValue\":\"Critical\"}", + PriorityLevel = 100 + }, + BlockingScript(), _sharedLibrary, options, NullLogger.Instance, + null, null, health, new SingleServiceProvider(siteLog), null, _scheduler, null)), + "alarm-shed-" + Guid.NewGuid().ToString("N")); + + // Each raise edge spawns one on-trigger run; clear between raises to re-arm the edge. + void Flap(int cycle) + { + alarm.Tell(new AttributeValueChanged( + "Inst1", "Status", "Status", "Critical", "Good", DateTimeOffset.UtcNow.AddSeconds(cycle))); + alarm.Tell(new AttributeValueChanged( + "Inst1", "Status", "Status", "Normal", "Good", DateTimeOffset.UtcNow.AddSeconds(cycle))); + } + + for (var i = 0; i < 4; i++) Flap(i); + AwaitAssert(() => Assert.Equal(4, alarm.UnderlyingActor.RunsInFlight), TimeSpan.FromSeconds(15)); + + // Fifth raise is shed — there is no Ask caller on this path, so it surfaces purely as + // a counter plus the rate-limited Warning event. + Flap(4); + Flap(5); + + AwaitAssert(() => + { + var shedEvents = siteLog.OfType("script") + .Where(r => r.Severity == "Warning" && r.Message.Contains("shed")) + .ToArray(); + Assert.Single(shedEvents); + Assert.Equal("AlarmActor:Flapper", shedEvents[0].Source); + }, TimeSpan.FromSeconds(10)); + + Assert.Equal(4, alarm.UnderlyingActor.RunsInFlight); + Assert.Equal(2, health.CollectReport("site-1").ScriptRunShedCount); + } +} + +/// Test hook used to hold script runs in flight while the cap is exercised. +public static class ShedHooks +{ + /// Gate the blocking test scripts wait on; reset per test. + public static SemaphoreSlim Gate = new(0); +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/StuckScriptWatchdogTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/StuckScriptWatchdogTests.cs new file mode 100644 index 00000000..84714c0e --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/StuckScriptWatchdogTests.cs @@ -0,0 +1,195 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 test group 4 — the stuck-script watchdog now REPLACES the thread a wedged script is +/// holding, not just names it. +/// +/// Before WP3.1 the watchdog was observability only: a script blocked in synchronous, +/// uninterruptible I/O never observes the cooperative cancellation the timeout requests, so +/// the pool simply lost that dedicated thread — permanently, and silently apart from one log +/// line. Eight such scripts left the site with no script execution at all. The watchdog now +/// detaches the worker (it exits when its body finally returns) and starts a replacement, and +/// the count of live detached workers is surfaced on the site health report as +/// DetachedScriptThreads. Replacement is capped at the pool size, because bounded +/// starvation beats unbounded thread growth when scripts wedge en masse. +/// +public class StuckScriptWatchdogTests : TestKit, IDisposable +{ + private readonly SharedScriptLibrary _sharedLibrary; + + public StuckScriptWatchdogTests() + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + WatchdogHooks.Gate = new SemaphoreSlim(0); + } + + void IDisposable.Dispose() + { + // Free any still-wedged worker before tearing down, so no test leaves a blocked thread. + WatchdogHooks.Gate.Release(8); + Shutdown(); + } + + private static Script CompileRaw(string code) + { + var scriptOptions = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, + typeof(WatchdogHooks).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(code, scriptOptions, typeof(ScriptGlobals)); + script.Compile(); + return script; + } + + /// A body that blocks its worker thread outright — cooperative cancellation is never observed. + private static Script WedgeScript() => CompileRaw( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.WatchdogHooks.Gate.Wait(); return null;"); + + private IActorRef BuildScriptActor( + string name, + Script compiled, + SiteRuntimeOptions options, + ScriptExecutionScheduler scheduler, + IServiceProvider? serviceProvider) + { + var instance = CreateTestProbe().Ref; + var config = new ResolvedScript { CanonicalName = name, TriggerType = "Call" }; + return ActorOf(Props.Create(() => new ScriptActor( + name, "Inst1", instance, compiled, config, _sharedLibrary, options, + NullLogger.Instance, null, null, null, serviceProvider, scheduler, null))); + } + + private static SiteRuntimeOptions WatchdogOptions() => new() + { + ScriptExecutionTimeoutSeconds = 1, + StuckScriptGraceMs = 200, + MaxScriptCallDepth = 10 + }; + + [Fact] + public void WedgedScript_DetachesItsWorker_StartsAReplacement_AndDrainsWhenFreed() + { + using var scheduler = new ScriptExecutionScheduler(1); + var siteLog = new FakeSiteEventLogger(); + var options = WatchdogOptions(); + + var wedged = BuildScriptActor("Wedged", WedgeScript(), options, scheduler, + new SingleServiceProvider(siteLog)); + wedged.Tell(new ScriptCallRequest("Wedged", null, 0, "corr-wedge"), ActorRefs.NoSender); + + // Timeout (1 s) + grace (200 ms) later the watchdog fires: the worker is detached and + // a replacement thread starts. + AwaitAssert( + () => Assert.Equal(1, scheduler.DetachedThreadCount), + TimeSpan.FromSeconds(15)); + + AwaitAssert( + () => Assert.Contains(siteLog.OfType("script"), r => + r.Severity == "Error" && + r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase) && + r.Message.Contains("Wedged") && + r.Message.Contains("DETACHED")), + TimeSpan.FromSeconds(5)); + + // The replacement worker is live: a fresh script runs even though the pool's only + // original thread is still blocked. Before WP3.1 this reply never came. + var healthy = BuildScriptActor("Healthy", CompileRaw("return 7;"), options, scheduler, null); + var caller = CreateTestProbe(); + healthy.Tell(new ScriptCallRequest("Healthy", null, 0, "corr-healthy"), caller.Ref); + var result = caller.ExpectMsg(TimeSpan.FromSeconds(15)); + Assert.True(result.Success, result.ErrorMessage); + Assert.Equal(7, result.ReturnValue); + + // Freeing the wedged body lets the detached worker exit rather than pull more work, + // so the pool does not silently end up double-sized. + WatchdogHooks.Gate.Release(); + AwaitAssert( + () => Assert.Equal(0, scheduler.DetachedThreadCount), + TimeSpan.FromSeconds(15)); + Assert.Equal(1, scheduler.MaximumConcurrencyLevel); + } + + [Fact] + public void AtTheDetachCap_NoFurtherReplacementIsStarted_AndAnErrorEventIsEmitted() + { + // A one-thread pool caps live detached workers at one, so the SECOND wedge cannot be + // replaced — the deliberate "bounded starvation beats unbounded thread growth" trade. + using var scheduler = new ScriptExecutionScheduler(1); + var siteLog = new FakeSiteEventLogger(); + var options = WatchdogOptions(); + + var first = BuildScriptActor("WedgeOne", WedgeScript(), options, scheduler, + new SingleServiceProvider(siteLog)); + first.Tell(new ScriptCallRequest("WedgeOne", null, 0, "corr-w1"), ActorRefs.NoSender); + AwaitAssert(() => Assert.Equal(1, scheduler.DetachedThreadCount), TimeSpan.FromSeconds(15)); + + var second = BuildScriptActor("WedgeTwo", WedgeScript(), options, scheduler, + new SingleServiceProvider(siteLog)); + second.Tell(new ScriptCallRequest("WedgeTwo", null, 0, "corr-w2"), ActorRefs.NoSender); + + AwaitAssert( + () => Assert.Contains(siteLog.OfType("script"), r => + r.Severity == "Error" && + r.Message.Contains("WedgeTwo") && + r.Message.Contains("at cap", StringComparison.OrdinalIgnoreCase)), + TimeSpan.FromSeconds(20)); + + // Still exactly one detached worker: the second wedge was NOT replaced. + Assert.Equal(1, scheduler.DetachedThreadCount); + + WatchdogHooks.Gate.Release(2); + } + + [Fact] + public async Task DetachedThreadCount_IsSurfacedOnTheSiteHealthReport() + { + using var scheduler = new ScriptExecutionScheduler(1); + var collector = new SiteHealthCollector(); + + var wedged = BuildScriptActor("GaugeWedge", WedgeScript(), WatchdogOptions(), scheduler, null); + wedged.Tell(new ScriptCallRequest("GaugeWedge", null, 0, "corr-gauge"), ActorRefs.NoSender); + AwaitAssert(() => Assert.Equal(1, scheduler.DetachedThreadCount), TimeSpan.FromSeconds(15)); + + using var reporter = new ScriptSchedulerStatsReporter( + collector, WatchdogOptions(), NullLogger.Instance, + pollInterval: TimeSpan.FromMilliseconds(50), scheduler: scheduler); + await reporter.StartAsync(CancellationToken.None); + try + { + AwaitAssert( + () => Assert.Equal(1, collector.CollectReport("site-1").DetachedScriptThreads), + TimeSpan.FromSeconds(10)); + } + finally + { + await reporter.StopAsync(CancellationToken.None); + WatchdogHooks.Gate.Release(); + } + } +} + +/// +/// Test hook the stuck-script watchdog tests use to block a script-execution thread +/// deterministically: the compiled body waits on , which the test releases +/// once it has observed the detach. +/// +public static class WatchdogHooks +{ + /// Gate a blocking test script waits on; reset per test. + public static SemaphoreSlim Gate = new(0); +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/TriggerEvalStarvationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/TriggerEvalStarvationTests.cs new file mode 100644 index 00000000..47da020f --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/TriggerEvalStarvationTests.cs @@ -0,0 +1,196 @@ +using System.Diagnostics; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming; +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.Scripts; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// WP3.1 test groups 1 and 2 — the regression pin for arch-review finding #4 (High). +/// +/// The finding. Trigger-expression evaluation used to be queued onto the same +/// fixed 8-thread that runs blocking script bodies. Eight +/// scripts blocked in synchronous I/O therefore stalled EVERY Expression trigger on the node — +/// scripts and alarms alike — for an unbounded time. Worse, the evaluation's 2 s timeout was +/// constructed INSIDE the queued body, so it did not start until the evaluation was dequeued: +/// the operator saw neither a raise nor a timeout, just silence. +/// +/// The fix these tests pin. Evaluations are non-blocking by construction +/// ( exposes only reads over an in-memory snapshot, and +/// the trust gate has already denied I/O), so they run as plain async work on the shared .NET +/// thread pool behind — never on the blocking pool. And their +/// deadline is armed at ENQUEUE, so gate-wait time burns the same budget. +/// +public class TriggerEvalStarvationTests : TestKit, IDisposable +{ + private readonly SharedScriptLibrary _sharedLibrary; + + public TriggerEvalStarvationTests() + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + StarvationHooks.Gate = new SemaphoreSlim(0); + } + + void IDisposable.Dispose() + { + StarvationHooks.Gate.Release(32); + Shutdown(); + } + + private static Script CompileRaw(string code) + { + var options = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly, + typeof(StarvationHooks).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(code, options, typeof(ScriptGlobals)); + script.Compile(); + return script; + } + + private static Script CompileTriggerExpression(string expression) + { + var options = ScriptOptions.Default + .WithReferences(typeof(object).Assembly, typeof(Enumerable).Assembly) + .WithImports("System", "System.Collections.Generic", "System.Linq", "System.Threading.Tasks"); + var script = CSharpScript.Create(expression, options, typeof(TriggerExpressionGlobals)); + script.Compile(); + return script; + } + + /// + /// GROUP 1 — the finding-#4 regression pin. Eight script bodies blocked on a semaphore + /// occupy every thread of an 8-thread pool; an Expression-triggered alarm must still raise + /// in well under 2 s. + /// + /// On the pre-WP3.1 wiring this test does not merely fail slowly — it never + /// completes: the evaluation sits in the same FIFO behind eight bodies that only unblock + /// after the assertion window, and its 2 s timeout has not even started ticking. + /// + [Fact] + public void EightBlockedScripts_DoNotDelayAnAlarmsExpressionEvaluation() + { + using var scheduler = new ScriptExecutionScheduler(8); + using var evalGate = new TriggerEvalGate(4); + + // Occupy every thread of the blocking pool with a real script run. + var wedge = CompileRaw( + "ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors.StarvationHooks.Gate.Wait(); return null;"); + for (var i = 0; i < 8; i++) + { + var name = $"Blocker{i}"; + var config = new ResolvedScript { CanonicalName = name, TriggerType = "Call" }; + var instance = CreateTestProbe().Ref; + var actor = ActorOf(Props.Create(() => new ScriptActor( + name, "Inst1", instance, wedge, config, _sharedLibrary, new SiteRuntimeOptions(), + NullLogger.Instance, null, null, null, null, scheduler, evalGate))); + actor.Tell(new ScriptCallRequest(name, null, 0, $"corr-block-{i}"), ActorRefs.NoSender); + } + + AwaitAssert( + () => Assert.Equal(8, scheduler.BusyThreadCount), + TimeSpan.FromSeconds(15)); + + // Now fire an Expression-triggered alarm through the SAME scheduler seam. + var instanceProbe = CreateTestProbe(); + var alarm = ActorOf(Props.Create(() => new AlarmActor( + "ExprAlarm", "Inst1", instanceProbe.Ref, + new ResolvedAlarm + { + CanonicalName = "ExprAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"true\"}", + PriorityLevel = 900 + }, + null, _sharedLibrary, new SiteRuntimeOptions(), NullLogger.Instance, + CompileTriggerExpression("true"), null, null, null, null, scheduler, evalGate))); + + var stopwatch = Stopwatch.StartNew(); + alarm.Tell(new AttributeValueChanged( + "Inst1", "Temp", "Temp", 99.0, "Good", DateTimeOffset.UtcNow)); + + var raised = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(2)); + stopwatch.Stop(); + + Assert.Equal(AlarmState.Active, raised.State); + Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(2), + $"alarm raised only after {stopwatch.Elapsed} — the evaluation queued behind blocked script bodies"); + + // The pool really was saturated for the whole window; the eval simply never used it. + Assert.Equal(8, scheduler.BusyThreadCount); + } + + /// + /// GROUP 2 — the evaluation deadline is measured from ENQUEUE, so time spent waiting on a + /// saturated burns the same budget. With the gate fully + /// occupied for the whole assertion window, a queued evaluation must still resolve (as + /// false) at its timeout rather than stalling indefinitely — and the trigger must drain + /// rather than park, so the next change still evaluates. + /// + [Fact] + public async Task SaturatedEvalGate_ResolvesTheQueuedEvaluationAtItsEnqueueAnchoredDeadline() + { + using var scheduler = new ScriptExecutionScheduler(1); + using var evalGate = new TriggerEvalGate(1); + var options = new SiteRuntimeOptions { TriggerEvalTimeoutSeconds = 1 }; + + var instanceProbe = CreateTestProbe(); + var alarm = ActorOf(Props.Create(() => new AlarmActor( + "ExprAlarm", "Inst1", instanceProbe.Ref, + new ResolvedAlarm + { + CanonicalName = "ExprAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"true\"}", + PriorityLevel = 500 + }, + null, _sharedLibrary, options, NullLogger.Instance, + CompileTriggerExpression("true"), null, null, null, null, scheduler, evalGate))); + + // 1. Free gate: the expression evaluates true and the alarm raises. + alarm.Tell(new AttributeValueChanged("Inst1", "A", "A", 1, "Good", DateTimeOffset.UtcNow)); + var raised = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal(AlarmState.Active, raised.State); + + // 2. Occupy the only permit for the whole of the next step. + await evalGate.WaitAsync(CancellationToken.None); + Assert.Equal(0, evalGate.AvailablePermits); + + // 3. The next evaluation can never acquire the gate. Its deadline was armed at + // enqueue, so it cancels at ~1 s, is treated as false, and clears the alarm. + // Pre-WP3.1 the clock started at dequeue, so this would hang forever. + var stopwatch = Stopwatch.StartNew(); + alarm.Tell(new AttributeValueChanged("Inst1", "A", "A", 2, "Good", DateTimeOffset.UtcNow)); + var cleared = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + stopwatch.Stop(); + + Assert.Equal(AlarmState.Normal, cleared.State); + Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(6), + $"queued evaluation took {stopwatch.Elapsed} to resolve — the deadline is not enqueue-anchored"); + Assert.Equal(0, evalGate.AvailablePermits); // still held: it genuinely never ran + + // 4. Not parked: releasing the gate and sending another change evaluates again. + evalGate.Release(); + alarm.Tell(new AttributeValueChanged("Inst1", "A", "A", 3, "Good", DateTimeOffset.UtcNow)); + var reRaised = instanceProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal(AlarmState.Active, reRaised.State); + } +} + +/// Test hook used to block script-execution worker threads deterministically. +public static class StarvationHooks +{ + /// Gate the blocking test scripts wait on; reset per test. + public static SemaphoreSlim Gate = new(0); +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/AlarmsAccessorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/AlarmsAccessorTests.cs index 65ad6949..eb35797c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/AlarmsAccessorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/AlarmsAccessorTests.cs @@ -20,7 +20,6 @@ public class AlarmsAccessorTests : TestKit, IDisposable { private ScriptRuntimeContext MakeContext(IActorRef instanceActor) => new( - instanceActor, instanceActor, sharedScriptLibrary: null!, currentCallDepth: 0, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs index 59fc0891..21929b9a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs @@ -75,7 +75,6 @@ public class ExecutionCorrelationContextTests compilationService, NullLogger.Instance); return new ScriptRuntimeContext( - ActorRefs.Nobody, ActorRefs.Nobody, sharedScriptLibrary, currentCallDepth: 0, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ParentExecutionTreeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ParentExecutionTreeTests.cs index 84222a27..c886a005 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ParentExecutionTreeTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ParentExecutionTreeTests.cs @@ -83,7 +83,6 @@ public class ParentExecutionTreeTests : TestKit { return new ScriptRuntimeContext( instanceActor, - ActorRefs.Nobody, library, currentCallDepth: 0, maxCallDepth: 10, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs index 3843d08d..f3c3e00e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs @@ -32,7 +32,6 @@ public class RecursionLimitSiteEventTests compilationService, NullLogger.Instance); return new ScriptRuntimeContext( - ActorRefs.Nobody, ActorRefs.Nobody, sharedScriptLibrary, currentCallDepth: maxCallDepth, // already AT the limit diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs index 573628f2..6d885f2b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs @@ -156,7 +156,6 @@ public class AttributeAccessorWaitAsyncTests : TestKit, IDisposable { private ScriptRuntimeContext MakeContext(IActorRef instanceActor) => new( - instanceActor, instanceActor, sharedScriptLibrary: null!, currentCallDepth: 0, diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptPoolSizingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptPoolSizingTests.cs new file mode 100644 index 00000000..0280db31 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptPoolSizingTests.cs @@ -0,0 +1,103 @@ +using ZB.MOM.WW.ScadaBridge.SiteRuntime; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; + +/// +/// WP3.1 test group 5 — the blocking script pool is no longer a fixed 8 threads forever. It +/// scales with the number of running instances between a configured floor and ceiling, and it +/// is deliberately GROW-ONLY: undeploying instances leaves idle threads (which cost nothing +/// measurable) rather than paying for drain/steal complexity. +/// +public class ScriptPoolSizingTests +{ + private static SiteRuntimeOptions Options(int floor = 8, int ceiling = 32) => new() + { + ScriptExecutionThreadCount = floor, + ScriptExecutionMaxThreadCount = ceiling + }; + + [Theory] + // At or below floor * 8 instances the result is exactly the pre-WP3.1 fixed size — + // existing configurations are byte-for-byte unchanged in behaviour. + [InlineData(0, 8)] + [InlineData(1, 8)] + [InlineData(64, 8)] + // Past that, one thread per 8 instances, rounding up. + [InlineData(65, 9)] + [InlineData(72, 9)] + [InlineData(200, 25)] + // …clamped at the ceiling. + [InlineData(256, 32)] + [InlineData(10_000, 32)] + public void ComputeTargetThreads_AppliesFloorRatioAndCeiling(int instances, int expected) + => Assert.Equal(expected, ScriptExecutionScheduler.ComputeTargetThreads(instances, Options())); + + [Fact] + public void ComputeTargetThreads_HonoursAnOverriddenFloorAndCeiling() + { + var options = Options(floor: 2, ceiling: 4); + Assert.Equal(2, ScriptExecutionScheduler.ComputeTargetThreads(0, options)); + Assert.Equal(2, ScriptExecutionScheduler.ComputeTargetThreads(16, options)); + Assert.Equal(3, ScriptExecutionScheduler.ComputeTargetThreads(17, options)); + Assert.Equal(4, ScriptExecutionScheduler.ComputeTargetThreads(1000, options)); + } + + [Fact] + public void ComputeTargetThreads_NeverReturnsLessThanOne_EvenWithADegenerateFloor() + { + // The validator rejects these, but a directly-constructed options object must still + // not produce a zero-thread scheduler. + var options = new SiteRuntimeOptions { ScriptExecutionThreadCount = 0, ScriptExecutionMaxThreadCount = 0 }; + Assert.Equal(1, ScriptExecutionScheduler.ComputeTargetThreads(0, options)); + Assert.Equal(1, ScriptExecutionScheduler.ComputeTargetThreads(500, options)); + } + + [Fact] + public void EnsureCapacity_GrowsOnce_IsIdempotent_AndNeverShrinks() + { + using var scheduler = new ScriptExecutionScheduler(2); + Assert.Equal(2, scheduler.MaximumConcurrencyLevel); + + Assert.Equal(5, scheduler.EnsureCapacity(5)); + Assert.Equal(5, scheduler.MaximumConcurrencyLevel); + + // Idempotent: asking for the same target again changes nothing. + Assert.Equal(5, scheduler.EnsureCapacity(5)); + Assert.Equal(5, scheduler.MaximumConcurrencyLevel); + + // Grow-only: a smaller target is a no-op, not a shrink. + Assert.Equal(5, scheduler.EnsureCapacity(1)); + Assert.Equal(5, scheduler.MaximumConcurrencyLevel); + } + + [Fact] + public async Task EnsureCapacity_WidensTheGauges_SoTheWholePoolIsObservable() + { + using var scheduler = new ScriptExecutionScheduler(1); + scheduler.EnsureCapacity(3); + + using var gate = new ManualResetEventSlim(false); + var blocking = Enumerable.Range(0, 3) + .Select(_ => Task.Factory.StartNew(() => gate.Wait(), + CancellationToken.None, TaskCreationOptions.None, scheduler)) + .ToArray(); + + // All three grown workers report busy — the bookkeeping widened with the pool. + await WaitUntilAsync(() => scheduler.BusyThreadCount == 3); + Assert.Equal(0, scheduler.QueueDepth); + Assert.NotNull(scheduler.OldestBusyAge); + + gate.Set(); + await Task.WhenAll(blocking); + await WaitUntilAsync(() => scheduler.BusyThreadCount == 0); + Assert.Null(scheduler.OldestBusyAge); + } + + private static async Task WaitUntilAsync(Func condition) + { + for (var i = 0; i < 200 && !condition(); i++) + await Task.Delay(25); + Assert.True(condition(), "condition not met within timeout"); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs index 435863a9..d25f7e9c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs @@ -43,7 +43,7 @@ public class SiteScriptCompileCacheTests } [Fact] - public void Overflow_ClearsWholesale() + public void Overflow_StaysWithinTheBound() { SiteScriptCompileCache.Clear(); for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++) @@ -51,4 +51,88 @@ public class SiteScriptCompileCacheTests Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries); } + + // ── WP3.1: approximate LRU replaced the wholesale-Clear overflow cliff ────────── + + /// + /// WP3.1 (test group 8): overflow must evict only the OLDEST batch, keeping hot entries. + /// The old behaviour cleared all 1024 entries, so the very next deploy or Instance-Actor + /// start paid a full recompile storm on actor threads. + /// + [Fact] + public void Overflow_EvictsOldestBatch_AndKeepsRecentlyTouchedEntries() + { + // Note: the cache is process-wide static and other test classes compile into it + // concurrently, so the assertions below are phrased as properties of the eviction + // policy (what survives, what does not, nothing is wiped) rather than as exact + // counts, which no test can own here. + SiteScriptCompileCache.Clear(); + + // Fill to the bound. Entries 1..N are inserted oldest-first. + for (var i = 0; i < SiteScriptCompileCache.MaxEntries; i++) + SiteScriptCompileCache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok); + + // Touch entry 0 so it carries the NEWEST access stamp despite being inserted first. + var hot = SiteScriptCompileCache.GetOrAdd("return 0;", typeof(ScriptGlobals), Ok); + var hitsAfterTouch = SiteScriptCompileCache.Hits; + + // Push well past the bound so at least one eviction sweep definitely runs. + for (var i = 0; i < 200; i++) + SiteScriptCompileCache.GetOrAdd($"return overflow{i};", typeof(ScriptGlobals), Ok); + + // Nothing was cleared wholesale: the cache is still most of the way full and the hit + // counter survived (Clear() would have reset it to 0). + Assert.True(SiteScriptCompileCache.Count > SiteScriptCompileCache.MaxEntries / 2, + $"cache collapsed to {SiteScriptCompileCache.Count} entries — this looks like a wholesale clear"); + Assert.True(SiteScriptCompileCache.Hits >= hitsAfterTouch); + + // The touched entry survived the sweep — a hit, not a recompile. This is the LRU + // property: recency, not insertion order, decides what stays. + var hitsBefore = SiteScriptCompileCache.Hits; + var again = SiteScriptCompileCache.GetOrAdd("return 0;", typeof(ScriptGlobals), + () => throw new InvalidOperationException("hot entry was evicted — LRU is not keeping recently-used entries")); + Assert.Same(hot, again); + Assert.Equal(hitsBefore + 1, SiteScriptCompileCache.Hits); + + // …and untouched old entries genuinely were evicted, so the sweep really ran. + var recomputed = 0; + for (var i = 1; i <= 16; i++) + { + var code = $"return {i};"; + SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), + () => { recomputed++; return Ok(); }); + } + Assert.True(recomputed > 0, "no old entry was evicted — the overflow sweep did not run"); + } + + /// + /// WP3.1: concurrent inserts crossing the bound together must not blow past it — the + /// eviction sweep is double-checked under its own lock precisely so a stampede performs + /// one sweep rather than one per racing thread. + /// + [Fact] + public async Task ConcurrentGetOrAddStorm_KeepsTheBound() + { + SiteScriptCompileCache.Clear(); + + const int perTask = 400; + var tasks = Enumerable.Range(0, 8).Select(t => Task.Run(() => + { + for (var i = 0; i < perTask; i++) + SiteScriptCompileCache.GetOrAdd($"return {t}_{i};", typeof(ScriptGlobals), Ok); + })); + + await Task.WhenAll(tasks); + + // A small transient overshoot is by design: threads that lose the double-checked + // eviction race still insert their own entry afterwards, so the ceiling is "bounded", + // not "never exceeded by one". The property under test is that 3200 distinct inserts + // do not accumulate — before WP3.1 this was a wholesale Clear(), and a per-entry + // eviction bug would show up here as unbounded growth, not as a handful of extra rows. + // (Other test classes compile into this process-wide cache concurrently, which is the + // other reason the bound is asserted with slack rather than exactly.) + Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + 512, + $"cache grew past its bound: {SiteScriptCompileCache.Count} vs max {SiteScriptCompileCache.MaxEntries} " + + $"after {8 * perTask} distinct inserts"); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs index 13a6b419..cb6b0356 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/SiteRuntimeOptionsValidatorTests.cs @@ -84,4 +84,64 @@ public class SiteRuntimeOptionsValidatorTests Assert.True(result.Failed); Assert.Contains("AlarmPublishQueueCapacity", result.FailureMessage); } + + // ── WP3.1 options ────────────────────────────────────────────────────────────── + + /// + /// The ceiling is the upper bound of the instance-scaled pool; below the floor it would + /// silently shrink the configured pool instead of growing it. + /// + [Fact] + public void ScriptExecutionMaxThreadCountBelowThreadCount_IsRejected() + { + var result = Validate(new SiteRuntimeOptions + { + ScriptExecutionThreadCount = 8, + ScriptExecutionMaxThreadCount = 4 + }); + + Assert.True(result.Failed); + Assert.Contains("ScriptExecutionMaxThreadCount", result.FailureMessage); + } + + [Fact] + public void EqualScriptExecutionThreadCountAndMax_IsAccepted() + { + var result = Validate(new SiteRuntimeOptions + { + ScriptExecutionThreadCount = 8, + ScriptExecutionMaxThreadCount = 8 + }); + + Assert.True(result.Succeeded, result.FailureMessage); + } + + /// A zero gate would park every Expression trigger on the node forever. + [Fact] + public void ZeroTriggerEvalMaxConcurrency_IsRejected() + { + var result = Validate(new SiteRuntimeOptions { TriggerEvalMaxConcurrency = 0 }); + + Assert.True(result.Failed); + Assert.Contains("TriggerEvalMaxConcurrency", result.FailureMessage); + } + + [Fact] + public void ZeroTriggerEvalTimeoutSeconds_IsRejected() + { + var result = Validate(new SiteRuntimeOptions { TriggerEvalTimeoutSeconds = 0 }); + + Assert.True(result.Failed); + Assert.Contains("TriggerEvalTimeoutSeconds", result.FailureMessage); + } + + /// A zero cap would shed every trigger, silently disabling all scripts. + [Fact] + public void ZeroMaxConcurrentRunsPerScript_IsRejected() + { + var result = Validate(new SiteRuntimeOptions { MaxConcurrentRunsPerScript = 0 }); + + Assert.True(result.Failed); + Assert.Contains("MaxConcurrentRunsPerScript", result.FailureMessage); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/TestSupport/ScopeSpyServiceProvider.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/TestSupport/ScopeSpyServiceProvider.cs new file mode 100644 index 00000000..bd12adff --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/TestSupport/ScopeSpyServiceProvider.cs @@ -0,0 +1,48 @@ +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.SiteEventLogging; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; + +/// +/// WP3.1 parity pin 3: an that counts +/// calls and each scope's +/// , so a test can assert that one script run creates +/// exactly one DI scope and disposes it exactly once — on the success, failure, AND timeout +/// paths alike. Removing the per-run execution actor moved the scope's finally into +/// ; a scope leaked +/// there would silently leak every scoped service a script touches. +/// +public sealed class ScopeSpyServiceProvider(ISiteEventLogger? logger = null) + : IServiceProvider, IServiceScopeFactory +{ + private int _scopesCreated; + private int _scopesDisposed; + + /// Scopes created so far. + public int ScopesCreated => Volatile.Read(ref _scopesCreated); + + /// Scope disposals observed so far (double-disposal is counted twice, deliberately). + public int ScopesDisposed => Volatile.Read(ref _scopesDisposed); + + /// + public object? GetService(Type serviceType) + { + if (serviceType == typeof(ISiteEventLogger)) return logger; + if (serviceType == typeof(IServiceScopeFactory)) return this; + return null; + } + + /// + public IServiceScope CreateScope() + { + Interlocked.Increment(ref _scopesCreated); + return new SpyScope(this); + } + + private sealed class SpyScope(ScopeSpyServiceProvider owner) : IServiceScope + { + public IServiceProvider ServiceProvider => owner; + + public void Dispose() => Interlocked.Increment(ref owner._scopesDisposed); + } +}