fix(scripting): only timeouts are quarantine strikes; alarm Bad annotation once per bucket change (fix round 1 I1, I3)
- I1 ScriptRunGuard: a run refused because the previous one is still executing is a Bad publish plus otopcua.script.run_refused, and no longer a quarantine strike. Only a timeout (and an allocation breach) counts; a refusal neither adds to nor resets the timeout streak; a completed run (value or throw) resets it. On a 1 Hz trigger with a 2 s budget a single 4 s overrun is one timeout plus two refusals and must not quarantine. Tests updated across the guard, the alarm engine, the Calculation evaluator and the VT evaluator, plus Refusals_neither_count_toward_nor_reset_the_timeout_streak. - I3 ScriptedAlarmEngine: the quality bucket is tracked only after the predicate completes (or in the inputs-not-ready path), so the tracked bucket is the last one annotated. A refused or quarantined predicate used to re-track the input bucket Good before the attempt and then flip it Bad, emitting QualityChanged on every push. The run-guard tests now count emissions (exactly 1 across N pushes, refused and quarantined cases), with a healthy sibling's dispatch count as the positive control that every push was processed. Both were red before the fix. Refs #734 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -24,8 +24,10 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Scripting;
|
||||
/// <see cref="ScriptRunRefusedException"/> (<see cref="PreviousRunStillRunningReason"/>), counted on
|
||||
/// <see cref="RefusedWhileRunningCount"/>. One orphan per script, never one per trigger.</item>
|
||||
/// <item><b>Quarantines after <see cref="ScriptingOptions.QuarantineAfterConsecutiveTimeouts"/>
|
||||
/// consecutive non-completions</b> (timeouts and still-running refusals both count; a run that
|
||||
/// returns — a value or a throw — resets the count): no further dispatch, every attempt throws
|
||||
/// consecutive timeouts</b> (only a timeout is a strike; a refusal is a Bad publish plus its counter
|
||||
/// and leaves the streak untouched — on a 1 Hz trigger with a 2 s budget one 4 s overrun is a
|
||||
/// timeout plus two refusals and must not quarantine; a run that returns — a value or a throw —
|
||||
/// resets the streak): no further dispatch, every attempt throws
|
||||
/// <see cref="ScriptQuarantinedException"/>, one Error log, one <c>otopcua.script.quarantined</c>
|
||||
/// increment, one <see cref="Quarantined"/> event. Lifted by a source change (the operator fixed the
|
||||
/// script — a new source identity under the same key starts clean and the old orphan no longer blocks
|
||||
@@ -177,10 +179,12 @@ public sealed class ScriptRunGuard
|
||||
// not) still holds the slot is refused here, before anything is dispatched to the pool.
|
||||
if (Interlocked.CompareExchange(ref state.InFlight, 1, 0) != 0)
|
||||
{
|
||||
// A refusal is a Bad publish plus its counter — NOT a strike. Only a timeout (and an allocation
|
||||
// breach) counts toward quarantine: on a 1 Hz trigger with a 2 s budget a single 4 s overrun is
|
||||
// one timeout followed by two refusals, and counting those would quarantine a script that merely
|
||||
// ran long once. The refusal also leaves the timeout streak untouched.
|
||||
Interlocked.Increment(ref _refused);
|
||||
RefusedCounter.Add(1, new KeyValuePair<string, object?>("script", key));
|
||||
if (NoteNonCompletion(key, state, "refused: previous evaluation still running"))
|
||||
throw new ScriptQuarantinedException(key, state.QuarantineReason!);
|
||||
throw new ScriptRunRefusedException(key);
|
||||
}
|
||||
|
||||
@@ -241,7 +245,7 @@ public sealed class ScriptRunGuard
|
||||
// The caller's cancel wins even when it lands a few µs after the timeout fired.
|
||||
if (ct.IsCancellationRequested) throw new OperationCanceledException(ct);
|
||||
runCts.Cancel();
|
||||
if (NoteNonCompletion(key, state, $"timed out after {timeout.TotalMilliseconds:F1} ms"))
|
||||
if (NoteTimeout(key, state, timeout))
|
||||
throw new ScriptQuarantinedException(key, state.QuarantineReason!);
|
||||
throw new ScriptTimeoutException(timeout);
|
||||
}
|
||||
@@ -252,11 +256,11 @@ public sealed class ScriptRunGuard
|
||||
catch (Exception)
|
||||
{
|
||||
// The script returned control (by throwing) — that is a completion for quarantine purposes.
|
||||
Interlocked.Exchange(ref state.ConsecutiveNonCompletions, 0);
|
||||
Interlocked.Exchange(ref state.ConsecutiveTimeouts, 0);
|
||||
throw;
|
||||
}
|
||||
|
||||
Interlocked.Exchange(ref state.ConsecutiveNonCompletions, 0);
|
||||
Interlocked.Exchange(ref state.ConsecutiveTimeouts, 0);
|
||||
if (BreachedBudget(slot.AllocatedBytes))
|
||||
{
|
||||
// The continuation above has already (or will have) quarantined the key; make the reason visible
|
||||
@@ -291,13 +295,14 @@ public sealed class ScriptRunGuard
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Record a non-completion; returns true when it tipped the key into quarantine.</summary>
|
||||
private bool NoteNonCompletion(string key, KeyState state, string what)
|
||||
/// <summary>Record a timeout strike; returns true when it tipped the key into quarantine. A completed run
|
||||
/// (value or throw) resets the streak; a refusal neither counts nor resets.</summary>
|
||||
private bool NoteTimeout(string key, KeyState state, TimeSpan timeout)
|
||||
{
|
||||
var count = Interlocked.Increment(ref state.ConsecutiveNonCompletions);
|
||||
var count = Interlocked.Increment(ref state.ConsecutiveTimeouts);
|
||||
if (count < _quarantineThreshold) return false;
|
||||
Quarantine(key, state,
|
||||
$"{count} consecutive evaluation(s) did not complete (last: {what}); " +
|
||||
$"{count} consecutive evaluation(s) timed out (budget {timeout.TotalMilliseconds:F0} ms); " +
|
||||
"the script is not evaluated again until its source changes or it is redeployed",
|
||||
"timeouts");
|
||||
return true;
|
||||
@@ -333,7 +338,7 @@ public sealed class ScriptRunGuard
|
||||
{
|
||||
public string? SourceIdentity = sourceIdentity;
|
||||
public int InFlight;
|
||||
public int ConsecutiveNonCompletions;
|
||||
public int ConsecutiveTimeouts;
|
||||
public string? QuarantineReason;
|
||||
}
|
||||
|
||||
@@ -368,7 +373,7 @@ public sealed class ScriptRunRefusedException : Exception
|
||||
|
||||
/// <summary>
|
||||
/// Thrown by <see cref="ScriptRunGuard.RunAsync{TContext, TResult}"/> for a quarantined script: repeated
|
||||
/// non-completion or an allocation-budget breach. Nothing is dispatched while quarantined; the seam
|
||||
/// timeouts or an allocation-budget breach. Nothing is dispatched while quarantined; the seam
|
||||
/// publishes Bad carrying the reason, which an operator clears by fixing the script (source change) or
|
||||
/// redeploying.
|
||||
/// </summary>
|
||||
|
||||
@@ -33,10 +33,11 @@ public sealed class ScriptingOptions
|
||||
public long MaxAllocBytesPerRun { get; set; } = DefaultMaxAllocBytesPerRun;
|
||||
|
||||
/// <summary>
|
||||
/// Consecutive non-completions (a run that times out, or a run refused because the previous one is
|
||||
/// still executing) after which the script is quarantined: no further runs are dispatched, every
|
||||
/// Consecutive timeouts after which the script is quarantined: no further runs are dispatched, every
|
||||
/// evaluation reports Bad, and the quarantine lifts only on a source change, a redeploy or an explicit
|
||||
/// reset. Default 3; values below 1 are treated as 1.
|
||||
/// reset. Only a timeout is a strike — a run refused because the previous one is still executing is
|
||||
/// reported Bad and counted, but neither adds to nor resets the streak; a completed run (value or
|
||||
/// throw) resets it. Default 3; values below 1 are treated as 1.
|
||||
/// </summary>
|
||||
public int QuarantineAfterConsecutiveTimeouts { get; set; } = DefaultQuarantineAfterConsecutiveTimeouts;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user