GH-3973: say out loud when a batch cannot be sequenced against its unbatched siblings - #3978
Merged
Merged
Conversation
…batched siblings Follow-up to GH-3867, taking the second of the two acceptance paths the issue offers. On a host with BatchMessagesOf<T> and NO partitioned topology there are two independent execution paths writing the same entity: the assembled batch on its own local queue, and the unbatched sibling handlers inside the listener receiver's own execution block. GH-3867 already resolves this when a topology exists -- resolveBatchExecutionTopologies points the batch at the same slots so every writer for one group id really is a single writer. Without one, findPartitionedExecutionSlots returns null and nothing sequences them. Sequential() on the batch queue does not close it: that serializes the batch against itself and against nothing else. The asymmetry is what makes this worth a startup message rather than documentation. With a GlobalPartitioned topology the configuration is safe; without one -- embedded hosts, single-node deployments, most test fixtures -- the same code has two concurrent writers to one event stream, surfacing as intermittent stream-version collisions under load. So a defect is unreachable in the configuration the tests use and reachable in the one that ships, or the reverse. The silent version is the bug. Startup now logs a warning naming the message type, the queue the batch landed on, the fix (put the element type into a partitioned topology) and the wrong fix it must not be confused with (Sequential). opts.AssertBatchExecutionIsSequenced() escalates it to a startup throw. Deliberately scoped to MultipleHandlerBehavior.Separated. Under the default Classic behaviour the direct handler wins and the batch never runs at all, so there is only one writer -- that shape is already reported by warnOrAssertBatchHandlerConflicts and must not be double-reported. A new opt-in rather than reusing AssertNoBatchHandlerConflicts: that flag is documented as having no effect under Separated, and widening it would start throwing for someone who set it. Option 1 from the issue -- making the listener's ShardedExecutionBlock an addressable lane a batch can join -- is not attempted here. The issue explicitly accepts this option as closing it, and the block is not a queue and cannot be enqueued to without restructuring the receivers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JG8Un6iNeyXECKJk3jo5uC
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 #3973. Follow-up to #3867, taking the second of the two acceptance paths the issue offers.
The gap
On a host with
BatchMessagesOf<T>and no partitioned topology there are two independent execution paths writing the same entity:#3867 already resolves this when a topology exists —
resolveBatchExecutionTopologiespoints the batch at the same slots, so every writer for one group id really is a single writer. Without one,findPartitionedExecutionSlotsreturnsnulland nothing sequences them. Its own closing comment says so:Sequential()on the batch queue does not close it: that serializes the batch against itself, and against nothing else.Why a startup message and not documentation
The asymmetry is the hazard. With a
GlobalPartitionedtopology the configuration is safe; without one — embedded hosts, single-node deployments, most test fixtures — the same code has two concurrent writers to one event stream, surfacing as intermittent stream-version collisions under load. A defect is therefore unreachable in the configuration the tests use and reachable in the one that ships, or the reverse.The docs already described this limitation in prose, which is exactly the state the issue is complaining about ("currently documents it in prose rather than enforcing it"). Startup now says it:
It names the fix and heads off the wrong fix, both asserted in tests.
opts.AssertBatchExecutionIsSequenced()escalates it to a startup throw.Scoping decisions
MultipleHandlerBehavior.Separated. Under the default Classic behaviour the direct handler wins and the batch never runs at all, so there is only one writer — that shape is already reported bywarnOrAssertBatchHandlerConflictsand must not be double-reported.AssertNoBatchHandlerConflicts. That flag is documented as having no effect underSeparated; widening it would start throwing for someone who already set it.ShardedExecutionBlockan addressable lane a batch can join means restructuring the receivers — the block is not a queue and cannot be enqueued to. The issue explicitly accepts option 2 as closing it, and says "Option 2 is a perfectly good answer if option 1 is structurally awkward."Tests
Five in
unsequenced_batch_execution_validation— the throw and its message content, silence when a partitioned topology does sequence them, silence with no unbatched sibling, silence under Classic, and warn-not-throw by default.Batching regression: 123/123.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JG8Un6iNeyXECKJk3jo5uC