fix(worker-tests): doc-comment placement + watchdog-window headroom in long-in-flight test
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Failing after 17s
ci / java (push) Successful in 2m38s
ci / portable (push) Successful in 8m34s

Three review findings on 7da52b6:

1. The AssertNotFault doc block landed between the predicate-overload ReadUntilAsync's
   doc block and the helper, so the compiler attached the merged block to the helper and
   ReadUntilAsync lost its docs entirely. Helper and its docs moved above ReadUntilAsync,
   whose docs are back where they belong.

2. The compressed watchdog windows (50 ms grace, 100 ms ceiling) reintroduced the
   load-sensitivity the fix removed, one layer down.
   ReportWatchdogFaultIfNeededAsync measures staleness AFTER the heartbeat frame is
   written and flushed over the real named pipe, so a beat whose pipe I/O outlasts the
   ceiling faults a healthy session no matter how fresh the captured activity was. At
   100 ms that is a plausible stall on a loaded box, and this is the one test asserting
   the watchdog NEVER fires. Widened to a 200 ms grace and a 1 s ceiling — still two
   orders of magnitude under the 75 s production default.

   The inspection loop is now bounded by a 2 s window (twice the ceiling, so a fake whose
   activity stopped advancing still accumulates past it and faults) with a 30-frame floor,
   rather than a fixed 30 frames that no longer outran the wider ceiling. The floor keeps
   a window that saw almost no beats from passing as a clean one. Two seconds stays well
   inside FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in
   flight for the whole window.

3. AssertNotFault renamed AssertNotWorkerFault, matching the WorkerFault body case it
   tests.

Scenario intent unchanged: long in-flight command, pump refreshing, zero fault frames,
reply delivered. Test-only.
This commit is contained in:
Joseph Doherty
2026-08-18 07:01:16 -04:00
parent d1ae43d0d3
commit 462850afb7
@@ -1,5 +1,6 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Diagnostics;
using System.IO; using System.IO;
using System.IO.Pipes; using System.IO.Pipes;
using System.Runtime.InteropServices; using System.Runtime.InteropServices;
@@ -1451,17 +1452,17 @@ public sealed class WorkerPipeSessionTests
/// refresh <c>LastActivityUtc</c> on every wait iteration, so a healthy /// refresh <c>LastActivityUtc</c> on every wait iteration, so a healthy
/// <c>ReadBulk</c> holding the STA far longer than /// <c>ReadBulk</c> holding the STA far longer than
/// <c>HeartbeatStuckCeiling</c> (75 s in production) keeps its activity /// <c>HeartbeatStuckCeiling</c> (75 s in production) keeps its activity
/// timestamp fresh. This test compresses the clock — a 100 ms ceiling /// timestamp fresh. This test compresses the clock — a 1 s ceiling with
/// with a command in flight across a window many multiples longer — and /// a command in flight across a window twice as long — and models the
/// models the pump refresh with /// pump refresh with
/// <see cref="FakeRuntimeSession.RefreshStaActivityOnCapture"/>, which /// <see cref="FakeRuntimeSession.RefreshStaActivityOnCapture"/>, which
/// stamps activity at every heartbeat capture exactly as the pump's /// stamps activity at every heartbeat capture exactly as the pump's
/// per-iteration <c>MarkActivity()</c> does. The refresh has to be in /// per-iteration <c>MarkActivity()</c> does. The refresh has to be in
/// effect from construction, not from the moment the command blocks: /// effect from construction, not from the moment the command blocks: the
/// with a 50 ms grace, the idle window covering handshake and startup /// idle window covering handshake and startup carries no correlation id
/// carries no correlation id for the watchdog to suppress on, so a fake /// for the watchdog to suppress on, so a fake whose activity timestamp is
/// whose activity timestamp is frozen at construction is reported /// frozen at construction is reported <c>StaHung</c> before the scenario
/// <c>StaHung</c> before the scenario under test even starts. Contrast /// under test even starts. Contrast
/// <see cref="RunAsync_WhenStaActivityIsStaleBeyondCeilingWithCommandInFlight_WritesWatchdogFault"/>, /// <see cref="RunAsync_WhenStaActivityIsStaleBeyondCeilingWithCommandInFlight_WritesWatchdogFault"/>,
/// where a frozen timestamp beyond the ceiling correctly faults; here /// where a frozen timestamp beyond the ceiling correctly faults; here
/// the refreshed timestamp must keep the fault suppressed and let the /// the refreshed timestamp must keep the fault suppressed and let the
@@ -1489,9 +1490,15 @@ public sealed class WorkerPipeSessionTests
runtime, runtime,
new WorkerPipeSessionOptions new WorkerPipeSessionOptions
{ {
// Compressed relative to production (75 s ceiling), but no further than the real
// pipe underneath can carry. ReportWatchdogFaultIfNeededAsync measures staleness
// AFTER the heartbeat frame has been written and flushed, so any beat whose pipe
// I/O outlasts the ceiling faults a healthy session. At a 100 ms ceiling that is a
// plausible stall on a loaded box; at 1 s it is not — and this is the one test
// asserting the watchdog NEVER fires, so it has to hold under load.
HeartbeatInterval = TimeSpan.FromMilliseconds(20), HeartbeatInterval = TimeSpan.FromMilliseconds(20),
HeartbeatGrace = TimeSpan.FromMilliseconds(50), HeartbeatGrace = TimeSpan.FromMilliseconds(200),
HeartbeatStuckCeiling = TimeSpan.FromMilliseconds(100), HeartbeatStuckCeiling = TimeSpan.FromSeconds(1),
}); });
Task runTask = session.RunAsync(cancellation.Token); Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token); await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
@@ -1514,16 +1521,22 @@ public sealed class WorkerPipeSessionTests
lastEventSequence: 0, lastEventSequence: 0,
currentCommandCorrelationId: "long-bulk-read")); currentCommandCorrelationId: "long-bulk-read"));
// Inspect a bounded number of frames over a window many multiples of the // Inspect frames across a window twice the stuck ceiling — long enough that a fake whose
// 100 ms ceiling (at least 30 heartbeats at 20 ms ~ 600 ms). None may be // activity timestamp stopped advancing would accumulate staleness past the ceiling and
// a WorkerFault while activity is continuously refreshed. // fault — and require the beats to have actually flowed while it ran, so an inspection
const int framesToInspect = 30; // that saw a couple of frames and timed out cannot pass for a clean window. None may be a
// WorkerFault while activity is continuously refreshed. The window stays well inside
// FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight
// throughout.
TimeSpan inspectionWindow = TimeSpan.FromSeconds(2);
const int minimumFramesInspected = 30;
Stopwatch inspection = Stopwatch.StartNew();
int frameIndex = 0; int frameIndex = 0;
for (; frameIndex < framesToInspect; frameIndex++) while (inspection.Elapsed < inspectionWindow || frameIndex < minimumFramesInspected)
{ {
WorkerEnvelope envelope = await pipePair.GatewayReader WorkerEnvelope envelope = await pipePair.GatewayReader
.ReadAsync(cancellation.Token); .ReadAsync(cancellation.Token);
AssertNotFault(envelope, frameIndex); AssertNotWorkerFault(envelope, frameIndex++);
} }
// Release the command with the pump still running — as it is in // Release the command with the pump still running — as it is in
@@ -1537,7 +1550,7 @@ public sealed class WorkerPipeSessionTests
{ {
WorkerEnvelope envelope = await pipePair.GatewayReader WorkerEnvelope envelope = await pipePair.GatewayReader
.ReadAsync(cancellation.Token); .ReadAsync(cancellation.Token);
AssertNotFault(envelope, frameIndex++); AssertNotWorkerFault(envelope, frameIndex++);
if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply
&& envelope.CorrelationId == "long-bulk-read") && envelope.CorrelationId == "long-bulk-read")
{ {
@@ -2224,12 +2237,6 @@ public sealed class WorkerPipeSessionTests
cancellationToken); cancellationToken);
} }
/// <summary>Reads frames until one matches the expected body type and predicate.</summary>
/// <param name="reader">Frame reader.</param>
/// <param name="expectedBody">Expected body case.</param>
/// <param name="predicate">Predicate to match against envelope.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>The matching envelope.</returns>
/// <summary> /// <summary>
/// Fails when the frame is a <c>WorkerFault</c>, naming the category and diagnostic message. /// Fails when the frame is a <c>WorkerFault</c>, naming the category and diagnostic message.
/// A bare body-case comparison reports only "expected not WorkerFault", which says nothing /// A bare body-case comparison reports only "expected not WorkerFault", which says nothing
@@ -2238,7 +2245,7 @@ public sealed class WorkerPipeSessionTests
/// </summary> /// </summary>
/// <param name="envelope">Frame read from the gateway end.</param> /// <param name="envelope">Frame read from the gateway end.</param>
/// <param name="frameIndex">Ordinal of the frame within the inspected run.</param> /// <param name="frameIndex">Ordinal of the frame within the inspected run.</param>
private static void AssertNotFault(WorkerEnvelope envelope, int frameIndex) private static void AssertNotWorkerFault(WorkerEnvelope envelope, int frameIndex)
{ {
if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault) if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault)
{ {
@@ -2250,6 +2257,12 @@ public sealed class WorkerPipeSessionTests
+ envelope.WorkerFault.DiagnosticMessage); + envelope.WorkerFault.DiagnosticMessage);
} }
/// <summary>Reads frames until one matches the expected body type and predicate.</summary>
/// <param name="reader">Frame reader.</param>
/// <param name="expectedBody">Expected body case.</param>
/// <param name="predicate">Predicate to match against envelope.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>The matching envelope.</returns>
private static async Task<WorkerEnvelope> ReadUntilAsync( private static async Task<WorkerEnvelope> ReadUntilAsync(
WorkerFrameReader reader, WorkerFrameReader reader,
WorkerEnvelope.BodyOneofCase expectedBody, WorkerEnvelope.BodyOneofCase expectedBody,