GH-3709: native-ack global partitioning cluster + failover tests and docs - #4069
Merged
Conversation
…itioning Finishes #3709 behind the opt-in landed in #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>
…rtitioning-tests
…#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>
…rtitioning-tests
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.
Closes #3709.
Finishes the issue behind the opt-in that landed in #4041 (items 1, 2, 3, 6). Merged with
origin/mainat2ea4266b6, so #4055, #4056, #4061 and #4043 are all in — no conflicts.Scope: item 5, the tests, the docs. Two product bugs the new tests exposed are fixed here as well, both with red-baselined regression coverage.
This is the most important thing in the PR and it contradicts one of #3709's acceptance criteria, so it needs a decision rather than an inference from a docs warning.
A truly storage-free multi-node cluster cannot use Wolverine's dynamic one-consumer-per-slot assignment. The chain:
ExclusiveListenerFamily, which runs underNodeAgentController.WolverineRuntime.startAgentsAsyncreturns early whenStorage is NullMessageStore— a storeless host never builds aNodeAgentControllerat all, so no exclusive listener agent is ever created or assigned.DurabilityMode.Solo(underBalanced,Endpoint.ShouldAutoStartAsListenerreturns true only forCompetingConsumers, so the exclusive slots would never start listening at all).Solo,ShouldAutoStartAsListenerreturns true unconditionally — every node starts every listener. Three storeless nodes means three competing consumers on every slot, and the guarantee is gone.Empirically confirmed: the first run of the multi-node test, before slot ownership was applied, logged 3 listeners started on each of the 6 slot queues and the ledger reported 37 intra-group concurrency violations.
So the honest statement is:
DurabilityMode.Balanced). The messages still never touch it — the slots stayEndpointMode.NativeAck, so there is no inbox insert, no mark-handled, and no per-message database work. It is a node-registry, not a message store, in this mode.I did not try to close this gap — storeless cluster coordination is a much larger piece of work than #3709. The tests are split accordingly (see below), and the docs carry a
::: warningsaying the same thing. If the acceptance criterion is meant to cover the clustered case, it needs its own issue.Item 5 — verified free, nothing added
The issue asks to skip inbox recovery and the
EnqueueDirectlyAsyncbridge branch for native-ack slots. Both are already no-ops, so this PR deliberately adds no guards for either.ListeningAgent.startInboxRecoveryIfNecessaryopens withif (Endpoint.Mode != EndpointMode.Durable) return;. A native-ack slot isEndpointMode.NativeAck, so the loop is never built. (A second guard,_runtime.Storage is NullMessageStore, covers the storage-free case independently.)EnqueueDirectlyAsyncbridge branch is_receiver is GlobalPartitionedReceiverBridge. The bridge is only ever constructed whenEndpoint.GlobalPartitionLocalQueueUri != null, and a native-ack topology never stamps that property — leaving it null is the opt-out, exactly as the re-verification comment on the issue predicted. Native-ack slots land in theNativeAckReceiverbranch added by ListeningAgent.EnqueueDirectlyAsync has no branch for a native-ack receiver #4011 instead.The guarantee, stated exactly
Used verbatim in the docs and in the test names:
Ordering is per-slot best effort, not per-group guaranteed — redelivery or requeue may reorder. And the existing caveat is restated unchanged: the ordering unit is the slot, not the group, so two group ids that hash to the same slot serialize against each other.
Tests
New transport-agnostic harness in the
ShardedProcessing.cs(GH-3467) style:Wolverine.ComplianceTests/Partitioning/NativeAckPartitionedProcessing.cs. A transport supplies only its ownUseSharded*()call plus the cluster shape; the message type, handler, cluster-wide group ledger, publishing burst and assertions live in the harness. The ledger detects overlap by claim-and-release on the group id, with a compare-and-remove on the claim token so a losing claimant cannot evict the rightful holder and cascade phantom violations.native_ack_global_partitioning_cluster(RabbitMQ, storage-free)no_two_messages_of_a_group_execute_concurrently_across_a_storage_free_cluster— 3 hosts with no message store at all, 6 slots split 2/2/2, 24 groups × 4 messages. Asserts no intra-group concurrency cluster-wide, at-least-once completeness, groups never straddle slots, every slot used, every node did work, and that slot ownership still held at the end of the run rather than only at the start.sends_go_through_the_broker_even_when_this_node_owns_every_slot— the routing test. Every executed envelope'sDestinationisrabbitmq://queue/...; there is nolocal://global-*destination anywhere, so the local shortcut really is disabled.Ownership is applied by stopping the unowned slot listeners after startup — which is precisely the state
ExclusiveListenerAgentleaves behind on a node not assigned a slot. ClearingIsListenerin configuration does not work: everyListenerConfigurationcarries a delayede.IsListener = truethat runs duringEndpoint.Compile(), after endpoint policies.native_ack_global_partitioning_failover(RabbitMQ + Postgres as the node registry only)no_intra_group_concurrency_when_a_slot_owner_leaves_mid_stream— the test the issue names. ~1,230 messages streaming, a non-leader slot owner stopped mid-stream; asserts reassignment lands each orphaned slot on exactly one survivor, processing continues after the handoff, a survivor actually drained the reassigned slots, no intra-group concurrency across the handoff, and at-least-once completeness.no_intra_group_concurrency_when_a_live_slot_handoff_moves_the_slot— the sharper form: pin a slot to a different node while both are alive, so nothing is helped along by a dying process and the only thing between the new owner's first pull and the old owner's in-flight handler is the drain.Product bug 1 —
LatchReceiver()never latched aNativeAckReceiverFound by the live-handoff test, which failed before the fix.
ListeningAgent.LatchReceiver()was an if/else chain namingDurableReceiver,BufferedReceiverandInlineReceiver. GH-3708 addedNativeAckReceiverand the chain silently skipped it — no compile error, no exception. The consequence:NativeAckReceiver.DrainAsyncdecides whether to wait on_latched, so an unlatched receiver returned immediately instead of waiting for in-flight handlers.StopAndDrainCoreAsyncthen disposed the listener underneath running work, the still-unsettled deliveries were requeued, and on an exclusive listener handoff the incoming node re-ran them concurrently with the outgoing node — exactly the intra-group concurrency the mode exists to prevent.#3709 asserted "exclusive-listener handoff drains in-flight work before releasing" as fact. It was false under this mode until this fix.
Fix: an
ILatchedReceiverinterface implemented by all four receivers, andLatchReceiver()becomes a single type test, so the next receiver type cannot silently drop out.Red baseline:
no_intra_group_concurrency_when_a_live_slot_handoff_moves_the_slotfailed with 4 violations on the moved slot before the fix — each pairing the same(group, sequence)on the old and new owner — and passes after. Duplicate count on that test went from 1252-handled-for-1216-published to 1232/1232.Regression coverage:
latched_receiver_contract_3709(reflection guard: anyIReceiverwith aLatch()method must declareILatchedReceiver, plus a[Theory]over the four known receivers so the guard is not vacuous) andnative_ack_receiver.draining_a_latched_receiver_waits_for_the_in_flight_handler. Verified the guard fails whenILatchedReceiveris removed fromNativeAckReceiver.Product bug 2 — pooled outgoing envelope recycled out from under the metrics read
Pre-existing, not introduced by #3709. Reproduced as an intermittent
NullReferenceExceptioninEnvelope.ToMetricsHeaders()—Destinationnon-null at its own guard, null one line later atDestination.ToString()— roughly 1 run in 4 of the new cluster suite.Mechanism:
BufferedSendingAgentis in the poolable set forWolverineRuntime.AcquireOutgoingEnvelope(wolverine#2955), but itsstoreAndForwardAsynconly posts the envelope to an in-memory block. The block's consumer then sends it, succeeds, and returns it to the pool —Envelope.Reset(), which nullsDestinationandMessageType— while the producing frame is still insideSendingAgent.StoreAndForwardAsyncreading that same envelope for_messageLogger.Sent().Attribution, corrected for #4061. I found this through
EndpointMode.NativeAck, which mapped toBufferedSendingAgentat the time. #4061 has since remapped NativeAck ontoInlineSendingAgent, so that trigger is gone. The race is unchanged and was never native-ack specific: the remaining exposure is anyBufferedInMemoryendpoint under concurrent publishing, and that is how both tests now drive it. Comments and test docs have been re-framed accordingly.Fix: in
SendingAgent.StoreAndForwardAsyncandEnqueueOutgoingAsync, do the metrics read before the handoff when the envelope is pooled. Non-pooled envelopes — every durable send, and anything published inside a tracking session — keep the original ordering, so a store that throws still reports no send.InlineSendingAgentwas already correct: it awaits the real send and calls_messageLogger.SentbeforetryReturnToPool.I deliberately did not take the other candidate fix, removing
BufferedSendingAgentfromIsPoolableOutgoingAgent. It works, butshared_memory_envelope_pooling_3015(GH-3015) explicitly asserts that aBufferedSendingAgenthands out a pooled envelope, so that set is intentional and load bearing.The stress test stopped reproducing after the merge, so the red baseline is now deterministic
The original guard was a 2,000-message concurrency stress test through the real
WolverineRuntimetracker. On the pre-merge tree it failed in 113ms with the fix reverted; after merging main it passed 8/8 with the fix reverted — the product files on that path are byte-identical, so this is machine load, not a behaviour change (the earlier failing runs were on a box busy with RabbitMQ suites).A probabilistic red baseline is not a red baseline, so
a_pooled_envelope_is_read_for_metrics_before_it_is_handed_to_the_sending_blockwas added. It gates the metrics hook on the recycle flag rather than racing it:Envelope.Reset()clearsFromPool, so a stand-in tracker spins insideSent()until the envelope has been recycled or a 2s deadline passes. With the fix nothing has been posted whenSent()runs, so no recycle is possible, the spin times out and the envelope reads intact. Without the fixSent()runs after the post and the spin waits for exactly the recycle it is racing, soDestinationis reliably null.Verified on the merged tree: 5/5 deterministic failures with the fix reverted, 2/2 pass with it. The stress test stays as a real-tracker smoke check, now documented as probabilistic rather than as the guard.
Docs
New
### Native Ack Global Partitioningsection indocs/guide/messaging/partitioning.md, placed after the RabbitMQ example and cross-linked both ways with the existing "Partitioned processing without the database" tip so the per-listener and cluster-wide stories read as one. Covers the config sample, the guarantee statement, a durable-vs-native-ack trade table, when to choose it (flood tolerance, no DB load, at-least-once plus best-effort order), the failover caveat, and a::: warningfor the storage-free clustering limitation above. Also amends two now-stale lines: the single-node local-shortcut tip, and the Postgres/SQL Server note that said the topology "forcesEndpointMode.Durable".Verification (all re-run after the merge)
dotnet build wolverine.slnx -c Release -f net9.0— clean, 0 warnings, 0 errors.CoreTests(Release, net9.0) — 2587 tests, 0 failed.native_ack_mode,global_partitioned_sharded_processing,exclusive_listeners,multi_node_exclusive_listener_failover,durable_compliance— 31 tests, 0 failed.Red baselines
Every assertion that claims to prove an invariant was verified to fail when the invariant is broken:
ILatchedReceiverfix.ILatchedReceiverremoved fromNativeAckReceiver— both guard tests fail, naming the offender.🤖 Generated with Claude Code