fix(scripting): give scripted-alarm predicates the configured run budget, not 250 ms (lmxopcua#779)

DriverHostActor built ScriptedAlarmEngine without a scriptTimeout, so every
production alarm predicate fell back to TimedScriptEvaluator.DefaultTimeout
(250 ms) while virtual tags and Calculation tags ran on 2 s. History shows no
decision behind it: 250 ms is the generic Phase-7 A.4 default, the VT
evaluator moved to 2 s in F8b (219d10a2) and the alarm engine was simply never
given a value.

- New node-wide Scripting:RunTimeout (default 2 s, clamped to (0, 60 s],
  non-positive -> default, each clamp a startup Warning) on ScriptingOptions.
- The VT evaluator and the alarm engine default to it; DriverHostActor now
  builds the engine through CreateScriptedAlarmEngine, which passes it
  explicitly. Calculation keeps its per-driver runTimeoutMs.
- Docs: ScriptEditor (config table), ScriptedAlarms, VirtualTags.

Tests: ScriptedAlarmEngineBudgetTests (a 600 ms predicate now completes and
activates; default and configured budgets), ScriptedAlarmEngineBudgetWiringTests
(the host-built engine), ScriptingOptionsBindingTests (bind + clamps). The
engine and wiring tests fail with the old 250 ms default restored.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 17:12:39 -05:00
parent 2331f7ed14
commit d683b83156
11 changed files with 192 additions and 13 deletions
@@ -238,7 +238,10 @@ public sealed class ScriptedAlarmEngine : IDisposable
/// <param name="loggerFactory">The factory for creating alarm loggers.</param>
/// <param name="engineLogger">The logger for engine-level diagnostics.</param>
/// <param name="clock">Optional function providing the current UTC time; defaults to DateTime.UtcNow.</param>
/// <param name="scriptTimeout">Optional timeout for script execution; defaults to the evaluator's default timeout.</param>
/// <param name="scriptTimeout">Optional per-predicate wall-clock budget; null ⇒ <see cref="ScriptingOptions.RunTimeout"/>
/// of <paramref name="scripting"/> (2 s by default — the same budget virtual tags get). lmxopcua#779: the null
/// default used to be <c>TimedScriptEvaluator.DefaultTimeout</c> (250 ms), which is what every production engine
/// silently ran with because the host never passed a value.</param>
/// <param name="scripting">Run-guard budget + quarantine threshold; null ⇒ <see cref="ScriptingOptions"/> defaults.</param>
public ScriptedAlarmEngine(
ITagUpstreamSource upstream,
@@ -254,7 +257,7 @@ public sealed class ScriptedAlarmEngine : IDisposable
_loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory));
_engineLogger = engineLogger ?? throw new ArgumentNullException(nameof(engineLogger));
_clock = clock ?? (() => DateTime.UtcNow);
_scriptTimeout = scriptTimeout ?? TimedScriptEvaluator<AlarmPredicateContext, bool>.DefaultTimeout;
_scriptTimeout = scriptTimeout ?? (scripting ?? new ScriptingOptions()).Sanitized(out _).RunTimeout;
_guard = new ScriptRunGuard(scripting, _engineLogger);
_shutdownToken = _shutdownCts.Token;
}
@@ -262,6 +265,9 @@ public sealed class ScriptedAlarmEngine : IDisposable
/// <summary>The run guard every predicate evaluation goes through (diagnostics / tests).</summary>
public ScriptRunGuard RunGuard => _guard;
/// <summary>The wall-clock budget each predicate run is given (diagnostics / tests).</summary>
public TimeSpan ScriptTimeout => _scriptTimeout;
/// <summary>
/// OPC UA <c>BadInternalError</c> — the quality a condition is annotated with while its predicate
/// cannot be evaluated (timed out, refused because the previous run is still executing, or
@@ -19,6 +19,12 @@ public sealed class ScriptingOptions
/// <summary>The appsettings section this binds from.</summary>
public const string SectionName = "Scripting";
/// <summary>Default <see cref="RunTimeout"/>: 2 s — the budget virtual tags and Calculation tags have always run with.</summary>
public static readonly TimeSpan DefaultRunTimeout = TimeSpan.FromSeconds(2);
/// <summary>Upper bound <see cref="Sanitized"/> applies to <see cref="RunTimeout"/>: 60 s.</summary>
public static readonly TimeSpan MaxRunTimeout = TimeSpan.FromSeconds(60);
/// <summary>Default <see cref="MaxAllocBytesPerRun"/>: 64 MB.</summary>
public const long DefaultMaxAllocBytesPerRun = 64L * 1024 * 1024;
@@ -28,6 +34,18 @@ public sealed class ScriptingOptions
/// <summary>Default <see cref="StuckOrphanQuarantineFactor"/>: 10.</summary>
public const double DefaultStuckOrphanQuarantineFactor = 10;
/// <summary>
/// Wall-clock budget for one script run on the node-wide seams: virtual tags and scripted-alarm
/// predicates. Default 2 s. A Calculation driver keeps its own per-driver <c>runTimeoutMs</c> (default
/// 2 s as well). Before lmxopcua#779 the scripted-alarm engine was built without a budget and silently
/// took the 250 ms Phase-7 default of <see cref="TimedScriptEvaluator{TContext, TResult}"/>, so an alarm
/// predicate got an eighth of the budget a virtual tag reading the same inputs got — not a decision
/// anywhere in the history, just a missing argument. Also scales the stuck-orphan rule
/// (<see cref="StuckOrphanQuarantineFactor"/> × this). Zero or negative becomes the default; above
/// <see cref="MaxRunTimeout"/> becomes that maximum.
/// </summary>
public TimeSpan RunTimeout { get; set; } = DefaultRunTimeout;
/// <summary>
/// Managed bytes one script run may allocate on its own thread (sampled with
/// <see cref="GC.GetAllocatedBytesForCurrentThread"/> around the run) before the script is
@@ -78,6 +96,8 @@ public sealed class ScriptingOptions
/// <item><see cref="QuarantineAfterConsecutiveTimeouts"/>: into [1, <see cref="MaxQuarantineAfterConsecutiveTimeouts"/>].</item>
/// <item><see cref="MaxAllocBytesPerRun"/>: a negative value becomes 0 (disabled), and a positive value
/// below <see cref="MinMaxAllocBytesPerRun"/> becomes that minimum.</item>
/// <item><see cref="RunTimeout"/>: zero or negative becomes <see cref="DefaultRunTimeout"/>, above
/// <see cref="MaxRunTimeout"/> becomes that maximum.</item>
/// </list>
/// </summary>
/// <param name="adjustments">One line per value that was out of range, naming its key. A negative value that
@@ -126,9 +146,22 @@ public sealed class ScriptingOptions
alloc = MinMaxAllocBytesPerRun;
}
var runTimeout = RunTimeout;
if (runTimeout <= TimeSpan.Zero)
{
notes.Add($"{SectionName}:{nameof(RunTimeout)} {runTimeout} is not positive; using the default {DefaultRunTimeout}.");
runTimeout = DefaultRunTimeout;
}
else if (runTimeout > MaxRunTimeout)
{
notes.Add($"{SectionName}:{nameof(RunTimeout)} {runTimeout} is above the maximum; clamped to {MaxRunTimeout}.");
runTimeout = MaxRunTimeout;
}
adjustments = notes;
return new ScriptingOptions
{
RunTimeout = runTimeout,
MaxAllocBytesPerRun = alloc,
QuarantineAfterConsecutiveTimeouts = threshold,
StuckOrphanQuarantineFactor = factor,
@@ -57,7 +57,8 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac
/// <summary>Initializes a new RoslynVirtualTagEvaluator with the given loggers and optional timeout.</summary>
/// <param name="logger">Logger for recording compilation and execution errors (host diagnostics).</param>
/// <param name="scriptRoot">Root script logger; user <c>ctx.Logger.*</c> output flows through this to the Script-log page.</param>
/// <param name="runTimeout">Maximum execution time for each script; defaults to 2 seconds if not specified.</param>
/// <param name="runTimeout">Maximum execution time for each script; null ⇒ <see cref="ScriptingOptions.RunTimeout"/>
/// of <paramref name="scripting"/> (2 s by default).</param>
/// <param name="scripting">Run-guard budget + quarantine threshold (the <c>Scripting</c> section); null ⇒ defaults.</param>
public RoslynVirtualTagEvaluator(
ILogger<RoslynVirtualTagEvaluator> logger,
@@ -67,7 +68,7 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac
{
_logger = logger;
_scriptRoot = (scriptRoot ?? throw new ArgumentNullException(nameof(scriptRoot))).Logger;
_runTimeout = runTimeout ?? TimeSpan.FromSeconds(2);
_runTimeout = runTimeout ?? (scripting ?? new ScriptingOptions()).Sanitized(out _).RunTimeout;
_guard = new ScriptRunGuard(scripting, _scriptRoot);
}
+3 -2
View File
@@ -367,8 +367,9 @@ if (hasDriver)
// F8b — production virtual-tag evaluator (Roslyn-compiled scripts cached per expression).
// Replaces the F8-default NullVirtualTagEvaluator so VirtualTagActor evaluates real user
// scripts at runtime.
// Node-wide script run-guard knobs (Scripting:MaxAllocBytesPerRun / QuarantineAfterConsecutiveTimeouts /
// StuckOrphanQuarantineFactor). Bound once and clamped into sane ranges here, so an absurd value is a startup
// Node-wide script run-guard knobs (Scripting:RunTimeout / MaxAllocBytesPerRun / QuarantineAfterConsecutiveTimeouts /
// StuckOrphanQuarantineFactor). RunTimeout is the per-run budget of virtual tags and scripted-alarm predicates
// (lmxopcua#779 — the alarm engine used to fall back to a 250 ms default instead). Bound once and clamped into sane ranges here, so an absurd value is a startup
// Warning rather than an OverflowException inside a script run. The one instance is shared by every script
// seam on the node: the virtual-tag evaluator below, the scripted-alarm engine (DriverHostActor, via
// WithOtOpcUaRuntimeActors) and every Calculation driver (DriverFactoryBootstrap).
@@ -1248,14 +1248,34 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
() =>
{
var upstream = new DependencyMuxTagUpstreamSource();
var engine = new ScriptedAlarmEngine(
upstream, store, new ScriptLoggerFactory(scriptLogger), scriptLogger, scripting: scripting);
return (upstream, engine);
return (upstream, CreateScriptedAlarmEngine(upstream, store, scriptLogger, scripting));
},
_localNode, telemetryHub: _telemetryHub),
"scripted-alarm-host");
}
/// <summary>
/// Builds the scripted-alarm engine the alarm host owns, with the node's <c>Scripting:RunTimeout</c> as the
/// per-predicate budget. lmxopcua#779: the engine used to be built without a budget and fell back to the
/// 250 ms <c>TimedScriptEvaluator.DefaultTimeout</c>, an eighth of what a virtual tag reading the same
/// inputs gets. Passed explicitly (not left to the engine's own null default) so the host is the one place
/// that decides it.
/// </summary>
/// <param name="upstream">The engine's upstream tag source.</param>
/// <param name="store">The condition-state store.</param>
/// <param name="scriptLogger">The root script logger.</param>
/// <param name="scripting">The node-wide <c>Scripting</c> options; null ⇒ defaults.</param>
/// <returns>A new, unloaded engine.</returns>
internal static ScriptedAlarmEngine CreateScriptedAlarmEngine(
ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.ITagUpstreamSource upstream, IAlarmStateStore store,
Serilog.ILogger scriptLogger, ScriptingOptions? scripting)
{
var budget = (scripting ?? new ScriptingOptions()).Sanitized(out _).RunTimeout;
return new ScriptedAlarmEngine(
upstream, store, new ScriptLoggerFactory(scriptLogger), scriptLogger,
scriptTimeout: budget, scripting: scripting);
}
private void Bootstrap()
{
// Per-cluster mesh Phase 3: a FetchAndCache node NEVER reads central SQL for config, at boot