GH-4073: refuse the agent/sender mismatch at bootstrap, plus the gates and tests #4070 left open - #4077
Merged
Merged
Conversation
…ck endpoints
Redis Streams is broken on main. Its native_ack_mode suite is red 4 of 6, and any
stream that both listens with native acks and is a send target -- the node's own
reply endpoint counts -- fails every outgoing batch with
InvalidOperationException: This sender has not been registered.
#4061 (GH-3709) remapped EndpointMode.NativeAck from BufferedSendingAgent to
InlineSendingAgent. InlineSendingAgent is not an ISenderCallback: unlike
SendingAgent, it drives a plain RetryBlock straight into Sender.SendAsync, with no
callback path at all. Meanwhile RedisStreamEndpoint.CreateSender still gated its
inline sender on `Mode == EndpointMode.Inline`, so NativeAck fell to the batched
branch. The agent said inline, the sender said batched, and CreateSendingAgent's
`sender is ISenderRequiresCallback && agent is ISenderCallback` quietly skipped
the registration. #4056 (GH-4046) made that reachable.
Three changes:
* Endpoint.SendsInline -- one definition of "this endpoint's outgoing side is an
inline agent". EndpointMode governs BOTH directions, so the modes that produce
an inline agent are not just Inline. Transports gate on this, not the literal.
* CreateSendingAgent refuses the mismatch at bootstrap instead of falling through.
The invariant is total -- BatchedSender is the only ISenderRequiresCallback in
the codebase and SendingAgent is the only ISenderCallback -- so a
callback-requiring sender under an inline agent is always a transport bug. The
message names the endpoint, both types, and the fix.
* Redis Streams gates on SendsInline.
The bug was already caught, which is the part worth recording: BatchedSender.SendAsync
posts to a block and returns, so the throw lands on a worker thread nothing awaits.
The block logs it and the caller sees only messages that never arrive. What surfaced
was a 30s TimeoutException far from the misconfigured endpoint, which on a
broker-backed suite reads as flakiness. Hence the bootstrap guard: with it, the same
defect fails in 147ms naming the fix, instead of after 2m39s naming nothing.
Rejected: making InlineSendingAgent implement ISenderCallback. It looks like the
cleaner central fix, but BatchedSender.SendAsync returns before the frame is written,
so an inline agent over a batched sender would reintroduce exactly the ack-before-send
window #4061 closed. It silences the exception and restores the bug.
Audited every transport. RabbitMQ and Pulsar, the other two NativeAck adopters, both
always build fire-and-forget senders and are unaffected. SQS, SNS, GCP, Kafka, MQTT
and HTTP share the `Mode == EndpointMode.Inline` gate but none opts into NativeAck, so
each is latent rather than broken; the guard makes any future adoption fail loudly.
TCP refuses Inline outright. Azure Service Bus is fixed separately on #4064.
Verified: Redis 160/160 (was 4 failing), CoreTests 2584/2584, RabbitMQ and Pulsar
native_ack_mode suites green, full wolverine.slnx builds clean on -f net9.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SQS, SNS, GCP Pub/Sub, Kafka, MQTT and HTTP all gated their inline sender on the literal `Mode == EndpointMode.Inline` and fell through to a BatchedSender otherwise -- the same shape that broke Redis Streams. None of them sets supportsNativeAck, so the Mode setter refuses NativeAck and none is broken today. Each would break the day it adopted the mode. Inert by construction: SendsInline is `Mode is Inline or NativeAck`, and NativeAck is unreachable for these six, so the predicate is exactly equivalent to what it replaces until one of them opts in. The value is that opting in then requires no second edit here, which is the failure GH-4073 was. Only CreateSender gates changed. Deliberately untouched: * PubsubEndpoint.buildListener -- the same literal on the LISTENING side, where SendsInline is the wrong question. * SqsListener's ExtendVisibilityWhileHandling check, likewise listening-side. * Endpoint.ModeIgnoresParallelism, which is about the execution block, not sending. * TcpEndpoint, whose supportsMode refuses Inline outright, so it never gets an inline agent and has no gate to widen. * Azure Service Bus, fixed separately on #4064. Verified: full wolverine.slnx clean on -f net9.0, 0 warnings. Wolverine.Http.Tests Transport 63/63 and Kafka configure_consumers_and_publishers 5/5 -- both directly exercise an inline-vs-batched sender gate changed here. The six transports' own CI jobs cover the rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#4070 fixed this regression independently and reached the same design -- an Endpoint.SendsInline predicate replacing the `Mode == EndpointMode.Inline` literal. It merged first, so everything this branch had in common with it is dropped in favor of main's version: * Endpoint.SendsInline -- main's declaration kept, mine removed. Note this auto-merged into TWO declarations of the same property (we added it in different places, so there was no textual conflict for git to report) and would not have compiled. * The Redis, SQS, SNS, Kafka and MQTT sender gates -- main's taken verbatim. * Azure Service Bus -- #4070 fixed it too, so the earlier "leave ASB to #4064" note is moot. What remains here is the delta #4070 does not cover: * The bootstrap guard in EndpointCollection.CreateSendingAgent. Main still has the silent `&&`, so a callback-requiring sender under an inline agent is still skipped quietly rather than refused. That is the part that made this expensive to diagnose: the throw lands on a block worker thread and surfaces only as a ~30s timeout far from the endpoint at fault. * Pub/Sub and HTTP, the two sender gates #4070 left on the literal. Inert today -- neither sets supportsNativeAck -- but they are the last two. * Five regression tests. #4070 shipped none, so nothing currently pins this behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #4070, which fixed this regression independently and landed first.
We reached the same design — an
Endpoint.SendsInlinepredicate replacing theMode == EndpointMode.Inlineliteral — so everything this branch had in common with #4070 has been dropped in favor of main's version. What is left is the delta #4070 does not cover.1. The bootstrap guard (the substantive part)
Main still registers the callback under a single
&&:When the second half is false the registration is silently skipped. #4070 removed the current way of reaching that state, but not the state itself — any future transport that gates its sender wrongly lands right back in it.
That matters because of how the failure presents.
BatchedSender.SendAsyncposts to a block and returns, so the throw lands on a worker thread nothing awaits; the block logs it and the caller sees only messages that never arrive. The observable symptom is a ~30sTimeoutExceptionfar from the endpoint at fault, which on a broker-backed suite reads as flakiness. Measured on the Redis suite: 2m39s to a timeout naming nothing, versus 147ms to a bootstrap error naming the endpoint, both types, and the fix.The invariant is total, which is what makes this safe to enforce:
BatchedSenderis the onlyISenderRequiresCallbackin the codebase andSendingAgentthe onlyISenderCallback, so a callback-requiring sender under an inline agent is always a transport bug.2. The two gates #4070 missed
PubsubEndpoint.CreateSenderandHttpEndpoint.CreateSenderare still on the literal. Inert today — neither setssupportsNativeAck— but they are the last two.Deliberately left alone, because they are not sender gates:
PubsubEndpoint.buildListenerandSqsListener'sExtendVisibilityWhileHandlingcheck (both listening-side, whereSendsInlineis the wrong question) andEndpoint.ModeIgnoresParallelism(execution block, not sending). TCP is exempt entirely —supportsModerefusesInline, so it never gets an inline agent.3. Regression tests
#4070 shipped none, so nothing currently pins this behavior.
native_ack_sender_pairing_is_validated(3, CoreTests) — the mismatch is refused at bootstrap; the same sender is fine under a buffered agent;SendsInlinecovers both inline-producing modes.the_outgoing_side_of_a_native_ack_stream_sends_inline(Redis) — asserts the agent/sender pairing directly, in milliseconds, rather than through a delivery that takes 30s to time out.a_native_ack_stream_actually_delivers_what_is_sent_to_it(Redis) — the end-to-end path.Verification
dotnet build wolverine.slnx -c Release -f net9.0— clean, 0 warningsnative_ack_mode8/8Full CI was green (39/39) on the pre-merge head; re-running against main.