Skip to content

GH-3709: send inline from NativeAck endpoints to close the interceptor loss window - #4061

Merged
jeremydmiller merged 1 commit into
mainfrom
gh-3709/interceptor-loss-window
Aug 24, 2026
Merged

GH-3709: send inline from NativeAck endpoints to close the interceptor loss window#4061
jeremydmiller merged 1 commit into
mainfrom
gh-3709/interceptor-loss-window

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Addresses item 4 of #3709 — see the analysis comment. Does not close the issue; the multi-node and failover tests remain.

The bug

GlobalPartitionedInterceptor.TryReRouteAsync re-publishes a message and then acknowledges the source delivery:

await bus.PublishAsync(envelope.Message!, options);
await listener.CompleteAsync(envelope);

#4040 mapped EndpointMode.NativeAck to BufferedSendingAgent, reasoning that the mode means "no outbox" — which is what BufferedInMemory means. That is correct for ordinary sends and wrong for this particular sequence. BufferedSendingAgent.storeAndForwardAsync posts to an in-memory Block, so the source delivery could be settled while the only copy of the message was sitting in this process's memory. Crash in between and it is gone, with no redelivery available because the source was already acked.

Under the durable topology this hop was safe for the reason #3709 item 4 predicted: the re-publish landed in the durable outbox before the source ack. #4041 made native-ack global partitioning reachable, which made this live on main.

The fix

NativeAck now maps to InlineSendingAgent, which posts to a RetryBlock that runs the send rather than queueing it.

The reasoning for doing it at the mode level rather than special-casing the interceptor: NativeAck is a listening optimization. Nobody selects it for its sending characteristics, so the outgoing side should take the safe option rather than the fast one. A narrower special-case (Mode == NativeAck && UsedInShardedTopology) was considered and rejected as more machinery for no benefit.

Two limits, stated rather than glossed

This narrows the window rather than closing it. Wolverine publishes with RabbitMQ publisher confirms disabled by default, so an inline send awaits the frame being written, not the broker acknowledging it. Enabling confirms for these endpoints is the remaining step and is not in this PR.

The window was never deterministic. Block<T>.PostAsync runs its handler inline when the block is idle, so the buffered agent happened to deliver synchronously for a single envelope. The gap opens only when the block already has queued work or a send is retrying — making this a load-dependent bug that works fine in tests and loses messages in production. My original write-up on #3709 implied it was deterministic; it is not, and that makes it harder to catch, not easier.

On the tests

There is deliberately no behavioral "the send reached the transport before the call returned" test. One was written. It passes identically under both mappings, for the reason above, and was deleted rather than kept as decoration — a test that cannot fail is worse than no test, because it reads like coverage.

What is here is the agent-type assertions, plus one guarding that BufferedInMemory still gets the buffered agent so the change is not over-applied. Both were red-baselined: with the NativeAck arm reverted to BufferedSendingAgent, a_native_ack_endpoint_gets_the_inline_sending_agent fails and the buffered one still passes.

Note the tests use a purpose-built Endpoint rather than StubEndpoint. StubEndpoint assigns Agent = this in its constructor and is its own ISendingAgent, so it bypasses buildSendingAgent entirely — the first version of these tests used it and passed under both mappings for that reason too.

Verification

  • dotnet build wolverine.slnx -c Release -f net9.0 — clean, 0 warnings, 0 errors
  • CoreTests — 2552 total, 0 failed, 2 skipped

🤖 Generated with Claude Code

…r loss window

GlobalPartitionedInterceptor.TryReRouteAsync re-publishes a message and then acks
the SOURCE delivery:

    await bus.PublishAsync(envelope.Message!, options);
    await listener.CompleteAsync(envelope);

GH-3708 mapped EndpointMode.NativeAck to BufferedSendingAgent on the sending
side, reasoning that the mode means "no outbox", which is what BufferedInMemory
means. That is right for ordinary sends and wrong for this sequence:
BufferedSendingAgent.storeAndForwardAsync posts to an in-memory Block, so the
source delivery could be settled while the only copy of the message lived in
this process's memory. A crash in between lost it outright -- no redelivery,
because the source was already acked. Under the durable topology that hop was
safe: the re-publish hit the outbox before the ack. GH-4041 made native-ack
global partitioning reachable, so this became live.

NativeAck now maps to InlineSendingAgent. The mode is a LISTENING optimization --
nobody chooses it for its sending characteristics -- so the outgoing side should
take the safe option rather than the fast one.

Two honest limits, both recorded in comments rather than left to be discovered:

* This narrows the window rather than closing it. Wolverine publishes with
  RabbitMQ publisher confirms disabled by default, so an inline send awaits the
  frame being written, not the broker acknowledging it.
* The window was never deterministic. Block<T>.PostAsync runs its handler inline
  when the block is idle, so the buffered agent happened to deliver
  synchronously for a single envelope; the gap opens only when the block already
  has queued work or a send is retrying. That makes it a load-dependent bug --
  it works in tests and loses messages in production.

That second point is also why there is no behavioral test here. One was written,
asserting the send had reached the transport before the call returned; it passes
identically under both mappings and was deleted rather than kept as decoration.
The agent-type assertions are what actually distinguish them, and both were
verified to fail with the NativeAck arm reverted to BufferedSendingAgent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit 2ea4266 into main Aug 24, 2026
39 checks passed
jeremydmiller added a commit that referenced this pull request Aug 24, 2026
Sending anything to an Azure Service Bus endpoint that listens with native acks
threw "This sender has not been registered." on every batch.

GH-3709 (#4061) remapped EndpointMode.NativeAck to InlineSendingAgent on the
sending side. InlineSendingAgent is not an ISenderCallback, so
EndpointCollection.CreateSendingAgent skips RegisterCallback -- but this
transport still chose a BatchedSender, because its gate asked only about
EndpointMode.Inline. Agent says inline, sender says batched, and BatchedSender
refuses to send without a callback.

This is the same mismatch the GH-3826 comment a few lines above already
describes for TenantedSender, reached through the mode instead of tenancy, so
the fix is the same: NativeAck takes the inline sender.

Caught by native_ack_native_scheduling_4049.a_scheduled_retry_goes_back_through_the_broker,
which is the first ASB test to both listen with native acks and publish to the
same queue.

Note the same gate exists in RedisStreamEndpoint.CreateSender, and Redis
accepted NativeAck in #4056, so Redis is exposed too -- raised separately
rather than fixed here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller added a commit that referenced this pull request Aug 24, 2026
…#4061

#4061 remapped EndpointMode.NativeAck from BufferedSendingAgent to
InlineSendingAgent, so the trigger that originally surfaced this race is gone.
The race itself is unchanged and still reachable through any BufferedInMemory
endpoint, but the concurrency stress test that caught it stopped reproducing
reliably -- it passed 8/8 with the fix reverted after the merge.

Adds a deterministic companion that gates the metrics hook on the recycle flag
(Envelope.Reset clears FromPool) instead of racing it, so the red baseline holds
regardless of machine load: 5/5 failures with the fix reverted, and the stress
test stays as the real-tracker smoke check it always was.

Re-frames the comments: found through NativeAck, remaining exposure is
BufferedInMemory.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller added a commit that referenced this pull request Aug 24, 2026
…stration

Fix live regression: NativeAck endpoints get an unregistered BatchedSender
jeremydmiller added a commit that referenced this pull request Aug 24, 2026
One conflict, and a welcome one: this branch and #4070 independently fixed the
same GH-4061 sender-registration regression on the ASB gates. This branch used
`Mode is EndpointMode.Inline or EndpointMode.NativeAck` inline at both call
sites; #4070 introduced `Endpoint.SendsInline` and applied it across all seven
transports.

Kept main's central form. The explanation this branch carried in a comment at
each site now lives in the xml-docs on SendsInline itself, which was the point
of centralising it -- the per-transport version is exactly what invites the bug
again the next time a mode is added.

Verified after resolution: dotnet build wolverine.slnx -c Release -f net9.0 clean.
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 25, 2026
…tered BatchedSender

Live regression on main, introduced by JasperFx#4061. Sending anything to a NativeAck
endpoint on Redis fails outright with:

    System.InvalidOperationException: This sender has not been registered.

Mechanism, end to end:

* JasperFx#4061 remapped EndpointMode.NativeAck from BufferedSendingAgent to
  InlineSendingAgent, to close the interceptor loss window.
* InlineSendingAgent is `ISendingAgent, IDisposable` -- deliberately NOT an
  ISenderCallback -- so the registration in EndpointCollection.CreateSendingAgent
  (`sender is ISenderRequiresCallback && agent is ISenderCallback`) is skipped.
* Every transport that uses BatchedSender chooses it by asking
  `Mode == EndpointMode.Inline`. NativeAck is not Inline, so those transports
  still build a BatchedSender.
* BatchedSender throws on every send, ping and failure path when `_callback` is
  null.

Redis adopted NativeAck in JasperFx#4056, so this is broken on main today. RabbitMQ and
Pulsar use their own senders rather than BatchedSender, which is exactly why CI
stayed green: the only NativeAck integration tests that exist run on those two
transports.

The fix is one property rather than nine copies of a boolean, because the
failure is silent and total, and because the next mode that sends inline should
not require finding all nine sites again:

    public bool SendsInline => Mode is EndpointMode.Inline or EndpointMode.NativeAck;

Applied to the sender gates in SNS, SQS, Azure Service Bus (topic and queue),
Kafka, MQTT and Redis. GCP Pub/Sub is deliberately untouched: it has not adopted
NativeAck (supportsNativeAck is false there), so it cannot reach this today, and
that file is being actively edited under JasperFx#4065/JasperFx#4066. The Pub/Sub gate should
move to SendsInline whenever JasperFx#4052 adopts the mode.

Red-baselined: with the Redis gate reverted to `Mode == EndpointMode.Inline`,
three of the six Redis native_ack_mode tests fail with the exact
"This sender has not been registered." exception; with SendsInline, 6/6 pass.

Verified: pinned dotnet build wolverine.slnx -c Release -f net9.0 clean,
CoreTests 2581 / 0 failed, Redis native_ack_mode 6/6.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 25, 2026
… NativeAck 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.

JasperFx#4061 (JasperFxGH-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. JasperFx#4056 (JasperFxGH-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 JasperFx#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 JasperFx#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant