Skip to content

GH-3709: ProcessInParallelWithNativeAcks() on the global partitioned topology - #4041

Merged
jeremydmiller merged 2 commits into
mainfrom
gh-3709/native-ack-topology
Aug 23, 2026
Merged

GH-3709: ProcessInParallelWithNativeAcks() on the global partitioned topology#4041
jeremydmiller merged 2 commits into
mainfrom
gh-3709/native-ack-topology

Conversation

@jeremydmiller

@jeremydmiller jeremydmiller commented Aug 23, 2026

Copy link
Copy Markdown
Member

Toward #3709 — the topology half. This is a partial implementation and #3709 must stay open after it merges; see Scope below.

Deliberately no linking keyword anywhere in this description. An earlier draft phrased the scope note as "does not ​close" immediately followed by the issue reference; GitHub's parser matched that as a closing keyword — negation and all — and linked this PR to auto-close the issue on merge. Please do not add one.

Partitioned clustering with no database: each slot listener settles its own broker deliveries and shards into sequential lanes by group id in memory, instead of bridging into a companion durable local queue and paying an inbox insert plus a mark-as-handled per message.

opts.MessagePartitioning.GlobalPartitioned(topology =>
{
    topology.ByMessage<WebhookEvent>(x => x.EntityId);
    topology.UseShardedRabbitQueues("webhooks", 5);
    topology.ProcessInParallelWithNativeAcks(); // NEW
});

Why this needed a new method rather than Mode(EndpointMode.NativeAck)

The default topology hands each external listener to GlobalPartitionedReceiverBridge, which forwards into a companion local queue where the partitioned execution actually happens. That is exactly why #4032 made Mode() reject NativeAck: a local queue has no broker delivery to settle, so the ack would have nothing to ack against.

Opting in removes the companion topology and the bridge, so the premise no longer holds. That guard is replaced, not deletedMode(NativeAck) still throws, because it would set the mode without removing the bridge, which is the silent-fallthrough case #4032 existed to prevent. It now names the call that works. The existing a_global_partitioned_topology_rejects_native_ack test passes unchanged.

What changed

  • ProcessInParallelWithNativeAcks() sets NativeAck on every external slot, drops any companion local topology, and marks the topology so the rest of the pipeline can see it.
  • LocalQueues() and ProcessInParallelWithNativeAcks() together are now a configuration error rather than a silently-ignored pair. Calling them in either order converges on the native-ack shape, matching how Mode() already behaves.
  • AssertValidity() drops only the two rules that exist to line the companion queues up with the bridge. The subscription and external-topology rules still apply, and there is a test for that.
  • GlobalPartitionedRoute skips the local shortcut. In this mode the broker delivery is the durability story, so handing a message straight to a local queue would drop it on a crash between send and handling.

Two steps from the issue that turned out to be no-ops

Worth recording, because the issue asks for both and neither is needed:

  • The bridge skip needs no ListeningAgent change. The bridge is wired off Endpoint.GlobalPartitionLocalQueueUri, so not tagging it is the opt-out. There is no separate branch to add.
  • The inbox-recovery skip is already free. startInboxRecoveryIfNecessary opens with if (Endpoint.Mode != EndpointMode.Durable) return;, so a native-ack slot already returns immediately. Verified rather than assumed.

That leaves EnqueueDirectlyAsync as the only ListeningAgent work in #3709's step 5, and #4040 is already in that method for #4011 — so it belongs there, not here. This PR touches no file that #4040 touches.

A transport that has not opted into NativeAck fails fast through the existing Endpoint.Mode setter, naming the endpoint type; no new validation was needed.

Scope — what is deliberately not here

The end-to-end, multi-node and slot-failover tests. No transport overrides supportsNativeAck on main yet — RabbitMQ's opt-in is in #4040 — so those cannot be written against anything real today. The NativeAckCapableEndpoint stand-in here mirrors the one #3708 already uses for the same reason.

#3709 should stay open until that coverage exists, in particular the failover test the issue calls out: killing the node owning a slot mid-handler and asserting the intra-group concurrency invariant holds across reassignment. That is the one genuine cross-node hazard in this design and nothing here proves it.

Verification

Result
dotnet build wolverine.slnx -c Release -f net9.0 clean, 0 warnings
CoreTests 2544, 0 failed, 2 skipped — 11 new

The local-shortcut test is the one worth a reviewer's eye: it passes a null runtime, so it only survives if the shortcut is skipped outright rather than merely taking a different path.

🤖 Generated with Claude Code

…topology

Partitioned clustering with no database: each slot listener settles its own broker
deliveries and shards into sequential lanes by group id in memory, instead of bridging
into a companion durable local queue and paying an inbox insert plus mark-handled per
message.

The default topology hands each external listener to GlobalPartitionedReceiverBridge,
which forwards into a companion local queue where the partitioned execution actually
happens -- which is exactly why GH-3708 made Mode() reject NativeAck: a local queue has no
broker delivery to settle. Opting in removes the companion topology and the bridge, so the
premise no longer holds and the mode becomes reachable.

* ProcessInParallelWithNativeAcks() sets NativeAck on every external slot, drops any
  companion local topology, and marks the topology so the rest of the pipeline can see it.
* Mode(NativeAck) still throws -- it would set the mode WITHOUT removing the bridge -- but
  now names the call that works. GH-3708's guard is replaced, not deleted.
* LocalQueues() and ProcessInParallelWithNativeAcks() together are a configuration error
  rather than a silently-ignored pair; calling them in either order converges on the
  native-ack shape, matching how Mode() already behaves.
* AssertValidity() drops only the two rules that exist to line the companion queues up with
  the bridge. The subscription and external-topology rules still apply.
* GlobalPartitionedRoute skips the local shortcut. In this mode the broker delivery IS the
  durability story, so handing the message straight to a local queue would drop it on a
  crash between send and handling.

Nothing in ListeningAgent had to change. The bridge is wired off
Endpoint.GlobalPartitionLocalQueueUri, so leaving it null is the opt-out; and
startInboxRecoveryIfNecessary already returns for any non-Durable endpoint. A transport
that has not opted into NativeAck fails fast through the existing Endpoint.Mode setter,
naming the endpoint type.

Scope: this is the topology half of GH-3709. The end-to-end and slot-failover tests need a
transport that actually supports the mode -- no transport overrides supportsNativeAck on
main yet -- so they follow once GH-3708's receiver lands. The NativeAckCapableEndpoint
stand-in here mirrors the one GH-3708 already uses for the same reason.

wolverine.slnx -c Release -f net9.0 clean. CoreTests 2544 (0 failed, 2 skipped), including
11 new tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller added a commit that referenced this pull request Aug 23, 2026
a_global_partitioned_topology_rejects_native_ack asserted the message contained
"companion local queue". That phrase appears in the Inline rejection at
GlobalPartitionedMessageTopology.cs:55 as well, so the assertion could not tell
the two branches apart: it would have passed even if NativeAck fell through to
the Inline guard and the mode-specific rejection had been lost entirely.

Verified by simulating exactly that -- making the NativeAck branch throw the
Inline message. The old assertion passes; the new one fails.

Now asserts the mode name (which the Inline message cannot contain) and that
the two branches produce different messages. Both are semantic rather than
prose, so a rewording of either message -- as #4041 is doing to this one right
now -- cannot silently hollow the test out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not shared prose

"companion local queue" appears in the Inline rejection too, so asserting on it would
still pass if NativeAck fell through to the Inline guard and the mode-specific message
were lost. Assert ParamName, the mode name, and that the two messages differ instead.
Shouldly's ShouldContain is case-insensitive, so ShouldNotContain("Inline") would trip
on any lowercase "inline" in the prose -- inequality is the robust form.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit 393cf67 into main Aug 23, 2026
39 checks passed
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 24, 2026
…terceptor loss window

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

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

JasperFxGH-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. JasperFxGH-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>
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 25, 2026
…bal partitioning

Finishes JasperFx#3709 behind the opt-in landed in JasperFx#4041: item 5, the multi-node and
failover test suites, the routing test, and the documentation.

Item 5 was verified free, not patched. Two product bugs the new tests exposed
are fixed here, both with red-baselined regression coverage.

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