diff --git a/docs/GatewayConfiguration.md b/docs/GatewayConfiguration.md index 378a734..04d425d 100644 --- a/docs/GatewayConfiguration.md +++ b/docs/GatewayConfiguration.md @@ -22,8 +22,6 @@ paths, timeouts, queue sizes, enum values, or protocol values are invalid. "WorkingDirectory": null, "RequiredArchitecture": "X86", "StartupTimeoutSeconds": 30, - "StartupProbeRetryAttempts": 3, - "StartupProbeRetryDelayMilliseconds": 250, "PipeConnectAttemptTimeoutMilliseconds": 2000, "ShutdownTimeoutSeconds": 10, "HeartbeatIntervalSeconds": 5, @@ -116,8 +114,6 @@ launch CWD (SEC-01, SEC-33). | `MxGateway:Worker:WorkingDirectory` | `null` | Optional working directory for the worker process. When set, it must be a valid filesystem path. | | `MxGateway:Worker:RequiredArchitecture` | `X86` | Required Portable Executable architecture for the worker. Supported values are `X86` and `X64`; MXAccess parity uses `X86`. | | `MxGateway:Worker:StartupTimeoutSeconds` | `30` | Total startup budget for process launch, startup probe, pipe connect, handshake, and worker readiness. | -| `MxGateway:Worker:StartupProbeRetryAttempts` | `3` | Number of retry attempts for transient worker startup probe failures before pipe connection and handshake continue. | -| `MxGateway:Worker:StartupProbeRetryDelayMilliseconds` | `250` | Delay between transient startup probe retry attempts. | | `MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds` | `2000` | Per-attempt timeout used by the worker named-pipe connect retry path. The overall pipe connection still stays under the startup budget. | | `MxGateway:Worker:WriteCompletionWaitMilliseconds` | `1500` | Bounded wait the worker holds a unary write reply (`Write`/`Write2`/`WriteSecured`/`WriteSecured2`; bulk writes excluded) for the matching MXAccess `OnWriteComplete` callback, so the reply's `statuses` carry the real commit outcome. `0` disables the wait (pure fire-and-forget replies). Must be `>= 0`. The gateway conveys the value to the worker via the `MXGATEWAY_WORKER_WRITE_COMPLETION_WAIT_MS` environment variable. Consumers that time their own writes must budget above this wait: OtOpcUa's GalaxyDriver wraps gateway writes in a 2 s Tier A resilience timeout, so a deployment raising this option past ~2000 must raise that driver `ResilienceConfig` write timeout in step or slow-but-successful commits surface as consumer-side failures. | | `MxGateway:Worker:EventQueueCapacity` | `10000` | Capacity, in events, of the worker's outbound MXAccess event queue. Must be between `1000` and `1000000`. This is burst headroom, not a throttle: the queue has no drop policy, so filling it records a `QueueOverflow` worker fault and faults the session. Raise it for sessions whose subscription set can outrun the drain loop (large advise sets, slow event consumers); the backing queue pre-allocates its slots, so the ceiling keeps a mistyped value from committing the 32-bit worker to an outsized allocation. The gateway conveys the value to the worker via the `MXGATEWAY_EVENT_QUEUE_CAPACITY` environment variable; a missing or unusable value leaves the worker on the 10000 default rather than failing the session. | @@ -126,7 +122,6 @@ launch CWD (SEC-01, SEC-33). | `MxGateway:Worker:HeartbeatGraceSeconds` | `15` | Maximum age of the last worker heartbeat before the gateway faults the worker. This must be greater than or equal to `HeartbeatIntervalSeconds`. | | `MxGateway:Worker:MaxMessageBytes` | `16842752` | Maximum worker IPC frame payload size in bytes. The validator allows values from `1024` through `268435456`, and additionally requires this to be at least `MxGateway:Protocol:MaxGrpcMessageBytes` plus a 65536-byte (64 KiB) envelope-overhead reserve **and at most that cap plus twice the reserve**. The default is the 16 MiB public gRPC cap plus one reserve. The gateway conveys the negotiated value to the worker in the handshake (`GatewayHello.max_frame_bytes`), so the worker frames to the same limit rather than a hard-coded default. The band is narrow by design, so changing either message-size knob alone no longer validates — move `MxGateway:Protocol:MaxGrpcMessageBytes` and this one together, keeping this one one to two reserves above it. | -`StartupProbeRetryAttempts`, `StartupProbeRetryDelayMilliseconds`, `PipeConnectAttemptTimeoutMilliseconds`, timeout values, heartbeat values, and `MaxMessageBytes` must be positive. `MaxMessageBytes` is intentionally bounded to avoid accidental large allocations from malformed or oversized frames. It diff --git a/docs/GatewayProcessDesign.md b/docs/GatewayProcessDesign.md index 549c563..d4dfabc 100644 --- a/docs/GatewayProcessDesign.md +++ b/docs/GatewayProcessDesign.md @@ -940,8 +940,6 @@ Suggested configuration shape: "WorkingDirectory": null, "RequiredArchitecture": "X86", "StartupTimeoutSeconds": 30, - "StartupProbeRetryAttempts": 3, - "StartupProbeRetryDelayMilliseconds": 250, "PipeConnectAttemptTimeoutMilliseconds": 2000, "ShutdownTimeoutSeconds": 10, "HeartbeatIntervalSeconds": 5, diff --git a/docs/Metrics.md b/docs/Metrics.md index 10cd4a0..3a0fe45 100644 --- a/docs/Metrics.md +++ b/docs/Metrics.md @@ -109,7 +109,7 @@ The scalar fields mirror the counters and gauges. The four dictionaries provide - `CommandFailuresByMethod` keys by gRPC method name. - `EventsByFamily` keys by event family (the `Family` enum on a worker event). - `EventsBySession` keys by `sessionId`; entries are removed via `RemoveSessionEvents` when a session closes so the map does not grow without bound. -- `RetryAttemptsByArea` keys by the resilience `area` tag, e.g. `worker_startup`. +- `RetryAttemptsByArea` keys by the resilience `area` tag passed to `RetryAttempted(area)`. No gateway code path records a retry at present (the worker launcher's startup-probe retry was removed in #188), so it is empty unless a future resilience path records one. `EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (PERF-02). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients. @@ -152,15 +152,13 @@ _metrics.RemoveSessionEvents(session.SessionId); ### Worker process launcher -`Workers/WorkerProcessLauncher.cs` records process-level kills and startup retries: +`Workers/WorkerProcessLauncher.cs` records process-level kills: ```csharp _metrics.WorkerKilled(reason); -... -_metrics.RetryAttempted("worker_startup"); ``` -The `worker_startup` tag is hard-coded so the `RetryAttemptsByArea` snapshot reports launcher retries distinctly from other resilience areas. +The launcher no longer records retries: its startup probe runs once (#188). ### Session worker client factory diff --git a/docs/WorkerProcessLauncher.md b/docs/WorkerProcessLauncher.md index aec577f..6667abf 100644 --- a/docs/WorkerProcessLauncher.md +++ b/docs/WorkerProcessLauncher.md @@ -49,17 +49,15 @@ Before starting the process, the launcher validates that the configured worker path exists, has a `.exe` extension, contains a valid Windows Portable Executable header, and matches the configured `RequiredArchitecture`. -After the process starts, `IWorkerStartupProbe` waits for startup readiness. -The default probe only verifies that the worker did not exit immediately. The -worker client replaces this probe when pipe connection, hello, and -`WorkerReady` handling are implemented. - -Startup probing uses a bounded Polly retry policy. The gateway starts the worker -process once, then retries only transient startup-probe failures while the -process remains alive. The policy is configured by -`WorkerOptions.StartupProbeRetryAttempts` and -`WorkerOptions.StartupProbeRetryDelayMilliseconds`; the retry counter is -recorded as `mxgateway.retries.attempted` with `area=worker_startup`. +After the process starts, `IWorkerStartupProbe` runs once. The default probe +only verifies that the worker did not exit immediately; a worker that has +already exited cannot be recovered by waiting, so the probe is not retried. +Real readiness is the pipe connection, hello, and `WorkerReady` handshake that +`WorkerClient` performs after the launch, under the same +`WorkerOptions.StartupTimeoutSeconds` budget. (The former +`StartupProbeRetryAttempts` / `StartupProbeRetryDelayMilliseconds` options were +removed because no shipped probe could ever produce a retryable failure; a +stale value in configuration is ignored by the binder.) The worker uses the exported pipe-attempt value inside its own bounded Polly retry loop. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs index 89710a3..eab05e7 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs @@ -246,14 +246,6 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBaseThe maximum time in seconds for the worker to start. public int StartupTimeoutSeconds { get; init; } = 30; - /// The number of retry attempts for the startup probe. - public int StartupProbeRetryAttempts { get; init; } = 3; - - /// The delay in milliseconds between startup probe retries. - public int StartupProbeRetryDelayMilliseconds { get; init; } = 250; - /// The timeout in milliseconds for connecting to the worker pipe. public int PipeConnectAttemptTimeoutMilliseconds { get; init; } = 2000; diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs index 11ca0f6..178d9d7 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs @@ -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 => - 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)) diff --git a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj index c850019..89e1b5b 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj +++ b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj @@ -35,7 +35,6 @@ - diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs index 8c471e9..889be21 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs @@ -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 /// Expected validation error message. [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.")] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs index 0f8a4ef..226863e 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs @@ -167,32 +167,48 @@ public sealed class WorkerProcessLauncherTests Assert.Equal(1, metrics.GetSnapshot().WorkerKills); } - /// Verifies that transient startup probe failures are retried without respawning the worker process. + /// + /// 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). + /// /// A task that represents the asynchronous operation. [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( + 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); + } + + /// + /// The retry options are gone from so configuration cannot + /// promise a startup-probe retry that never happens (#188). + /// + [Fact] + public void WorkerOptions_HasNoStartupProbeRetryOptions() + { + Assert.DoesNotContain( + typeof(WorkerOptions).GetProperties(), + property => property.Name.StartsWith("StartupProbeRetry", StringComparison.Ordinal)); } /// Verifies that a startup probe timeout kills and disposes the worker process. @@ -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; + /// Gets the number of times the probe was invoked. + public int Attempts => Volatile.Read(ref _attempts); + /// public Task WaitUntilReadyAsync( IWorkerProcess process,