Merge fix/c3-ipc-boundaries: IPC-33 drain budget bounded under the gRPC send cap + validator upper coupling, IPC-39 pre-check sequence allowance, IPC-34 WorkerCancel on caller cancellation + CallerCancelled code, IPC-35 handshake WorkerFault surfaces category
This commit is contained in:
@@ -606,6 +606,15 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
// wrapped in a WorkerEnvelope and the outbound write faults the whole session. Fail fast
|
||||
// at startup rather than mid-traffic. Only checked when both knobs are themselves in range so the
|
||||
// message is not doubled up with the individual range errors.
|
||||
//
|
||||
// IPC-33: the coupling also has an upper half. The worker budgets a DrainEvents reply against the
|
||||
// negotiated frame maximum less two reserves, and that same MxCommandReply is then returned
|
||||
// verbatim through Invoke, where the gateway's gRPC MaxSendMessageSize (= MaxGrpcMessageBytes)
|
||||
// applies. Letting the pipe maximum run arbitrarily far above the gRPC cap lets the worker pack a
|
||||
// reply the pipe carries and the gRPC layer then refuses to send — with the drained events already
|
||||
// removed from the worker queue, so they are lost to both the caller and the event stream. Keeping
|
||||
// the pipe maximum within two reserves of the gRPC cap makes "frame max - 2x reserve" a budget that
|
||||
// is always at or below the gRPC cap, so the wrapper always has a full reserve of margin.
|
||||
private static void ValidateFrameSizeHeadroom(
|
||||
WorkerOptions worker,
|
||||
ProtocolOptions protocol,
|
||||
@@ -628,6 +637,20 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
+ $"{WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes}-byte worker-frame envelope reserve "
|
||||
+ $"(>= {requiredWorkerMax}).");
|
||||
}
|
||||
|
||||
long permittedWorkerMax =
|
||||
(long)protocol.MaxGrpcMessageBytes + (2L * WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes);
|
||||
if (worker.MaxMessageBytes > permittedWorkerMax)
|
||||
{
|
||||
builder.Add(
|
||||
$"MxGateway:Worker:MaxMessageBytes ({worker.MaxMessageBytes}) must be at most "
|
||||
+ $"MxGateway:Protocol:MaxGrpcMessageBytes ({protocol.MaxGrpcMessageBytes}) plus twice the "
|
||||
+ $"{WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes}-byte worker-frame envelope reserve "
|
||||
+ $"(<= {permittedWorkerMax}). The worker budgets a DrainEvents reply against the negotiated "
|
||||
+ "frame maximum less two reserves; a pipe maximum further above the gRPC cap lets the worker "
|
||||
+ "pack a reply the pipe carries but the gRPC send cap refuses, losing the already-drained "
|
||||
+ "events. Lower MxGateway:Worker:MaxMessageBytes or raise MxGateway:Protocol:MaxGrpcMessageBytes.");
|
||||
}
|
||||
}
|
||||
|
||||
private static void AddIfBlank(string? value, string message, ValidationBuilder builder)
|
||||
|
||||
@@ -58,9 +58,12 @@ public sealed class WorkerOptions
|
||||
/// The maximum worker-frame (pipe) message size in bytes. Must stay at least
|
||||
/// <see cref="Workers.WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes"/> above
|
||||
/// <see cref="ProtocolOptions.MaxGrpcMessageBytes"/> so a maximally-sized accepted gRPC payload
|
||||
/// still fits one worker frame; the gateway conveys this value to the worker in the
|
||||
/// handshake (<c>GatewayHello.max_frame_bytes</c>). Default is the 16 MB public gRPC cap
|
||||
/// plus that reserve.
|
||||
/// still fits one worker frame, and at most <em>twice</em> that reserve above it so a
|
||||
/// <c>DrainEvents</c> reply budgeted against this maximum (frame max less two reserves) still
|
||||
/// fits the gRPC send cap it is returned through — otherwise the worker can pack a reply the
|
||||
/// pipe carries and the gRPC layer refuses, losing the already-drained events (IPC-33). The
|
||||
/// gateway conveys this value to the worker in the handshake
|
||||
/// (<c>GatewayHello.max_frame_bytes</c>). Default is the 16 MB public gRPC cap plus one reserve.
|
||||
/// </summary>
|
||||
public int MaxMessageBytes { get; init; } =
|
||||
(16 * 1024 * 1024) + Workers.WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes;
|
||||
|
||||
@@ -1051,6 +1051,7 @@ public sealed class MxAccessGatewayService(
|
||||
{
|
||||
WorkerClientErrorCode.CommandTimeout => StatusCode.DeadlineExceeded,
|
||||
WorkerClientErrorCode.GatewayShutdown => StatusCode.Cancelled,
|
||||
WorkerClientErrorCode.CallerCancelled => StatusCode.Cancelled,
|
||||
WorkerClientErrorCode.InvalidState => StatusCode.FailedPrecondition,
|
||||
WorkerClientErrorCode.ProtocolViolation => StatusCode.Internal,
|
||||
WorkerClientErrorCode.CommandTooLarge => StatusCode.ResourceExhausted,
|
||||
|
||||
@@ -237,15 +237,22 @@ public sealed class WorkerClient : IWorkerClient
|
||||
// CalculateSize. WriteLoopAsync stamps envelope.Sequence immediately before the write
|
||||
// (GWC-28), and a non-zero varint field grows the encoding — so the number computed here
|
||||
// is a lower bound on the frame the writer actually emits, never the frame length. Passing
|
||||
// it as a knownSize would under-length the prefix and desync the worker's framing. The
|
||||
// pre-check stays a pre-check: it is conservative in the right direction.
|
||||
// it as a knownSize would under-length the prefix and desync the worker's framing.
|
||||
//
|
||||
// IPC-39: being a lower bound is conservative for the knownSize question but the wrong
|
||||
// direction for this pre-check, which must not admit an envelope the writer will then
|
||||
// reject with a session-fatal MessageTooLarge. Compare against the frame maximum less the
|
||||
// MaxSequenceEncodingBytes the stamp can add, so every envelope this check accepts still
|
||||
// fits once the sequence is written.
|
||||
int envelopeSize = commandEnvelope.CalculateSize();
|
||||
if (envelopeSize > _connection.FrameOptions.MaxMessageBytes)
|
||||
int preCheckLimit = _connection.FrameOptions.MaxMessageBytes - MaxSequenceEncodingBytes;
|
||||
if (envelopeSize > preCheckLimit)
|
||||
{
|
||||
throw new WorkerClientException(
|
||||
WorkerClientErrorCode.CommandTooLarge,
|
||||
$"Worker command {method} serializes to {envelopeSize} bytes, exceeding the negotiated "
|
||||
+ $"worker-frame maximum of {_connection.FrameOptions.MaxMessageBytes} bytes.");
|
||||
+ $"worker-frame maximum of {_connection.FrameOptions.MaxMessageBytes} bytes less the "
|
||||
+ $"{MaxSequenceEncodingBytes}-byte sequence-stamp allowance.");
|
||||
}
|
||||
|
||||
await EnqueueAsync(commandEnvelope, cancellationToken).ConfigureAwait(false);
|
||||
@@ -284,7 +291,10 @@ public sealed class WorkerClient : IWorkerClient
|
||||
// throw, so it can never replace the timeout the caller is owed.
|
||||
if (removed)
|
||||
{
|
||||
TrySendCancelForTimedOutCommand(correlationId, method, timeout);
|
||||
TrySendCancelForAbandonedCommand(
|
||||
correlationId,
|
||||
method,
|
||||
$"gateway command timeout after {timeout}");
|
||||
}
|
||||
|
||||
throw new WorkerClientException(
|
||||
@@ -293,11 +303,7 @@ public sealed class WorkerClient : IWorkerClient
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
RemovePendingCommandAsFailed(
|
||||
correlationId,
|
||||
pendingCommand,
|
||||
WorkerClientErrorCode.GatewayShutdown,
|
||||
"Command wait was canceled.");
|
||||
AbandonCommandOnCallerCancellation(correlationId, pendingCommand, method);
|
||||
|
||||
// WaitAsync surfaces TaskCanceledException; throwing through the token keeps the
|
||||
// exception the caller observes exactly what the hand-rolled wait produced.
|
||||
@@ -309,11 +315,7 @@ public sealed class WorkerClient : IWorkerClient
|
||||
// The deadline and the caller's cancellation raced and WaitAsync picked the timer.
|
||||
// The old wait classified this as cancellation, so this clause — reached only when
|
||||
// the filter above saw a canceled token — reproduces that treatment exactly.
|
||||
RemovePendingCommandAsFailed(
|
||||
correlationId,
|
||||
pendingCommand,
|
||||
WorkerClientErrorCode.GatewayShutdown,
|
||||
"Command wait was canceled.");
|
||||
AbandonCommandOnCallerCancellation(correlationId, pendingCommand, method);
|
||||
cancellationToken.ThrowIfCancellationRequested();
|
||||
throw;
|
||||
}
|
||||
@@ -455,6 +457,18 @@ public sealed class WorkerClient : IWorkerClient
|
||||
_stopCts.Dispose();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Worst-case number of bytes <see cref="WriteLoopAsync"/>'s sequence stamp adds to an
|
||||
/// envelope that was sized before it: one tag byte plus a ten-byte varint, the maximum
|
||||
/// encoding of a 64-bit protobuf field. The <c>CommandTooLarge</c> pre-check subtracts this
|
||||
/// from the negotiated frame maximum (IPC-39) because it measures the envelope while
|
||||
/// <c>Sequence</c> is still 0 — an unset field costs nothing to encode, so the measured size
|
||||
/// is a lower bound on the frame the writer emits, and without the allowance an envelope in
|
||||
/// the last eleven bytes of the range would pass the pre-check and then fault the session on
|
||||
/// the write.
|
||||
/// </summary>
|
||||
private const int MaxSequenceEncodingBytes = 11;
|
||||
|
||||
/// <summary>Manages writing envelopes to the worker pipe.</summary>
|
||||
private async Task WriteLoopAsync()
|
||||
{
|
||||
@@ -803,6 +817,45 @@ public sealed class WorkerClient : IWorkerClient
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Fails the pending command for a caller whose own token fired — a client deadline shorter
|
||||
/// than the gateway's command timeout, or a disconnect that cancelled the RPC's token — and
|
||||
/// forwards a <c>WorkerCancel</c> for the correlation the caller has abandoned (IPC-34).
|
||||
/// <para>
|
||||
/// Caller cancellation is the common abandonment path, not the rare one, and it leaves the
|
||||
/// worker in exactly the state the timeout arm's cancel exists for: the correlation is still
|
||||
/// on the worker's single STA queue and would execute in its turn, behind live callers, for
|
||||
/// a caller that is already gone. Under a burst of deadline-cancelled calls that backlog is
|
||||
/// precisely what the cancel was added to shorten, so both arms send it.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Gated the same way as the timeout arm: only the caller that owned the removal sends, so a
|
||||
/// reply that won the race is never chased. Additionally skipped once <c>_stopCts</c> has
|
||||
/// fired — the client is closing, the outbound channel is being completed and every pending
|
||||
/// command failed anyway, so a cancel would be at best pointless and at worst a write racing
|
||||
/// teardown.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <param name="correlationId">Correlation id of the abandoned command.</param>
|
||||
/// <param name="pendingCommand">The pending command entry to fail.</param>
|
||||
/// <param name="method">Command method name, for the cancel reason and diagnostics.</param>
|
||||
private void AbandonCommandOnCallerCancellation(
|
||||
string correlationId,
|
||||
PendingCommand pendingCommand,
|
||||
string method)
|
||||
{
|
||||
bool removed = RemovePendingCommandAsFailed(
|
||||
correlationId,
|
||||
pendingCommand,
|
||||
WorkerClientErrorCode.CallerCancelled,
|
||||
"Command wait was canceled.");
|
||||
|
||||
if (removed && !_stopCts.IsCancellationRequested)
|
||||
{
|
||||
TrySendCancelForAbandonedCommand(correlationId, method, "caller cancelled the command");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Forwards a <c>WorkerCancel</c> for a correlation the gateway has given up waiting for, so the
|
||||
/// worker can drop it from its STA queue (<c>WorkerPipeSession</c> routes the envelope to
|
||||
@@ -819,9 +872,10 @@ public sealed class WorkerClient : IWorkerClient
|
||||
/// <remarks>
|
||||
/// The whole body sits under one catch-all that debug-logs, because the guarantee this method
|
||||
/// owes its caller is structural, not incidental: the caller is on the throw path for the
|
||||
/// timeout, so anything escaping here — an envelope that failed to build, a <c>TryWrite</c>
|
||||
/// against a disposed channel, a scheduler refusing the detached task — would replace the
|
||||
/// <see cref="WorkerClientErrorCode.CommandTimeout"/> the caller is owed with an unrelated
|
||||
/// timeout or the cancellation, so anything escaping here — an envelope that failed to build,
|
||||
/// a <c>TryWrite</c> against a disposed channel, a scheduler refusing the detached task —
|
||||
/// would replace the <see cref="WorkerClientErrorCode.CommandTimeout"/> (or the
|
||||
/// <see cref="OperationCanceledException"/>) the caller is owed with an unrelated
|
||||
/// exception. Losing the cancel costs the worker one wasted command; losing the timeout
|
||||
/// misreports why the call failed. The detached task carries its own handler for the same
|
||||
/// reason: its failures (including the <see cref="ObjectDisposedException"/> from
|
||||
@@ -829,13 +883,13 @@ public sealed class WorkerClient : IWorkerClient
|
||||
/// and would otherwise be unobserved. It is deliberately not tracked or awaited — it holds no
|
||||
/// resource the shutdown path needs back, and the outbound channel is completed on close.
|
||||
/// </remarks>
|
||||
/// <param name="correlationId">Correlation id of the command that timed out.</param>
|
||||
/// <param name="method">Command method name, for the cancel reason and diagnostics.</param>
|
||||
/// <param name="timeout">The elapsed command timeout, for the cancel reason.</param>
|
||||
private void TrySendCancelForTimedOutCommand(
|
||||
/// <param name="correlationId">Correlation id of the abandoned command.</param>
|
||||
/// <param name="method">Command method name, for diagnostics.</param>
|
||||
/// <param name="reason">Why the gateway abandoned the command, carried on the <c>WorkerCancel</c>.</param>
|
||||
private void TrySendCancelForAbandonedCommand(
|
||||
string correlationId,
|
||||
string method,
|
||||
TimeSpan timeout)
|
||||
string reason)
|
||||
{
|
||||
try
|
||||
{
|
||||
@@ -843,7 +897,7 @@ public sealed class WorkerClient : IWorkerClient
|
||||
correlationId,
|
||||
envelope => envelope.WorkerCancel = new WorkerCancel
|
||||
{
|
||||
Reason = $"gateway command timeout after {timeout}",
|
||||
Reason = reason,
|
||||
});
|
||||
|
||||
if (_outboundEnvelopes.Writer.TryWrite(cancelEnvelope))
|
||||
@@ -869,10 +923,10 @@ public sealed class WorkerClient : IWorkerClient
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Records a cancel that could not be forwarded for a timed-out command.</summary>
|
||||
/// <summary>Records a cancel that could not be forwarded for an abandoned command.</summary>
|
||||
/// <param name="exception">The failure that stopped the cancel from being sent.</param>
|
||||
/// <param name="method">Command method name of the timed-out command.</param>
|
||||
/// <param name="correlationId">Correlation id of the timed-out command.</param>
|
||||
/// <param name="method">Command method name of the abandoned command.</param>
|
||||
/// <param name="correlationId">Correlation id of the abandoned command.</param>
|
||||
private void LogCancelNotForwarded(
|
||||
Exception exception,
|
||||
string method,
|
||||
@@ -880,7 +934,7 @@ public sealed class WorkerClient : IWorkerClient
|
||||
{
|
||||
_logger.LogDebug(
|
||||
exception,
|
||||
"Could not forward a cancel for timed-out worker command {Method} on session {SessionId} "
|
||||
"Could not forward a cancel for abandoned worker command {Method} on session {SessionId} "
|
||||
+ "and correlation {CorrelationId}.",
|
||||
method,
|
||||
SessionId,
|
||||
@@ -896,6 +950,21 @@ public sealed class WorkerClient : IWorkerClient
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
WorkerEnvelope envelope = await _reader.ReadAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
// IPC-35: a fault frame during the handshake is the worker reporting a structured startup
|
||||
// failure — most often MXAccess COM creation (not installed, not registered, access denied)
|
||||
// — not a protocol problem. Flattening it to ProtocolViolation discarded the category and
|
||||
// diagnostic exactly where they matter most: first session open on a freshly deployed host,
|
||||
// where the message reaches the operator log and the client's OpenFailed detail. Use the
|
||||
// same formatter the steady-state fault path uses so both windows read alike.
|
||||
if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerFault
|
||||
&& expectedBody != WorkerEnvelope.BodyOneofCase.WorkerFault)
|
||||
{
|
||||
throw new WorkerClientException(
|
||||
WorkerClientErrorCode.WorkerFaulted,
|
||||
CreateWorkerFaultMessage(envelope.WorkerFault));
|
||||
}
|
||||
|
||||
if (envelope.BodyCase != expectedBody)
|
||||
{
|
||||
throw new WorkerClientException(
|
||||
|
||||
@@ -10,6 +10,14 @@ public enum WorkerClientErrorCode
|
||||
HeartbeatExpired,
|
||||
ShutdownTimeout,
|
||||
GatewayShutdown,
|
||||
|
||||
// The caller's own cancellation token fired while it waited for the reply — a client deadline
|
||||
// shorter than the gateway's command timeout, or a disconnect that cancelled the RPC's token.
|
||||
// Distinct from GatewayShutdown (which these arms used to report) so metrics and logs stop
|
||||
// recording a shutdown that did not happen; both map to Cancelled at the gRPC boundary, so the
|
||||
// status the client observes is unchanged.
|
||||
CallerCancelled,
|
||||
|
||||
WriteFailed,
|
||||
PendingCommandLimitExceeded,
|
||||
|
||||
|
||||
Reference in New Issue
Block a user