fix(scripting): out-of-process soft deadline measures the script's own run, not the pipe or a starved worker (#827)
The soft deadline T was armed on the host at submit, so the pipe round trip
and the time a starved worker waits to be scheduled counted against the
script: under load a trivial script reported 'script timed out', in Shadow
mode that was a script mismatch, and at T + G the host killed the worker and
struck the key.
- Protocol (additive, negotiated): Hello/Ready.worker_soft_clock and
EvalResults.soft_expired_eval_ids. protocol_version stays 1; a worker that
does not echo the flag keeps the pre-#827 host clock.
- Worker: the watchdog runs each run's soft clock from the moment an
evaluation thread starts it and reports the crossing (woken early for
budgets shorter than its 50 ms poll). It still never cancels by itself;
its fail-fast is anchored at the soft notice + G + margin.
- Host (PendingEval): the soft notice answers the caller and arms the kill G
later. The host's own timer becomes a transport deadline, T + G from submit
(restarted at a deferred run's start); passing it with no result and no
notice yields TransportTimedOut -> Unavailable 'script worker unresponsive'
(BadResourceUnavailable, recovery = the run's completion), no strike, no
kill, no Cancel. A re-trigger refused by that still-unanswered run is
Unavailable too, not 'previous run still running'. Counted as
otopcua.script.ipc.timeouts{kind}.
- Frame writer: coalescing now carries soft_expired_eval_ids (it silently
dropped them); the host processes soft notices before results in a frame.
- Shadow: unresponsive is shadow_unavailable (Unavailable), never a mismatch.
Tests: WorkerSoftClockTests suspends the real worker (SIGSTOP via libc kill /
NtSuspendProcess) - a stall past T still returns 7; a stall past T + G is a
runner fault with nothing struck; a late-starting runaway is still timed out
at T from its start and killed G after; Shadow stays match /
shadow_unavailable; both clocks enforce T and G; PendingEval state machine.
This commit is contained in:
@@ -81,6 +81,10 @@ internal sealed record BatchSubmission(string Key, EvalRequest Request, TimeSpan
|
||||
/// Windows only: the worker identity's SID (<c>Scripting:Worker:Identity</c>). Set, the worker starts under a restricted
|
||||
/// token (<see cref="RestrictedWorkerLauncher"/>) and the pipe ACL admits it beside the host; null keeps the host's token.
|
||||
/// </param>
|
||||
/// <param name="WorkerSoftClock">
|
||||
/// Ask the worker to run the soft clock from each run's start (lmxopcua#827; see <see cref="WorkerConnection.WorkerSoftClock"/>).
|
||||
/// False (tests only) keeps the pre-#827 host clock, which is also what a worker that does not echo it gets.
|
||||
/// </param>
|
||||
internal sealed record WorkerLaunchSpec(
|
||||
string Shard,
|
||||
string WorkerPath,
|
||||
@@ -94,7 +98,8 @@ internal sealed record WorkerLaunchSpec(
|
||||
string? MarkerDirectory = null,
|
||||
TimeSpan? LoadTimeout = null,
|
||||
int CompileThreads = 0,
|
||||
string? WorkerSid = null)
|
||||
string? WorkerSid = null,
|
||||
bool WorkerSoftClock = true)
|
||||
{
|
||||
/// <summary>The default compile timeout.</summary>
|
||||
public static readonly TimeSpan DefaultLoadTimeout = TimeSpan.FromSeconds(60);
|
||||
@@ -151,8 +156,9 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
|
||||
private WorkerConnection(string shard, Process process, NamedPipeServerStream pipe, IDisposable? job, IDisposable? stdio,
|
||||
string workDir, ILogger log, Action<LogBatch> onLogs, TimeSpan heartbeatInterval, TimeSpan loadTimeout,
|
||||
Action<WorkerExit> onExiting, string? markerPath)
|
||||
Action<WorkerExit> onExiting, string? markerPath, bool workerSoftClock)
|
||||
{
|
||||
WorkerSoftClock = workerSoftClock;
|
||||
_stdio = stdio;
|
||||
_loadTimeout = loadTimeout;
|
||||
_onExiting = onExiting;
|
||||
@@ -178,6 +184,15 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
/// <summary>The worker's process id.</summary>
|
||||
public int Pid { get; }
|
||||
|
||||
/// <summary>
|
||||
/// lmxopcua#827: the worker runs each evaluation's soft clock from the moment an evaluation thread starts it and
|
||||
/// reports the crossing, so the soft deadline measures the script's own execution — not the pipe, not the time
|
||||
/// a starved worker takes to be scheduled. The host keeps a transport deadline of its own instead (see
|
||||
/// <see cref="PendingEval"/>). Negotiated: asked for in <c>Hello</c>, true only when the worker echoed it in
|
||||
/// <c>Ready</c>; otherwise the host's clock, started at submit, is the soft deadline (the pre-#827 contract).
|
||||
/// </summary>
|
||||
public bool WorkerSoftClock { get; }
|
||||
|
||||
/// <summary>Completes when the worker has ended, with why.</summary>
|
||||
public Task<WorkerExit> Exited => _exited.Task;
|
||||
|
||||
@@ -300,19 +315,19 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
HeartbeatIntervalMs = (uint)spec.HeartbeatInterval.TotalMilliseconds,
|
||||
MaxLogLines = 1000,
|
||||
CompileThreads = (uint)Math.Max(0, spec.CompileThreads),
|
||||
WorkerSoftClock = spec.WorkerSoftClock,
|
||||
};
|
||||
if (spec.PeCacheDirectory is { } cacheDir && spec.PeCacheKey is { } cacheKey)
|
||||
{
|
||||
hello.PeCacheDir = cacheDir;
|
||||
hello.PeCacheKey = Google.Protobuf.ByteString.CopyFrom(cacheKey);
|
||||
}
|
||||
if (ScriptWorkerFrameCodec.UseBlockingIo)
|
||||
await HandshakeBlockingAsync(pipe, hello, nonce, process.Id, handshakeCts.Token).ConfigureAwait(false);
|
||||
else
|
||||
await HandshakeAsync(pipe, hello, nonce, process.Id, handshakeCts.Token).ConfigureAwait(false);
|
||||
var ready = ScriptWorkerFrameCodec.UseBlockingIo
|
||||
? await HandshakeBlockingAsync(pipe, hello, nonce, process.Id, handshakeCts.Token).ConfigureAwait(false)
|
||||
: await HandshakeAsync(pipe, hello, nonce, process.Id, handshakeCts.Token).ConfigureAwait(false);
|
||||
|
||||
var connection = new WorkerConnection(spec.Shard, process, pipe, job, stdio, workDir, log, onLogs, spec.HeartbeatInterval,
|
||||
spec.EffectiveLoadTimeout, onExiting, markerPath);
|
||||
spec.EffectiveLoadTimeout, onExiting, markerPath, workerSoftClock: spec.WorkerSoftClock && ready.WorkerSoftClock);
|
||||
connection.Begin();
|
||||
return connection;
|
||||
}
|
||||
@@ -557,7 +572,7 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
request.EvalId = (ulong)Interlocked.Increment(ref _nextEvalId);
|
||||
Interlocked.Exchange(ref _lastUsedTimestamp, Stopwatch.GetTimestamp());
|
||||
var pending = new PendingEval(key, request.EvalId, onFinished, _dispatcher);
|
||||
if (softTimeout is { } soft) pending.Arm(soft, hardGrace, OnHardDeadline);
|
||||
if (softTimeout is { } soft) pending.Arm(soft, hardGrace, OnHardDeadline, WorkerSoftClock);
|
||||
_pending[request.EvalId] = pending;
|
||||
var batch = new EvalBatch();
|
||||
batch.Items.Add(request);
|
||||
@@ -586,7 +601,7 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
var item = items[i];
|
||||
item.Request.EvalId = (ulong)Interlocked.Increment(ref _nextEvalId);
|
||||
var pending = new PendingEval(item.Key, item.Request.EvalId, item.OnFinished, _dispatcher);
|
||||
pending.Arm(item.SoftTimeout, item.HardGrace, OnHardDeadline);
|
||||
pending.Arm(item.SoftTimeout, item.HardGrace, OnHardDeadline, WorkerSoftClock);
|
||||
_pending[item.Request.EvalId] = pending;
|
||||
batch.Items.Add(item.Request);
|
||||
pendings[i] = pending;
|
||||
@@ -764,6 +779,16 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
switch (frame.BodyCase)
|
||||
{
|
||||
case Envelope.BodyOneofCase.Results:
|
||||
// Worker soft clock (lmxopcua#827): the script itself has run for T. Only a worker that agreed to run
|
||||
// the clock may send these. Before the results: the writer coalesces frames, and a notice the worker
|
||||
// sent ahead of the same run's result must still answer the caller with its timeout, as in process.
|
||||
if (frame.Results.SoftExpiredEvalIds.Count > 0 && !WorkerSoftClock)
|
||||
throw new ScriptWorkerProtocolException("soft-deadline notice from a worker that does not run the soft clock");
|
||||
foreach (var id in frame.Results.SoftExpiredEvalIds)
|
||||
{
|
||||
if (_pending.TryGetValue(id, out var expired)) expired.SoftExpired();
|
||||
else CheckIssued(id, "soft-deadline notice");
|
||||
}
|
||||
foreach (var result in frame.Results.Items)
|
||||
{
|
||||
// An eval id with no outstanding request is a protocol violation (design §6 "what the host
|
||||
@@ -1026,18 +1051,45 @@ internal sealed class WorkerConnection : IAsyncDisposable
|
||||
/// <summary>
|
||||
/// One evaluation outstanding on a worker. Its deadlines share ONE timer (lmxopcua#789: the per-eval
|
||||
/// <c>WaitAsync(timeout)</c> timer, the hard-deadline <c>CancellationTokenSource</c> and their continuations were a
|
||||
/// measurable share of the host's CPU): it fires at the soft deadline T and is re-armed for the grace G.
|
||||
/// measurable share of the host's CPU).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The deadlines are absolute timestamps the timer callback re-checks, so a deferral (the worker queued the run behind
|
||||
/// busy evaluation threads, lmxopcua#780 batch submit) can stop the clock and a start notice can restart it — the full
|
||||
/// T, then G, from the start — without a stale tick firing early.
|
||||
/// <para>
|
||||
/// <b>Worker soft clock</b> (lmxopcua#827, <see cref="WorkerConnection.WorkerSoftClock"/>): the soft deadline T is
|
||||
/// the worker's — it reports, through <see cref="SoftExpired"/>, that the script has itself run for T, and the
|
||||
/// hard deadline G is armed from that notice. The host's timer is then only a TRANSPORT deadline: T + G from
|
||||
/// submit (restarted at a deferred run's start). Passing it with neither a result nor a soft notice means the
|
||||
/// pipe or the worker process is late, not the script — <see cref="Outcome"/> yields
|
||||
/// <see cref="TransportTimedOut"/> (a runner fault: nothing is struck and the worker is not killed; a worker that
|
||||
/// is really hung goes silent and the heartbeat monitor kills it). So a starved worker can no longer make a
|
||||
/// trivial script report "timed out", which in Shadow mode read as a script mismatch.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Host clock</b> (a worker that did not agree to the above): the timer fires at the soft deadline T, counted
|
||||
/// from submit, and is re-armed for the grace G.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The deadlines are absolute timestamps the timer callback re-checks, so a deferral (the worker queued the run
|
||||
/// behind busy evaluation threads, lmxopcua#780 batch submit) can stop the clock and a start notice can restart
|
||||
/// it — the full T (or transport deadline), then G, from the start — without a stale tick firing early.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal sealed class PendingEval
|
||||
{
|
||||
/// <summary>What <see cref="Outcome"/> yields when the soft deadline passed first. Never on the wire.</summary>
|
||||
internal static readonly EvalResult SoftTimedOut = new();
|
||||
|
||||
/// <summary>
|
||||
/// What <see cref="Outcome"/> yields when the transport deadline passed with no word from the worker (worker soft
|
||||
/// clock only, lmxopcua#827): a runner fault, not the script's. Never on the wire.
|
||||
/// </summary>
|
||||
internal static readonly EvalResult TransportTimedOut = new();
|
||||
|
||||
private const int Waiting = 0;
|
||||
private const int SoftReported = 1;
|
||||
private const int HardFired = 2;
|
||||
private const int TransportReported = 3;
|
||||
|
||||
private readonly TaskCompletionSource<EvalResult?> _completion = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
private readonly TaskCompletionSource<EvalResult?> _outcome;
|
||||
private readonly ResultDispatcher? _dispatcher;
|
||||
@@ -1047,8 +1099,10 @@ internal sealed class PendingEval
|
||||
private Action<PendingEval>? _onHardDeadline;
|
||||
private TimeSpan _soft;
|
||||
private TimeSpan _grace;
|
||||
private bool _workerClock;
|
||||
private long _softDue;
|
||||
private long _hardDue;
|
||||
private long _transportDue;
|
||||
private bool _deferred;
|
||||
private bool _started;
|
||||
private int _phase;
|
||||
@@ -1086,24 +1140,31 @@ internal sealed class PendingEval
|
||||
public Task<EvalResult?> Completion => _completion.Task;
|
||||
|
||||
/// <summary>
|
||||
/// What the caller waits on: the worker's result, null when the worker ended first, or
|
||||
/// <see cref="SoftTimedOut"/> at the soft deadline (the run itself is still outstanding then).
|
||||
/// What the caller waits on: the worker's result, null when the worker ended first, <see cref="SoftTimedOut"/> at
|
||||
/// the soft deadline, or <see cref="TransportTimedOut"/> at the transport deadline (the run itself is still
|
||||
/// outstanding in both of the last two cases).
|
||||
/// </summary>
|
||||
public Task<EvalResult?> Outcome => _outcome.Task;
|
||||
|
||||
/// <summary>Starts the deadline timer. Called once, before the request is sent.</summary>
|
||||
internal void Arm(TimeSpan soft, TimeSpan grace, Action<PendingEval> onHardDeadline)
|
||||
/// <param name="soft">T.</param>
|
||||
/// <param name="grace">G.</param>
|
||||
/// <param name="onHardDeadline">The kill.</param>
|
||||
/// <param name="workerClock">The worker runs the soft clock (lmxopcua#827): this timer is the transport deadline.</param>
|
||||
internal void Arm(TimeSpan soft, TimeSpan grace, Action<PendingEval> onHardDeadline, bool workerClock = false)
|
||||
{
|
||||
_soft = soft;
|
||||
_grace = grace;
|
||||
_onHardDeadline = onHardDeadline;
|
||||
_workerClock = workerClock;
|
||||
HardDeadline = soft + grace;
|
||||
var now = Stopwatch.GetTimestamp();
|
||||
_softDue = now + Ticks(soft);
|
||||
_hardDue = _softDue + Ticks(grace);
|
||||
_transportDue = _hardDue;
|
||||
// Created disarmed and assigned before it can fire, so the soft tick can always re-arm it for the grace.
|
||||
_timer = new Timer(static s => ((PendingEval)s!).OnTimer(), this, Timeout.Infinite, Timeout.Infinite);
|
||||
_timer.Change(soft, Timeout.InfiniteTimeSpan);
|
||||
_timer.Change(workerClock ? soft + grace : soft, Timeout.InfiniteTimeSpan);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1122,7 +1183,8 @@ internal sealed class PendingEval
|
||||
|
||||
/// <summary>
|
||||
/// The worker started this run: a deferred run's deadlines restart from now (T, or T + G when its soft deadline
|
||||
/// had already been reported). A run that was never deferred keeps the clock it has had since submit.
|
||||
/// had already been reported; with the worker soft clock, the transport deadline T + G). A run that was never
|
||||
/// deferred keeps the clock it has had since submit.
|
||||
/// </summary>
|
||||
internal void Start()
|
||||
{
|
||||
@@ -1132,7 +1194,16 @@ internal sealed class PendingEval
|
||||
if (_timer is null || !_deferred || Volatile.Read(ref _done) != 0) return;
|
||||
_deferred = false;
|
||||
var now = Stopwatch.GetTimestamp();
|
||||
if (Volatile.Read(ref _phase) == 0)
|
||||
if (_workerClock)
|
||||
{
|
||||
// Its soft deadline is the worker's from here; the transport deadline restarts. Already answered (a
|
||||
// deferral notice that arrived after the transport deadline): nothing to re-arm — a soft notice or the
|
||||
// result is what comes next.
|
||||
if (Volatile.Read(ref _phase) != Waiting) return;
|
||||
_transportDue = now + Ticks(_soft + _grace);
|
||||
TryChange(_soft + _grace);
|
||||
}
|
||||
else if (Volatile.Read(ref _phase) == Waiting)
|
||||
{
|
||||
_softDue = now + Ticks(_soft);
|
||||
_hardDue = _softDue + Ticks(_grace);
|
||||
@@ -1148,31 +1219,63 @@ internal sealed class PendingEval
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The worker reports that the script has itself run for T (worker soft clock, lmxopcua#827): the caller gets its
|
||||
/// soft timeout now — unless the transport deadline already answered it — and the hard deadline is G from here.
|
||||
/// </summary>
|
||||
internal void SoftExpired()
|
||||
{
|
||||
bool signal;
|
||||
lock (_clock)
|
||||
{
|
||||
if (_timer is null || !_workerClock || Volatile.Read(ref _done) != 0) return;
|
||||
var phase = Volatile.Read(ref _phase);
|
||||
if (phase is SoftReported or HardFired) return;
|
||||
_started = true;
|
||||
_deferred = false;
|
||||
signal = phase == Waiting;
|
||||
Interlocked.Exchange(ref _phase, SoftReported);
|
||||
_hardDue = Stopwatch.GetTimestamp() + Ticks(_grace);
|
||||
TryChange(_grace);
|
||||
}
|
||||
// Outside the lock: a caller with a dispatcher resumes synchronously inside TrySetResult.
|
||||
if (signal) _outcome.TrySetResult(SoftTimedOut);
|
||||
}
|
||||
|
||||
private void OnTimer()
|
||||
{
|
||||
if (Volatile.Read(ref _done) != 0) return;
|
||||
bool soft = false, hard = false;
|
||||
bool soft = false, hard = false, transport = false;
|
||||
lock (_clock)
|
||||
{
|
||||
if (_deferred || Volatile.Read(ref _done) != 0) return;
|
||||
var now = Stopwatch.GetTimestamp();
|
||||
if (Volatile.Read(ref _phase) == 0)
|
||||
var phase = Volatile.Read(ref _phase);
|
||||
if (phase == Waiting && _workerClock)
|
||||
{
|
||||
// A tick armed before a deferral/start can land early: re-arm for what is left.
|
||||
if (now < _transportDue) { TryChange(Stopwatch.GetElapsedTime(now, _transportDue)); return; }
|
||||
Interlocked.Exchange(ref _phase, TransportReported);
|
||||
transport = true;
|
||||
}
|
||||
else if (phase == Waiting)
|
||||
{
|
||||
// A tick armed before a deferral/start can land early: re-arm for what is left.
|
||||
if (now < _softDue) { TryChange(Stopwatch.GetElapsedTime(now, _softDue)); return; }
|
||||
Interlocked.Exchange(ref _phase, 1);
|
||||
Interlocked.Exchange(ref _phase, SoftReported);
|
||||
soft = true;
|
||||
TryChange(_hardDue > now ? Stopwatch.GetElapsedTime(now, _hardDue) : TimeSpan.Zero);
|
||||
}
|
||||
else if (Volatile.Read(ref _phase) == 1)
|
||||
else if (phase == SoftReported)
|
||||
{
|
||||
if (now < _hardDue) { TryChange(Stopwatch.GetElapsedTime(now, _hardDue)); return; }
|
||||
Interlocked.Exchange(ref _phase, 2);
|
||||
Interlocked.Exchange(ref _phase, HardFired);
|
||||
hard = true;
|
||||
}
|
||||
}
|
||||
// Outside the lock: a caller with a dispatcher resumes synchronously inside TrySetResult.
|
||||
if (soft) _outcome.TrySetResult(SoftTimedOut);
|
||||
if (transport) _outcome.TrySetResult(TransportTimedOut);
|
||||
if (hard && Volatile.Read(ref _done) == 0) _onHardDeadline?.Invoke(this);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user