docs(plans): script pool split design — WP3.1a
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,412 @@
|
||||
# Script Pool Split — Design Memo (WP3.1 stage a)
|
||||
|
||||
**Date:** 2026-08-15 · **Status:** APPROVED FOR IMPLEMENTATION (WP3.1 stage b)
|
||||
**Source:** `docs/plans/2026-08-14-arch-review-remediation-plan.md` §5 WP3.1 — findings #4 (High:
|
||||
trigger evals starve behind blocking scripts on one fixed 8-thread pool) + actor-per-execution
|
||||
overhead and the `SiteScriptCompileCache` wholesale-Clear cliff (Med).
|
||||
**Scope:** design only. Implementation, tests, and the `Component-SiteRuntime.md` /
|
||||
CLAUDE.md doc updates land in stage (b).
|
||||
|
||||
---
|
||||
|
||||
## 0. Current machinery (evidence baseline)
|
||||
|
||||
All line numbers verified 2026-08-15 on `arch-review-remediation` (post-Phase-2, tip `a5882753`).
|
||||
|
||||
| Mechanism | Where |
|
||||
|---|---|
|
||||
| One fixed pool for everything: `ScriptExecutionScheduler`, N dedicated threads, unbounded FIFO `BlockingCollection` | `SiteRuntime/Scripts/ScriptExecutionScheduler.cs:21-84` |
|
||||
| Pool size: `SiteRuntimeOptions.ScriptExecutionThreadCount`, static default 8, never scales | `SiteRuntime/SiteRuntimeOptions.cs:46` |
|
||||
| Script bodies run on the pool via short-lived `ScriptExecutionActor` (spawn-execute-die, ctor kicks the task) | `Actors/ScriptExecutionActor.cs:79-87,130,339` |
|
||||
| Alarm on-trigger bodies: same pattern, `AlarmExecutionActor` | `Actors/AlarmExecutionActor.cs:100-165` |
|
||||
| ScriptActor Expression-trigger eval: `Task.Factory.StartNew(..., scheduler)` on the SAME pool, hardcoded 2 s CTS created **inside** the body (timeout excludes queue wait) | `Actors/ScriptActor.cs:297-331` (2 s at :314) |
|
||||
| AlarmActor Expression-trigger eval: identical, plus `SourceExecutionId` snapshot capture | `Actors/AlarmActor.cs:578-619` (2 s at :600, capture at :591) |
|
||||
| Per-trigger coalescing (one in flight + one pending) already bounds eval fan-out per actor | `ScriptActor.cs:75-76,300`; `AlarmActor.cs:102-103,581` |
|
||||
| Script timeout CTS created inside the pooled body ⇒ the 30 s script budget also starts at **dequeue**, not enqueue | `ScriptExecutionActor.cs:136`; `AlarmExecutionActor.cs:102` |
|
||||
| Stuck-script watchdog: names the script, logs + site event — **observability only, the thread stays lost** | `ScriptExecutionActor.cs:147-161`; grace `SiteRuntimeOptions.cs:79` |
|
||||
| Pool gauges (queue depth / busy / oldest-busy age) → health report | `ScriptExecutionScheduler.cs:94-127`; `Scripts/ScriptSchedulerStatsReporter.cs:75-91` |
|
||||
| No per-script concurrency bound: every trigger spawns another child actor | `ScriptActor.cs:463-501`; `AlarmActor.cs:725-758` |
|
||||
| Compile cache: 1024 entries, **wholesale `Clear()` on overflow** | `Scripts/SiteScriptCompileCache.cs:37,71-72` |
|
||||
| Compile call sites on actor threads: deploy gate (deliberately synchronous, S3) and InstanceActor start | `Actors/DeploymentManagerActor.cs:522-556` (Validate at :540); `Actors/InstanceActor.cs:1631,1682,1795` |
|
||||
| In-repo off-dispatcher-compile reference: shared-script load = `Task.Run` + `PipeTo(Self)` | `DeploymentManagerActor.cs:1540-1563` |
|
||||
| Akka HOCON has no script dispatcher — the pool lives entirely outside Akka (only `audit-telemetry-dispatcher` is custom) | `Host/Actors/AkkaHostedService.cs:296-302` |
|
||||
|
||||
The failure mode (finding #4): 8 scripts blocked in synchronous I/O ⇒ every Expression trigger
|
||||
eval on the node (scripts **and alarms**) queues behind them. An alarm that should raise in
|
||||
milliseconds waits until a thread frees — unbounded. The 2 s eval CTS doesn't even start until
|
||||
the eval is dequeued, so the operator sees neither a timeout nor a raise.
|
||||
|
||||
---
|
||||
|
||||
## 1. (a) Trigger/expression evals leave the blocking pool
|
||||
|
||||
**Decision (confirms the plan default): evals run as plain async on the .NET thread pool
|
||||
(`Task.Run`), gated by a process-wide `SemaphoreSlim`. No second dedicated pool.**
|
||||
|
||||
Justification: trigger expressions are non-blocking **by construction** —
|
||||
`TriggerExpressionGlobals` exposes only reads over an in-memory snapshot dictionary ("no I/O,
|
||||
no actor Ask, no side-effecting APIs", `Scripts/TriggerExpressionGlobals.cs:5-13,42-50`), and
|
||||
the script trust gate has already denied I/O/network/threading before the expression ever
|
||||
deploys. They are short CPU-bound work — exactly what the shared thread pool is for. A second
|
||||
dedicated pool would add threads, gauges, and a second starvation surface for no isolation gain;
|
||||
the gate alone prevents an eval storm from swamping the pool.
|
||||
|
||||
Mechanics (both `ScriptActor.StartExpressionEvaluation` and `AlarmActor.StartExpressionEvaluation`):
|
||||
|
||||
```csharp
|
||||
var cts = new CancellationTokenSource(evalTimeout); // clock starts AT ENQUEUE
|
||||
Task.Run(async () =>
|
||||
{
|
||||
try
|
||||
{
|
||||
await gate.WaitAsync(cts.Token); // queue wait burns the same budget
|
||||
try
|
||||
{
|
||||
var state = await expression.RunAsync(new TriggerExpressionGlobals(snapshot), cts.Token);
|
||||
return state.ReturnValue is bool b && b;
|
||||
}
|
||||
finally { gate.Release(); }
|
||||
}
|
||||
catch (Exception ex) { /* existing treated-as-false path, incl. OCE */ }
|
||||
finally { cts.Dispose(); }
|
||||
}).PipeTo(self, success: r => new ExpressionEvalResult(r, ...), failure: ex => new ExpressionEvalFailed(ex, ...));
|
||||
```
|
||||
|
||||
- **Queue-inclusive timeout falls out of CTS placement**: today the CTS is constructed inside
|
||||
the pooled body (`ScriptActor.cs:314`, `AlarmActor.cs:600`), so its 2 s starts at dequeue.
|
||||
Moving construction to the actor thread before `Task.Run` makes gate-wait time part of the
|
||||
budget; a cancellation during `WaitAsync` flows through the existing catch → `false` → the
|
||||
existing error/edge handling (`HandleExpressionEvalResult` / `HandleExpressionEvalFailed`,
|
||||
which already survive faults per N2). No new message types.
|
||||
- **Gate**: one process-wide `SemaphoreSlim` (mirroring the `ScriptExecutionScheduler.Shared`
|
||||
lazy-singleton + injectable-seam pattern, `ScriptExecutionScheduler.cs:48-60` / the #18 ctor
|
||||
seams). Size: **`TriggerEvalMaxConcurrency`, default `max(2, Environment.ProcessorCount)`**.
|
||||
Per-trigger coalescing (unchanged) already caps waiters at ≤ one per Expression trigger, so
|
||||
the gate queue is bounded by trigger count.
|
||||
- **Timeout becomes configurable**: `TriggerEvalTimeoutSeconds`, default 2 (today hardcoded in
|
||||
two places). Validated > 0.
|
||||
- The tag-cascade capture is untouched: `sourceExecutionId` is still captured on the actor
|
||||
thread with the snapshot (`AlarmActor.cs:584-591`) and echoed on the result records
|
||||
(`AlarmActor.cs:888,899`) — only the execution context under the lambda changes.
|
||||
- `ScriptExecutionScheduler` keeps its `TryExecuteTaskInline` name-prefix guard and thread
|
||||
naming; nothing about the blocking pool's identity changes in this section.
|
||||
|
||||
Net effect: the starvation scenario is dead by construction — evals never share a queue or a
|
||||
thread with blocking script bodies.
|
||||
|
||||
---
|
||||
|
||||
## 2. (b) Blocking-pool sizing, config, watchdog thread replacement
|
||||
|
||||
### Sizing
|
||||
|
||||
**Decision (confirms plan default): `threads = clamp(max(ScriptExecutionThreadCount, ceil(enabledInstances / 8)), 1, ScriptExecutionMaxThreadCount)`.**
|
||||
|
||||
- `ScriptExecutionThreadCount` (existing, default 8, `SiteRuntimeOptions.cs:46`) keeps its name
|
||||
and becomes the **floor** — existing configs keep exactly today's behavior at ≤ 64 instances.
|
||||
- New `ScriptExecutionMaxThreadCount`, default **32** — a 1 MB-stack thread is cheap; 32 covers
|
||||
256 instances at the /8 ratio, and beyond that the per-script cap (§3) is the real regulator.
|
||||
Validated ≥ `ScriptExecutionThreadCount`.
|
||||
- The `/8` divisor is a named constant (`InstancesPerScriptThread = 8`), not another option —
|
||||
no evidence any deployment needs to tune the ratio independently of floor and ceiling.
|
||||
- **Grow-only.** `ScriptExecutionScheduler` gains `EnsureCapacity(int target)`: starts
|
||||
additional workers up to `target` (idempotent, lock-guarded, widens `_busySinceTicks`
|
||||
bookkeeping); it never shrinks — undeploying instances leaves idle threads, which cost
|
||||
nothing measurable and avoid drain/steal complexity.
|
||||
- **Call site**: `DeploymentManagerActor.UpdateInstanceCounts()` (`DeploymentManagerActor.cs:2095`,
|
||||
already invoked on every deploy/undeploy/enable/disable and per startup batch at :439, :485,
|
||||
:578, :645, :666, and `:485` per staggered batch) additionally calls
|
||||
`scheduler.EnsureCapacity(ComputeTargetThreads(enabled, options))`. `ComputeTargetThreads`
|
||||
is a pure static — unit-testable without threads.
|
||||
|
||||
### Watchdog replaces lost threads
|
||||
|
||||
Today the watchdog (CTS-register + grace delay, `ScriptExecutionActor.cs:147-161`) only *names*
|
||||
the wedged script; the pool permanently loses the thread. New mechanics:
|
||||
|
||||
1. **Slot capture at body start.** `WorkerLoop` (`ScriptExecutionScheduler.cs:129-150`) exposes
|
||||
`[ThreadStatic] internal static int? CurrentWorkerSlot`. The first synchronous segment of the
|
||||
script body records `(slot, busySinceTicks)` into the run's state. A truly wedged (blocking)
|
||||
script never leaves that thread, so the pair identifies its worker exactly; an async script
|
||||
that hopped threads will fail the guard below and cause no detach (correct — it holds no
|
||||
worker).
|
||||
2. **Detach on watchdog fire.** When the existing watchdog declares the script stuck (timeout +
|
||||
`StuckScriptGraceMs`), it additionally calls
|
||||
`scheduler.TryDetachWorker(slot, observedBusySince)`: atomically, if the slot's current
|
||||
`_busySinceTicks` still equals the recorded value (same task still running), mark the slot
|
||||
**detached** and start one replacement worker thread. The detached worker, on eventually
|
||||
finishing its task, sees its flag and exits instead of pulling more work — capacity never
|
||||
silently doubles-and-drains.
|
||||
3. **Bound.** Live detached threads are capped at the current pool size (worst case the process
|
||||
briefly holds 2× threads, all but N wedged). At the cap: no replacement, log Error + site
|
||||
event `script`/`Error` ("script-execution pool has N detached stuck threads; not replacing")
|
||||
— bounded starvation is preferable to unbounded thread growth when scripts wedge en masse.
|
||||
4. **Observability.** New gauge `DetachedScriptThreads` alongside the existing
|
||||
queue/busy/oldest-age stats through `ISiteHealthCollector.SetScriptSchedulerStats`
|
||||
(`ScriptSchedulerStatsReporter.cs:80-83` extends by one argument; additive on the health
|
||||
report DTO).
|
||||
|
||||
The watchdog itself (grace, message text, site event) is otherwise unchanged, and it keeps
|
||||
running on the shared thread pool, never on the possibly-saturated script pool
|
||||
(`ScriptExecutionActor.cs:146` comment holds).
|
||||
|
||||
### Script-body deadline captured at enqueue
|
||||
|
||||
Same CTS relocation as §1: the run's deadline CTS (30 s default / per-script override,
|
||||
`ScriptExecutionActor.cs:113-116,136`) is created **before** the body is queued to the
|
||||
scheduler, so queue wait consumes the script's own budget. Additionally, the launcher records
|
||||
`deadline = enqueueUtc + timeout`; if the body dequeues with the deadline already passed it
|
||||
**skips execution entirely** and takes the existing timeout path (site event + `IncrementScriptError`
|
||||
+ error reply + `ScriptExecutionCompleted(false)`) — a saturated pool now sheds stale work at
|
||||
dequeue instead of running it late. The watchdog registers on the same token, so its grace
|
||||
window is deadline-anchored too; a run cancelled while still queued flips `completed` before the
|
||||
grace check and is correctly not reported as a stuck thread.
|
||||
|
||||
---
|
||||
|
||||
## 3. (c) Per-script in-flight cap + shed policy
|
||||
|
||||
**Decision (confirms plan default): cap = 4 concurrent runs per script; overflow sheds the
|
||||
NEWEST (the incoming trigger), with a counter always and a rate-limited site event.**
|
||||
Justification stands as the plan gave it: a trigger-driven run that cannot even start before
|
||||
its own deadline is stale; keeping the 4 oldest (already queued/running, closest to their
|
||||
deadlines… and already charged against them) and refusing the newcomer is the policy that never
|
||||
reorders runs and needs no queue at all — the "bounded queue" in the plan collapses to a
|
||||
counter, because the scheduler's own FIFO *is* the queue and in-flight = enqueued-or-running.
|
||||
|
||||
Mechanics (identical in `ScriptActor` and `AlarmActor`):
|
||||
|
||||
- **Counter**: private `_runsInFlight` on the owning actor, incremented at launch
|
||||
(`SpawnExecution` / `SpawnAlarmExecution`), decremented on the **existing completion
|
||||
messages** — `ScriptActor.ScriptExecutionCompleted` (`ScriptActor.cs:648`, handled at :503)
|
||||
and `AlarmActor.AlarmExecutionCompleted` (`AlarmActor.cs:878`, handled at :221). Both are
|
||||
emitted on every path (success/timeout/failure) today and remain so under §4; the launch-path
|
||||
try/catch (§4, behavior 2) guarantees a completion message even when the launch itself
|
||||
throws, so the counter cannot leak.
|
||||
- **Config**: `MaxConcurrentRunsPerScript`, default 4, validated ≥ 1. One knob for both script
|
||||
and alarm on-trigger runs.
|
||||
- **Shed action** when `_runsInFlight >= cap`:
|
||||
- increment new `ISiteHealthCollector.IncrementScriptRunShed()` — surfaced on the site health
|
||||
report as a raw per-interval count like the existing script/alarm error counters
|
||||
(`ISiteHealthCollector.cs:16,21` pattern);
|
||||
- site event: channel `script`, severity `Warning`, source `ScriptActor:<name>` /
|
||||
`AlarmActor:<name>`, message naming script, instance, and the in-flight count — **rate-limited
|
||||
to one event per script per minute** (actor-local last-emit timestamp; the counter still
|
||||
counts every shed) so a hot trigger cannot flood `site_events` (WP3.2's concern);
|
||||
- **Ask callers are never silently dropped**: a shed `ScriptCallRequest`
|
||||
(`ScriptActor.cs:223-242`) replies
|
||||
`ScriptCallResult(correlationId, false, null, "shed: N runs already in flight")` — a nested
|
||||
`CallScript` or inbound-API route fails fast instead of hanging to Ask-timeout;
|
||||
- trigger-driven spawns (interval/value-change/conditional/expression, alarm on-trigger) are
|
||||
simply not launched.
|
||||
- `MinTimeBetweenRuns` (`ScriptActor.cs:443-453`) remains the first gate; the cap is the second.
|
||||
|
||||
---
|
||||
|
||||
## 4. (d) `ScriptExecutionActor` / `AlarmExecutionActor` elimination
|
||||
|
||||
**Decision: ELIMINATE both actors.** The pre-work for this memo shows the risk is materially
|
||||
lower than the plan feared, because the actors are already almost inert shells:
|
||||
|
||||
- **Neither actor has a single `Receive` handler** — they execute from the constructor
|
||||
(`ScriptExecutionActor.cs:79-87`; `AlarmExecutionActor.cs:61-68`) and the entire lifecycle
|
||||
(timeout, DI scope, telemetry, replies) lives inside a detached `Task` the actor does not
|
||||
observe.
|
||||
- **The execution actor's `IActorRef` is never a message target.** `ScriptRuntimeContext._self`
|
||||
is stored (`ScriptRuntimeContext.cs:43,243`) and passed to child shared-script contexts
|
||||
(`:305`) but never used in a `Tell`/`Ask` anywhere in the class. Asks to the instance actor
|
||||
use temporary ask-actors. Nothing routes through the shell.
|
||||
- **No `PostStop`, no state, no stash** — cleanup is the task's own `finally`
|
||||
(`ScriptExecutionActor.cs:329-338`), which merely poisons the shell.
|
||||
|
||||
What remains is per-run actor-cell/mailbox/name-registration overhead plus a per-spawn
|
||||
expression-tree `Props.Create` (`ScriptActor.cs:476-498`) — pure cost, no semantics.
|
||||
|
||||
### Replacement shape
|
||||
|
||||
New `Scripts/ScriptRunLauncher` (static class, same project): the current
|
||||
`ScriptExecutionActor.ExecuteScript` and `AlarmExecutionActor.ExecuteAlarmScript` bodies move
|
||||
there essentially verbatim (they are already `static`), unified behind two entry points
|
||||
(`LaunchScript`, `LaunchAlarmScript`) that take `completionTarget` (the owning coordinator's
|
||||
`Self`) instead of `self`/`parent`. `ScriptActor.SpawnExecution` and
|
||||
`AlarmActor.SpawnAlarmExecution` call it directly — no `Props`, no child. This also deletes
|
||||
~150 lines of duplicated body between the two actors.
|
||||
|
||||
### Behavior map — every current behavior → its replacement
|
||||
|
||||
| # | Current behavior (provider) | Replacement under direct launch |
|
||||
|---|---|---|
|
||||
| 1 | Execution starts immediately on spawn (ctor, `ScriptExecutionActor.cs:79-87`) | `ScriptRunLauncher.LaunchScript(...)` called inline in `SpawnExecution` — same thread, same ordering |
|
||||
| 2 | **Supervision Stop-on-failure** (`ScriptActor.cs:205-217`, `AlarmActor.cs:242-254`). Reachable today ONLY via ctor throw → `ActorInitializationException` (e.g. `StartNew` on a disposed scheduler; the body's exceptions never escape its try/catch). Effect: warn-log, child stopped, coordinator unaffected; the Ask caller **hangs** (no reply is sent on this path today). | `try/catch` around the launch call: warn-log with the same message shape, `replyTo.Tell(ScriptCallResult(false, error))` (an intentional improvement — the caller no longer hangs), `Self.Tell(ScriptExecutionCompleted(false, error))` for the §3 counter. The now-childless `SupervisorStrategy()` overrides are **deleted**; ScriptActor/AlarmActor remain supervised by InstanceActor (Resume) exactly as before — a launch-path throw must not reach InstanceActor, and the catch guarantees it. |
|
||||
| 3 | Script-body exception/timeout containment (in-lambda try/catch, `ScriptExecutionActor.cs:295-328`) | Moves verbatim into the launcher — unchanged |
|
||||
| 4 | **DI scope per run**: `CreateScope` at `:198`, disposed in `finally :335` | Unchanged — scope creation/disposal stays inside the launched task, one scope per run, disposed on every path |
|
||||
| 5 | **Telemetry**: `IncrementScriptError` (`:297,:314`), started/completed/error site events (`:275-320`), stuck-script watchdog (`:147-161`) | Unchanged — all live inside the launcher body. (Started/Completed Info events become sampled under WP3.2; not this WP's concern.) |
|
||||
| 6 | **ExecutionId / ParentExecutionId**: `ScriptRuntimeContext` mints its own `ExecutionId`; `parentExecutionId` threaded at `:247` (`AlarmExecutionActor.cs:121`); shared-script child contexts chain via `CreateChildContextForSharedScript` (`ScriptRuntimeContext.cs:302-330`) | Unchanged — the launcher builds the identical context. The dead `self` ctor argument is replaced with `ActorRefs.Nobody` in stage (b) and the parameter deleted (it is stored-and-passed but never messaged). |
|
||||
| 7 | Ask-caller reply from the task (`:284,:307,:324`) — `IActorRef.Tell` off-thread | Unchanged (`Tell` is thread-safe) |
|
||||
| 8 | Completion notification `parent.Tell(ScriptExecutionCompleted/AlarmExecutionCompleted)` (`:293,:310,:327`; `AlarmExecutionActor.cs:144,151,159`) | `completionTarget.Tell(...)` with the coordinator's captured `Self` — identical delivery, and now doubles as the §3 in-flight decrement |
|
||||
| 9 | `PoisonPill` self-stop in `finally` (`:337`) | Nothing to stop — dropped. The per-run actor teardown (cell, mailbox, name unregistration) simply disappears. |
|
||||
| 10 | Per-run actor name `{script}-exec-{n}` (`ScriptActor.cs:470,500`) — visible in Akka logs | `_executionCounter` survives as a run-id inside the launcher's log messages; the actor *path* is gone. Accepted cosmetic loss; log message content is preserved. |
|
||||
| 11 | **Dead letters / stop-during-redeploy**: `Context.Stop` on the instance subtree stops the shells but does NOT cancel the detached task; its later `parent.Tell` dead-letters (dead letters are a health metric). | Parity by construction: an in-flight run still runs to completion after its ScriptActor stops, and its `completionTarget.Tell` dead-letters identically. **Explicit decision: ScriptActor/AlarmActor `PostStop` does NOT cancel in-flight CTSs** — redeploy/undeploy semantics (let running scripts finish) are unchanged. Cancelling-on-stop is a separate behavior change; if wanted later it gets its own decision record. |
|
||||
| 12 | Exec-actor mailbox on the default dispatcher | Never received a message — no replacement needed |
|
||||
| 13 | `Terminated`-based redeploy sequencing (`DeploymentManagerActor.cs:508-556` waits for the instance subtree to fully stop) | Unaffected: exec shells were stopped instantly anyway (no children of their own); subtree termination time is dominated by Instance/Script/Alarm actors, which all remain |
|
||||
|
||||
### Parity pins stage (b) MUST implement as tests
|
||||
|
||||
1. Throwing script body ⇒ ScriptActor stays alive (no restart — assert no `PreRestart`), error
|
||||
site event + `IncrementScriptError`, Ask caller gets `ScriptCallResult(false)`,
|
||||
`ScriptExecutionCompleted(false)` arrives, in-flight counter returns to 0.
|
||||
2. Launch-path throw (inject a disposed `ScriptExecutionScheduler` via the #18 seam) ⇒
|
||||
ScriptActor neither dies nor restarts, caller gets an error reply (improved over today's
|
||||
hang — pin the reply), counter returns to 0.
|
||||
3. DI scope disposed exactly once per run on success, failure, AND timeout paths (spy provider).
|
||||
4. Audit threading: existing tag-cascade tests (`LastOnTriggerParentExecutionId`,
|
||||
`AlarmActor.cs:135`; `SeedAttributesReference` isolation contracts) pass unmodified; a
|
||||
routed `ScriptCallRequest.ParentExecutionId` still reaches the context.
|
||||
5. Timeout parity: per-script override and global fallback (`perScript ?? global`, ≤ 0 ⇒
|
||||
global) produce the same OperationCanceled path, site event, and watchdog behavior.
|
||||
6. Stop-during-run parity: stop the ScriptActor mid-run; the run completes, no crash, its
|
||||
completion message dead-letters (assert via dead-letter subscription), CTS not cancelled.
|
||||
7. Alarm side: on-trigger run gets `Alarm` globals (name/level/priority/message) and
|
||||
`AlarmExecutionCompleted` still reaches the AlarmActor — existing `ExecutionActorTests`
|
||||
reworked to launcher-level tests rather than deleted.
|
||||
|
||||
Fallback stance: if stage (b) uncovers a real consumer of the exec-actor `IActorRef` this memo
|
||||
missed, the cheap variant is pooled Props reuse — but the `_self` grep evidence above makes
|
||||
that contingency unlikely, and no design effort is spent on it here.
|
||||
|
||||
---
|
||||
|
||||
## 5. (e) `SiteScriptCompileCache` LRU + off-dispatcher cache-miss compiles
|
||||
|
||||
### LRU eviction (kills the overflow cliff)
|
||||
|
||||
The wholesale `Cache.Clear()` at 1024 entries (`SiteScriptCompileCache.cs:71-72`) is a latency
|
||||
cliff — the uncommitted `Component-SiteRuntime.md` WIP documents it precisely (instance scripts
|
||||
*and* trigger expressions share the cache; crossing the bound triggers a full recompile storm on
|
||||
actor threads). **This design resolves it; the WIP doc paragraph gets its "remains one" clause
|
||||
rewritten in stage (b).**
|
||||
|
||||
**Decision: approximate LRU via access-stamped entries + batch eviction; no wholesale clear.**
|
||||
|
||||
- Entry becomes `(ScriptCompilationResult Result, long LastAccess)` where `LastAccess` is a
|
||||
`Volatile`-written stamp from a process-wide `Interlocked` access sequence (not a clock —
|
||||
deterministic for tests, immune to clock steps).
|
||||
- Hits update the stamp lock-free; no lock, no linked list on the hot path.
|
||||
- On insert with `Count >= MaxEntries`: evict the oldest **⅛ (128) entries** by one O(n) scan
|
||||
under a small eviction lock (double-checked so concurrent inserts don't stampede). Overflow
|
||||
now costs one 1024-element scan instead of 1023 future recompiles.
|
||||
- `MaxEntries` stays 1024; `Hits`/`Count`/`Clear()` keep their signatures (tests use them,
|
||||
`SiteScriptCompileCacheTests.cs`).
|
||||
|
||||
### Cache-miss compiles move off the actor thread
|
||||
|
||||
Two on-actor-thread compile sites (`Task.Run`+`PipeTo` shared-script loader at
|
||||
`DeploymentManagerActor.cs:1540-1563` is the pattern reference):
|
||||
|
||||
1. **Deploy gate** (`HandleDeploy`, `DeploymentManagerActor.cs:522-556`). The S3 comment's
|
||||
ordering constraint (mailbox-FIFO between deploy/delete/disable) is real — so the design is
|
||||
**warm-then-gate**: `HandleDeploy` first runs `_deployCompileValidator.Validate` in a
|
||||
`Task.Run`, piping back `DeployCompileWarmed(command, replyTo)`; on receipt it re-runs the
|
||||
existing synchronous gate (now all cache hits — memoised at `ScriptCompilationService.cs:148`)
|
||||
and proceeds down the unchanged `ProceedWithDeploy` path. Ordering is preserved with a
|
||||
**per-instance in-flight guard**: while a warm is in flight for instance X, other mutating
|
||||
commands for X are buffered last-write-wins with a `Failed`-superseded reply to the displaced
|
||||
sender — the exact machinery the redeploy path already uses
|
||||
(`_pendingRedeploys`/`_terminatingActorsByName`, `DeploymentManagerActor.cs:569-601`).
|
||||
Commands for other instances flow freely (an improvement over today's whole-mailbox stall
|
||||
during a long compile, and safe: cross-instance ordering was never guaranteed to callers).
|
||||
2. **Staggered startup / InstanceActor PreStart** (`InstanceActor.cs:1631,1682,1795`). Each
|
||||
startup batch (`HandleStartNextBatch`, `DeploymentManagerActor.cs:467-500`) gains a pre-warm
|
||||
step: before creating the batch's Instance Actors, compile the batch's distinct script bodies
|
||||
and trigger expressions in a `Task.Run`, then pipe `BatchCompileWarmed` and create the actors
|
||||
— PreStart compiles become cache hits. This closes the long-deferred "per-instance
|
||||
compilation during staggered startup" item the WIP `Component-SiteRuntime.md` carries
|
||||
(failover time-to-recover), at the cost of one extra message per batch.
|
||||
InstanceActor's own compile calls stay where they are (they are the correctness backstop and
|
||||
are hits in every warmed path).
|
||||
|
||||
No change to `ScriptCompilationService` semantics: results (success AND failure) stay memoised,
|
||||
error text stays name-free, the process-static `ScriptOptions` + `CachingScriptMetadataResolver`
|
||||
invariants (`ScriptCompilationService.cs:104-111`) are untouched.
|
||||
|
||||
---
|
||||
|
||||
## 6. (f) Test plan (stage b)
|
||||
|
||||
All in `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests` unless noted; the injectable scheduler
|
||||
seam (#18) and injectable eval gate make every scenario deterministic without the process-wide
|
||||
singletons.
|
||||
|
||||
1. **Starvation regression (the finding-#4 pin):** injected 8-thread scheduler; 8 scripts
|
||||
blocked on a `ManualResetEventSlim`; fire an Expression-trigger alarm eval ⇒
|
||||
`AlarmStateChanged` (or `ExpressionEvalResult`) observed **< 2 s** while all 8 threads stay
|
||||
blocked. Companion negative: same setup on the *old* wiring is the currently-failing shape
|
||||
(documented in the test comment, not committed failing).
|
||||
2. **Queue-inclusive eval timeout:** eval gate of 1, occupy it > `TriggerEvalTimeoutSeconds`
|
||||
with a slow eval; second eval resolves as false within ~the timeout **measured from
|
||||
enqueue** (assert elapsed-from-StartExpressionEvaluation, not from gate acquisition), and
|
||||
the trigger is not parked (`_evalPending` drains).
|
||||
3. **Queue-inclusive script deadline:** 1-thread scheduler, first script blocks past the second
|
||||
script's full timeout; second script's failure path fires without its body ever executing
|
||||
(side-effect flag never set), error reply carries the timeout message, watchdog does NOT
|
||||
flag it as a stuck thread.
|
||||
4. **Watchdog thread replacement:** 1-thread scheduler, wedge a script past timeout+grace ⇒
|
||||
`TryDetachWorker` fires, a subsequent script executes (replacement thread live),
|
||||
`DetachedScriptThreads` gauge = 1; unwedge ⇒ detached worker exits, gauge returns to 0.
|
||||
Cap test: detached-at-cap ⇒ no new thread, Error site event emitted.
|
||||
5. **Pool sizing:** `ComputeTargetThreads` pure-function table (floor 8, /8 growth, ceiling,
|
||||
override precedence); `EnsureCapacity` grows once, is idempotent, never shrinks; gauges
|
||||
reflect the widened pool.
|
||||
6. **Shed policy:** cap 4 via options; block 4 runs; 5th trigger ⇒ shed counter +1, Warning
|
||||
site event (and second shed within the minute ⇒ counter +1, NO second event), Ask caller
|
||||
gets error reply; one completion ⇒ next trigger launches. Alarm on-trigger variant.
|
||||
7. **Supervision-parity set:** the seven pins of §4, replacing/reworking
|
||||
`Actors/ExecutionActorTests.cs`.
|
||||
8. **Compile cache LRU:** fill to `MaxEntries`, touch entry #1, insert one more ⇒ entry #1
|
||||
survives, an untouched old entry is evicted, `Count ≤ MaxEntries`, no wholesale clear
|
||||
(`Hits` preserved); concurrent GetOrAdd storm keeps the bound.
|
||||
9. **Warm-then-gate ordering:** Deploy(X) immediately followed by Delete(X) ⇒ delete is
|
||||
buffered, deploy applies first, terminal states correct; Deploy(X) then Deploy(X') ⇒ first
|
||||
sender gets superseded reply; Deploy(X) + Deploy(Y) interleave freely. Batch pre-warm:
|
||||
staggered startup with a compile-heavy config ⇒ InstanceActor PreStart compiles are cache
|
||||
hits (assert via `SiteScriptCompileCache.Hits`).
|
||||
|
||||
Suite gate: full `dotnet test ZB.MOM.WW.ScadaBridge.slnx` green; rig rebuild + failover drill
|
||||
per the Phase-3 gate (the drill guards the timeout/actor changes, plan §6).
|
||||
|
||||
---
|
||||
|
||||
## 7. Configuration summary (all under `ScadaBridge:SiteRuntime`, validated in `SiteRuntimeOptionsValidator`)
|
||||
|
||||
| Option | Default | New? | Meaning |
|
||||
|---|---|---|---|
|
||||
| `ScriptExecutionThreadCount` | 8 | existing | Blocking-pool floor (was: fixed size) |
|
||||
| `ScriptExecutionMaxThreadCount` | 32 | new | Blocking-pool ceiling for instance-scaled growth |
|
||||
| `TriggerEvalMaxConcurrency` | `max(2, ProcessorCount)` | new | Thread-pool gate for Expression evals |
|
||||
| `TriggerEvalTimeoutSeconds` | 2 | new | Was hardcoded; now queue-inclusive |
|
||||
| `MaxConcurrentRunsPerScript` | 4 | new | Per-script/per-alarm in-flight cap; overflow sheds newest |
|
||||
| `ScriptExecutionTimeoutSeconds` / per-script override | 30 / null | existing | Unchanged semantics; deadline now anchored at enqueue |
|
||||
| `StuckScriptGraceMs` | 30000 | existing | Unchanged; now also triggers thread detach/replace |
|
||||
|
||||
---
|
||||
|
||||
## 8. Notes for WP3.1 stage (b) — scope & risk deltas vs. the plan
|
||||
|
||||
- **Risk DOWN on actor elimination:** the shells have no Receive handlers, no PostStop, and
|
||||
their `IActorRef` is provably unused as a message target (`ScriptRuntimeContext.cs:43,243,305`
|
||||
are the only `_self` references). Elimination is a refactor of dead structure, not a
|
||||
semantics change — the parity surface is the launcher body, which moves verbatim.
|
||||
- **Scope +:** `AlarmExecutionActor` is eliminated together with `ScriptExecutionActor`
|
||||
(same pattern, shared launcher); the plan text named only the latter.
|
||||
- **Scope +:** batch pre-warm of startup compiles closes the deferred "per-instance compilation
|
||||
during staggered startup" item; the uncommitted `Component-SiteRuntime.md` WIP (compile-cache
|
||||
overflow-cliff paragraph) should be finalized in stage (b) to record the cliff as resolved by
|
||||
LRU eviction. Do not touch that file in stage (a) — it is user WIP.
|
||||
- **Scope note:** one intentional behavior improvement is folded in (launch-path failure now
|
||||
replies to the Ask caller instead of letting it hang, §4 map row 2) — pinned by test 2 of the
|
||||
parity set so it is explicit, not accidental.
|
||||
- **Ordering with WP3.2:** the shed site event is deliberately rate-limited (1/script/minute)
|
||||
so this WP does not add a new `site_events` volume source while WP3.2 is deciding volume
|
||||
policy; merge order (3.1 before 3.2) per the plan's conflict matrix is unaffected.
|
||||
- **Not in scope:** cancelling in-flight runs on actor stop (explicitly kept at parity, §4 row
|
||||
11); eval-side dedicated pool (rejected, §1); pool shrink (rejected, §2).
|
||||
Reference in New Issue
Block a user