fix(client-dotnet): size the safe-unary retry budget for every attempt (#197)
ExecuteSafeUnaryAsync capped the whole retry pipeline at one DefaultCallTimeout while each attempt carried the same deadline, so a first attempt ending in DeadlineExceeded spent the entire budget and the retry never ran. The outer cap is now MaxAttempts x DefaultCallTimeout plus MaxDelay per backoff (MxGatewayClientRetryPolicy.SafeUnaryBudget), in both MxGatewayClient and GalaxyRepositoryClient; a budget too large for a cancellation timer leaves only the per-attempt deadlines. Tests: MxGatewayClientWireTests.SafeUnaryRetry_AfterDeadlineExceededFirstAttempt_RunsTheRetry (real gRPC loopback server stalls the first Invoke; fails with Cancelled on the old code), plus two SafeUnaryBudget unit tests.
This commit is contained in:
@@ -122,6 +122,11 @@ The .NET client applies a bounded Polly retry policy only to idempotent calls:
|
||||
streams, writes, secured writes, authentication, registration, item management,
|
||||
or subscription changes because those calls can partially succeed in MXAccess.
|
||||
|
||||
Each attempt carries its own `DefaultCallTimeout` deadline. The overall budget
|
||||
for a retried call is `MaxAttempts × DefaultCallTimeout` plus `MaxDelay` for each
|
||||
backoff, so an attempt that ends in `DeadlineExceeded` leaves the retry its own
|
||||
full deadline instead of consuming the whole budget.
|
||||
|
||||
API key may be loaded from `MXGATEWAY_API_KEY` by the CLI, not implicitly by the
|
||||
library constructor unless a helper explicitly says it does that.
|
||||
|
||||
|
||||
@@ -157,7 +157,9 @@ worker; call until empty). `DisposeAsync` closes the session best-effort and doe
|
||||
not throw when the gateway is unreachable or the client is already disposed.
|
||||
|
||||
Retry applies only to idempotent calls (`CloseSessionRawAsync`, ping,
|
||||
session-state and worker-info commands, Galaxy reads). `AcknowledgeAlarmAsync`
|
||||
session-state and worker-info commands, Galaxy reads). Each attempt gets its own
|
||||
`DefaultCallTimeout`; a retried call can take up to `Retry.MaxAttempts ×
|
||||
DefaultCallTimeout` plus the backoff between attempts. `AcknowledgeAlarmAsync`
|
||||
is never retried because it is a non-idempotent operator write that re-sends the
|
||||
operator password and can double-acknowledge in the Historian.
|
||||
|
||||
|
||||
@@ -2,6 +2,39 @@ namespace ZB.MOM.WW.MxGateway.Client.Tests;
|
||||
|
||||
public sealed class MxGatewayClientOptionsTests
|
||||
{
|
||||
/// <summary>
|
||||
/// The safe-unary budget covers every attempt's deadline plus the backoff between them, so a
|
||||
/// first attempt that hits its deadline does not consume the retry's time (#197).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void SafeUnaryBudget_CoversEveryAttemptAndBackoff()
|
||||
{
|
||||
var options = new MxGatewayClientOptions
|
||||
{
|
||||
Endpoint = new Uri("http://localhost:5000"),
|
||||
ApiKey = "test-api-key",
|
||||
DefaultCallTimeout = TimeSpan.FromSeconds(30),
|
||||
Retry = new MxGatewayClientRetryOptions { MaxAttempts = 3, MaxDelay = TimeSpan.FromSeconds(2) },
|
||||
};
|
||||
|
||||
Assert.Equal(TimeSpan.FromSeconds(94), MxGatewayClientRetryPolicy.SafeUnaryBudget(options));
|
||||
}
|
||||
|
||||
/// <summary>A budget beyond what a cancellation timer accepts means no outer cap rather than a throw.</summary>
|
||||
[Fact]
|
||||
public void SafeUnaryBudget_WhenTooLargeForATimer_ReturnsNull()
|
||||
{
|
||||
var options = new MxGatewayClientOptions
|
||||
{
|
||||
Endpoint = new Uri("http://localhost:5000"),
|
||||
ApiKey = "test-api-key",
|
||||
DefaultCallTimeout = TimeSpan.FromDays(40),
|
||||
Retry = new MxGatewayClientRetryOptions { MaxAttempts = 2 },
|
||||
};
|
||||
|
||||
Assert.Null(MxGatewayClientRetryPolicy.SafeUnaryBudget(options));
|
||||
}
|
||||
|
||||
/// <summary>Verifies that options with valid endpoint and API key pass validation.</summary>
|
||||
[Fact]
|
||||
public void Validate_WithAbsoluteEndpointAndApiKey_Succeeds()
|
||||
|
||||
@@ -90,6 +90,41 @@ public sealed class MxGatewayClientWireTests
|
||||
server.Service.AuthorizationByMethod);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A safe unary call whose first attempt runs into its deadline must still get its retry:
|
||||
/// the outer budget covers every attempt, not just one DefaultCallTimeout (#197).
|
||||
/// </summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task SafeUnaryRetry_AfterDeadlineExceededFirstAttempt_RunsTheRetry()
|
||||
{
|
||||
await using WireFakeGatewayServer server = await WireFakeGatewayServer.StartAsync(service =>
|
||||
service.StallFirstInvokes = 1);
|
||||
await using MxGatewayClient client = server.CreateClient(
|
||||
ApiKey,
|
||||
TimeSpan.FromMilliseconds(500),
|
||||
new MxGatewayClientRetryOptions
|
||||
{
|
||||
MaxAttempts = 2,
|
||||
Delay = TimeSpan.FromMilliseconds(10),
|
||||
MaxDelay = TimeSpan.FromMilliseconds(10),
|
||||
UseJitter = false,
|
||||
});
|
||||
|
||||
MxCommandReply reply = await client.InvokeAsync(new MxCommandRequest
|
||||
{
|
||||
SessionId = WireFakeGatewayServer.FakeGatewayService.SessionId,
|
||||
Command = new MxCommand
|
||||
{
|
||||
Kind = MxCommandKind.Ping,
|
||||
Ping = new PingCommand(),
|
||||
},
|
||||
});
|
||||
|
||||
Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code);
|
||||
Assert.Equal(2, server.Service.InvokeCount);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies the gateway's replay-gap sentinel survives serialization and is
|
||||
/// surfaced as a typed, non-terminal stream item.
|
||||
|
||||
@@ -88,12 +88,26 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
||||
/// <param name="apiKey">API key the client should present.</param>
|
||||
/// <returns>A client that talks to this server over h2c.</returns>
|
||||
public MxGatewayClient CreateClient(string apiKey) =>
|
||||
CreateClient(apiKey, TimeSpan.FromSeconds(30), new MxGatewayClientRetryOptions());
|
||||
|
||||
/// <summary>
|
||||
/// Creates a client bound to this server's endpoint with an explicit call timeout and retry policy.
|
||||
/// </summary>
|
||||
/// <param name="apiKey">API key the client should present.</param>
|
||||
/// <param name="defaultCallTimeout">Per-attempt unary deadline.</param>
|
||||
/// <param name="retry">Retry options for safe unary calls.</param>
|
||||
/// <returns>A client that talks to this server over h2c.</returns>
|
||||
public MxGatewayClient CreateClient(
|
||||
string apiKey,
|
||||
TimeSpan defaultCallTimeout,
|
||||
MxGatewayClientRetryOptions retry) =>
|
||||
MxGatewayClient.Create(new MxGatewayClientOptions
|
||||
{
|
||||
Endpoint = Endpoint,
|
||||
ApiKey = apiKey,
|
||||
UseTls = false,
|
||||
DefaultCallTimeout = TimeSpan.FromSeconds(30),
|
||||
DefaultCallTimeout = defaultCallTimeout,
|
||||
Retry = retry,
|
||||
});
|
||||
|
||||
/// <summary>Stops and disposes the underlying web application.</summary>
|
||||
@@ -145,6 +159,17 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
||||
/// </summary>
|
||||
public ReplayGap? ReplayGap { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets how many leading <c>Invoke</c> calls stall until the client's deadline
|
||||
/// expires, so the client sees <see cref="StatusCode.DeadlineExceeded"/> for them.
|
||||
/// </summary>
|
||||
public int StallFirstInvokes { get; set; }
|
||||
|
||||
/// <summary>Gets the number of <c>Invoke</c> calls the server received.</summary>
|
||||
public int InvokeCount => Volatile.Read(ref _invokeCount);
|
||||
|
||||
private int _invokeCount;
|
||||
|
||||
/// <summary>
|
||||
/// Gets the last <c>Invoke</c> request the client sent, as decoded from the wire.
|
||||
/// </summary>
|
||||
@@ -179,17 +204,32 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public override Task<MxCommandReply> Invoke(MxCommandRequest request, ServerCallContext context)
|
||||
public override async Task<MxCommandReply> Invoke(MxCommandRequest request, ServerCallContext context)
|
||||
{
|
||||
Record(context);
|
||||
InvokeRequest = request;
|
||||
|
||||
if (Interlocked.Increment(ref _invokeCount) <= StallFirstInvokes)
|
||||
{
|
||||
// Hold the call past the client's deadline; the cancellation is the deadline
|
||||
// firing (or the client giving up), either way the caller has stopped waiting.
|
||||
try
|
||||
{
|
||||
await Task.Delay(Timeout.InfiniteTimeSpan, context.CancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
}
|
||||
|
||||
throw new RpcException(new Status(StatusCode.DeadlineExceeded, "stalled attempt"));
|
||||
}
|
||||
|
||||
if (DenyInvoke)
|
||||
{
|
||||
throw new RpcException(new Status(StatusCode.PermissionDenied, "invoke scope required"));
|
||||
}
|
||||
|
||||
return Task.FromResult(new MxCommandReply
|
||||
return new MxCommandReply
|
||||
{
|
||||
SessionId = request.SessionId,
|
||||
CorrelationId = request.ClientCorrelationId,
|
||||
@@ -197,7 +237,7 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
||||
ProtocolStatus = Ok(),
|
||||
Hresult = 0,
|
||||
Register = new RegisterReply { ServerHandle = ServerHandle },
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -517,8 +517,15 @@ public sealed class GalaxyRepositoryClient : IAsyncDisposable
|
||||
Func<CancellationToken, Task<T>> call,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// Each attempt carries its own DefaultCallTimeout deadline, so the outer cap must cover
|
||||
// every attempt plus the backoff between them; capping it at one DefaultCallTimeout let a
|
||||
// DeadlineExceeded first attempt spend the whole budget and the retry never ran (#197).
|
||||
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||
timeout.CancelAfter(Options.DefaultCallTimeout);
|
||||
TimeSpan? budget = MxGatewayClientRetryPolicy.SafeUnaryBudget(Options);
|
||||
if (budget is not null)
|
||||
{
|
||||
timeout.CancelAfter(budget.Value);
|
||||
}
|
||||
|
||||
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
||||
async token => await call(token).ConfigureAwait(false),
|
||||
|
||||
@@ -375,8 +375,15 @@ public sealed class MxGatewayClient : IAsyncDisposable
|
||||
Func<CancellationToken, Task<T>> call,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// Each attempt carries its own DefaultCallTimeout deadline, so the outer cap must cover
|
||||
// every attempt plus the backoff between them; capping it at one DefaultCallTimeout let a
|
||||
// DeadlineExceeded first attempt spend the whole budget and the retry never ran (#197).
|
||||
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||
timeout.CancelAfter(Options.DefaultCallTimeout);
|
||||
TimeSpan? budget = MxGatewayClientRetryPolicy.SafeUnaryBudget(Options);
|
||||
if (budget is not null)
|
||||
{
|
||||
timeout.CancelAfter(budget.Value);
|
||||
}
|
||||
|
||||
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
||||
async token => await call(token).ConfigureAwait(false),
|
||||
|
||||
@@ -41,6 +41,29 @@ internal static class MxGatewayClientRetryPolicy
|
||||
.Build();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the overall budget for one retried safe unary call: every attempt's
|
||||
/// <see cref="MxGatewayClientOptions.DefaultCallTimeout"/> deadline plus the longest backoff
|
||||
/// between attempts. Returns <see langword="null"/> when the budget is too large for a
|
||||
/// cancellation timer, in which case the per-attempt deadlines and the bounded attempt count
|
||||
/// already bound the call.
|
||||
/// </summary>
|
||||
/// <param name="options">The client options supplying the call timeout and retry settings.</param>
|
||||
/// <returns>The overall budget, or <see langword="null"/> for no outer cap.</returns>
|
||||
public static TimeSpan? SafeUnaryBudget(MxGatewayClientOptions options)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(options);
|
||||
|
||||
int attempts = Math.Max(1, options.Retry.MaxAttempts);
|
||||
double milliseconds = (options.DefaultCallTimeout.TotalMilliseconds * attempts)
|
||||
+ (options.Retry.MaxDelay.TotalMilliseconds * (attempts - 1));
|
||||
|
||||
// CancellationTokenSource.CancelAfter rejects anything above uint.MaxValue - 1 ms.
|
||||
return milliseconds >= uint.MaxValue - 1
|
||||
? null
|
||||
: TimeSpan.FromMilliseconds(milliseconds);
|
||||
}
|
||||
|
||||
/// <summary>Returns whether a command kind is eligible for automatic retry on transient failures.</summary>
|
||||
/// <param name="kind">The command kind to check.</param>
|
||||
/// <returns><see langword="true"/> if the command kind may be automatically retried; otherwise, <see langword="false"/>.</returns>
|
||||
|
||||
Reference in New Issue
Block a user