dohertj2/mxaccessgw
1
0
Fork 0
Code Issues 43 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(gateway): remove the worker startup-probe retry options that could never retry (#188)

Browse Source 
WorkerProcessStartedProbe only checks HasExited and throws a non-retryable
WorkerProcessLaunchException, so StartupProbeRetryAttempts and
StartupProbeRetryDelayMilliseconds configured a Polly retry that no shipped
probe could trigger. Real readiness is WorkerClient's WorkerReady handshake
under the same StartupTimeoutSeconds budget, so the probe now runs once and
the two options, their validation, the Polly pipeline (and the server's now
unused Polly.Core reference) and their docs are removed. A stale value in
configuration is ignored by the binder.

Tests: WorkerProcessLauncherTests.LaunchAsync_WhenStartupProbeFails_DoesNotRetryTheProbe,
WorkerOptions_HasNoStartupProbeRetryOptions (both fail on the old code).
This commit is contained in:
Joseph Doherty
2026-11-06 16:05:42 -05:00
parent 3a3d9e8e06
commit ff397b27be
10 changed files with 48 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs
-8
View File
@@ -246,14 +246,6 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
options.StartupTimeoutSeconds,
"MxGateway:Worker:StartupTimeoutSeconds must be greater than zero.",
builder);
AddIfNotPositive(
options.StartupProbeRetryAttempts,
"MxGateway:Worker:StartupProbeRetryAttempts must be greater than zero.",
builder);
AddIfNotPositive(
options.StartupProbeRetryDelayMilliseconds,
"MxGateway:Worker:StartupProbeRetryDelayMilliseconds must be greater than zero.",
builder);
AddIfNotPositive(
options.PipeConnectAttemptTimeoutMilliseconds,
"MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.",
src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs
-6
View File
@@ -15,12 +15,6 @@ public sealed class WorkerOptions
/// <summary>The maximum time in seconds for the worker to start.</summary>
public int StartupTimeoutSeconds { get; init; } = 30;
/// <summary>The number of retry attempts for the startup probe.</summary>
public int StartupProbeRetryAttempts { get; init; } = 3;
/// <summary>The delay in milliseconds between startup probe retries.</summary>
public int StartupProbeRetryDelayMilliseconds { get; init; } = 250;
/// <summary>The timeout in milliseconds for connecting to the worker pipe.</summary>
public int PipeConnectAttemptTimeoutMilliseconds { get; init; } = 2000;
src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs
+5 -48
View File
@@ -5,8 +5,6 @@ using Microsoft.Extensions.Options;
using ZB.MOM.WW.MxGateway.Contracts;
using ZB.MOM.WW.MxGateway.Server.Configuration;
using ZB.MOM.WW.MxGateway.Server.Metrics;
using Polly;
using Polly.Retry;
namespace ZB.MOM.WW.MxGateway.Server.Workers;
@@ -140,15 +138,11 @@ public sealed class WorkerProcessLauncher : IWorkerProcessLauncher
using CancellationTokenSource startupTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
startupTimeout.CancelAfter(TimeSpan.FromSeconds(_workerOptions.StartupTimeoutSeconds));
await CreateStartupProbePipeline(process)
.ExecuteAsync(
async token =>
{
await _startupProbe
.WaitUntilReadyAsync(process, request, token)
.ConfigureAwait(false);
},
startupTimeout.Token)
// The probe runs once. Real readiness is the WorkerReady handshake WorkerClient performs
// after launch, under the same StartupTimeoutSeconds budget; the probe only catches a
// worker that exited immediately, which no retry can recover (#188).
await _startupProbe
.WaitUntilReadyAsync(process, request, startupTimeout.Token)
.ConfigureAwait(false);
return new WorkerProcessHandle(process, commandLine, startedAt);
@@ -310,43 +304,6 @@ public sealed class WorkerProcessLauncher : IWorkerProcessLauncher
}
}
private ResiliencePipeline CreateStartupProbePipeline(IWorkerProcess process)
{
RetryStrategyOptions retryOptions = new()
{
MaxRetryAttempts = Math.Max(0, _workerOptions.StartupProbeRetryAttempts - 1),
BackoffType = DelayBackoffType.Exponential,
UseJitter = true,
Delay = TimeSpan.FromMilliseconds(_workerOptions.StartupProbeRetryDelayMilliseconds),
MaxDelay = TimeSpan.FromSeconds(2),
ShouldHandle = new PredicateBuilder().Handle<Exception>(exception =>
ShouldRetryStartupProbe(exception, process)),
OnRetry = args =>
{
_metrics.RetryAttempted("worker_startup");
_logger.LogDebug(
args.Outcome.Exception,
"Retrying worker startup probe after transient failure. Attempt {Attempt}.",
args.AttemptNumber + 1);
return default;
},
};
return new ResiliencePipelineBuilder()
.AddRetry(retryOptions)
.Build();
}
private static bool ShouldRetryStartupProbe(Exception exception, IWorkerProcess process)
{
if (exception is OperationCanceledException or WorkerProcessLaunchException)
{
return false;
}
return !process.HasExited;
}
private static void ValidateRequest(WorkerProcessLaunchRequest request)
{
if (string.IsNullOrWhiteSpace(request.SessionId))
src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj
-1
View File
@@ -35,7 +35,6 @@
<!-- Security pin: GHSA-846f-f764-76mx / GHSA-2fmv-rv2c-59x7. Microsoft.Data.SqlClient 6.0.2
pulls the vulnerable System.Security.Cryptography.Pkcs 9.0.4; 10.0.13 patches it. -->
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="10.0.13" />
<PackageReference Include="Polly.Core" Version="8.6.6" />
</ItemGroup>
<ItemGroup>
src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs
-3
View File
@@ -38,8 +38,6 @@ public sealed class GatewayOptionsTests
Assert.Equal(@"src\ZB.MOM.WW.MxGateway.Worker\bin\x86\Release\ZB.MOM.WW.MxGateway.Worker.exe", options.Worker.ExecutablePath);
Assert.Equal(WorkerArchitecture.X86, options.Worker.RequiredArchitecture);
Assert.Equal(30, options.Worker.StartupTimeoutSeconds);
Assert.Equal(3, options.Worker.StartupProbeRetryAttempts);
Assert.Equal(250, options.Worker.StartupProbeRetryDelayMilliseconds);
Assert.Equal(2000, options.Worker.PipeConnectAttemptTimeoutMilliseconds);
Assert.Equal(1500, options.Worker.WriteCompletionWaitMilliseconds);
Assert.Equal(10, options.Worker.ShutdownTimeoutSeconds);
@@ -110,7 +108,6 @@ public sealed class GatewayOptionsTests
/// <param name="expectedFailure">Expected validation error message.</param>
[Theory]
[InlineData("MxGateway:Worker:ExecutablePath", "worker.dll", "MxGateway:Worker:ExecutablePath must point to a .exe file.")]
[InlineData("MxGateway:Worker:StartupProbeRetryAttempts", "0", "MxGateway:Worker:StartupProbeRetryAttempts must be greater than zero.")]
[InlineData("MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds", "0", "MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.")]
[InlineData("MxGateway:Worker:WriteCompletionWaitMilliseconds", "-1", "MxGateway:Worker:WriteCompletionWaitMilliseconds must be greater than or equal to zero.")]
[InlineData("MxGateway:Sessions:DefaultLeaseSeconds", "0", "MxGateway:Sessions:DefaultLeaseSeconds must be greater than zero.")]
src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs
+31 -16
View File
@@ -167,32 +167,48 @@ public sealed class WorkerProcessLauncherTests
Assert.Equal(1, metrics.GetSnapshot().WorkerKills);
}
/// <summary>Verifies that transient startup probe failures are retried without respawning the worker process.</summary>
/// <summary>
/// The startup probe runs once. A probe failure fails the launch — there is no retry to
/// configure, because real readiness is the WorkerReady handshake that follows the launch
/// (#188).
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task LaunchAsync_WhenStartupProbeFailsTransiently_RetriesWithoutRespawningWorker()
public async Task LaunchAsync_WhenStartupProbeFails_DoesNotRetryTheProbe()
{
using TestDirectory directory = TestDirectory.Create();
string executablePath = directory.CreateWorkerExecutable(machine: 0x014c);
FakeWorkerProcess process = new(processId: 1234);
FakeWorkerProcessFactory processFactory = new(process);
GatewayMetrics metrics = new();
TransientStartupProbe probe = new(failuresBeforeSuccess: 1);
WorkerProcessLauncher launcher = CreateLauncher(
executablePath,
processFactory,
new TransientStartupProbe(failuresBeforeSuccess: 1),
metrics,
startupProbeRetryAttempts: 2,
startupProbeRetryDelayMilliseconds: 1);
probe,
metrics);
using WorkerProcessHandle handle = await launcher.LaunchAsync(CreateRequest());
WorkerProcessLaunchException exception =
await Assert.ThrowsAsync<WorkerProcessLaunchException>(
async () => await launcher.LaunchAsync(CreateRequest()));
Assert.Same(process, handle.Process);
Assert.Equal(WorkerProcessLaunchErrorCode.StartupFailed, exception.ErrorCode);
Assert.Equal(1, probe.Attempts);
Assert.Equal(1, processFactory.StartCount);
Assert.False(process.KillCalled);
GatewayMetricsSnapshot snapshot = metrics.GetSnapshot();
Assert.Equal(1, snapshot.RetryAttempts);
Assert.Equal(1, snapshot.RetryAttemptsByArea["worker_startup"]);
Assert.True(process.KillCalled);
Assert.Equal(0, metrics.GetSnapshot().RetryAttempts);
}
/// <summary>
/// The retry options are gone from <see cref="WorkerOptions"/> so configuration cannot
/// promise a startup-probe retry that never happens (#188).
/// </summary>
[Fact]
public void WorkerOptions_HasNoStartupProbeRetryOptions()
{
Assert.DoesNotContain(
typeof(WorkerOptions).GetProperties(),
property => property.Name.StartsWith("StartupProbeRetry", StringComparison.Ordinal));
}
/// <summary>Verifies that a startup probe timeout kills and disposes the worker process.</summary>
@@ -290,8 +306,6 @@ public sealed class WorkerProcessLauncherTests
IWorkerStartupProbe startupProbe,
GatewayMetrics? metrics = null,
int startupTimeoutSeconds = 30,
int startupProbeRetryAttempts = 3,
int startupProbeRetryDelayMilliseconds = 250,
int eventQueueCapacity = 10000,
string ackClientName = "MxGateway")
{
@@ -303,8 +317,6 @@ public sealed class WorkerProcessLauncherTests
ExecutablePath = executablePath,
RequiredArchitecture = WorkerArchitecture.X86,
StartupTimeoutSeconds = startupTimeoutSeconds,
StartupProbeRetryAttempts = startupProbeRetryAttempts,
StartupProbeRetryDelayMilliseconds = startupProbeRetryDelayMilliseconds,
EventQueueCapacity = eventQueueCapacity,
},
};
@@ -388,6 +400,9 @@ public sealed class WorkerProcessLauncherTests
{
private int _attempts;
/// <summary>Gets the number of times the probe was invoked.</summary>
public int Attempts => Volatile.Read(ref _attempts);
/// <inheritdoc />
public Task WaitUntilReadyAsync(
IWorkerProcess process,