GH-3972: coalesce outgoing SignalR messages into one envelope per destination - #3979
Merged
Conversation
…tination
Wolverine.SignalR had no batching, buffering or coalescing of any kind, and Wolverine
offers no sender-side hook for it, so an application that wanted it had to route its
outbound messages through a LOCAL QUEUE to get them batched. That detour is the direct
cause of a three-issue bug family: a local queue that carries a handler's own output is a
cascade target for that handler, so forwarding with SendAsync re-sends onto the very queue
the message was read from, and the queue fills until back pressure blocks the producer.
A sending-agent buffer creates none of that, because nothing round-trips a queue -- there
is no queue to re-enter. It also sits AFTER the outbox, unlike the application-level
ConcurrentQueue-plus-timer accumulator this replaces, which relayed before the commit and
therefore carried a standing "never use it for a message that tells the client to go read
something" caveat.
opts.PublishAllMessages().ToSignalR()
.CoalesceOutgoing(o =>
{
o.FlushInterval = 100.Milliseconds();
o.MaxBatchSize = 200;
});
The four design points from the issue, and what each turned into:
- ENVELOPE CONTRACT. A batch carries the individual CloudEvents documents verbatim rather
than flattening them into bare payloads. The CloudEvents envelope is per-outer-message,
so flattening would lose the message type of every item; keeping whole documents gives
each item its own type for free.
- KEYED BY DESTINATION. Buffers are keyed by locator AND operation. An application that
only ever broadcasts would get away with one global buffer; the transport must not,
because coalescing a message bound for connection A with one bound for connection B
delivers each to both. There is a test for exactly that.
- ORDERING is arrival order within a batch, asserted end to end.
- DRAIN ON SHUTDOWN, on both IListener.StopAsync and DisposeAsync. The drain deliberately
runs BEFORE the disposal latch, or anything already buffered would be lost.
Batches go out on a dedicated ReceiveCoalescedMessages operation rather than wrapped into
ReceiveMessage. A client that does not know about coalescing then simply never receives
them, instead of receiving something on ReceiveMessage that it tries to read as a single
CloudEvents document and fails on for every message -- an obvious "nothing arrived" is a
much better failure than a silent per-message one. A batch that holds a single message is
sent on the normal operation anyway, so the trickle case needs no client change at all.
Wolverine's own .NET SignalR client registers the unwrap unconditionally: the SERVER decides
whether to coalesce, and a client that only listened when locally configured to would
silently drop every batch the moment the server turned coalescing on.
Note the issue refers to a TS client under Wolverine.SignalR/Client needing a matching
unwrap. There is no TS client in this repository -- that directory holds the .NET client,
which is updated here. The docs carry the browser-side snippet instead.
Full Wolverine.SignalR.Tests suite: 37/37.
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 #3972.
Wolverine.SignalRhad no batching, buffering or coalescing of any kind, and Wolverine offers no sender-side hook for it — so an application that wanted it had to route outbound messages through a local queue to get them batched. That detour is the direct cause of the three-issue bug family in the issue: a local queue carrying a handler's own output is a cascade target for that handler, so forwarding withSendAsyncre-sends onto the very queue the message was read from, and the queue fills until back pressure blocks the producer.A sending-agent buffer creates none of that, because nothing round-trips a queue — there is no queue to re-enter. It also sits after the outbox, unlike the
ConcurrentQueue-plus-timer accumulator it replaces, which relayed before the commit and carried a standing "never use it for a message that tells the client to go and read something" caveat.The four design points, and what each became
1. Envelope contract. A batch carries the individual CloudEvents documents verbatim rather than flattening them into bare payloads. The CloudEvents envelope is per-outer-message, so flattening would lose the message type of every item; keeping whole documents gives each item its own type for free.
2. Keyed by destination. Buffers key on locator and operation. An application that only ever broadcasts would get away with one global buffer; the transport must not, because coalescing a message bound for connection A with one bound for connection B delivers each to both. There's a test that interleaves two connections at
MaxBatchSize = 2and asserts they never mix.3. Ordering is arrival order within a batch, asserted end-to-end.
4. Drain on shutdown, on both
IListener.StopAsyncandDisposeAsync. The drain deliberately runs before the disposal latch, or anything already buffered would be lost.A dedicated client operation, not a wrapped one
Batches go out on
ReceiveCoalescedMessagesrather than wrapped intoReceiveMessage. A client that doesn't know about coalescing then simply never receives them, instead of receiving something onReceiveMessagethat it tries to read as a single CloudEvents document and fails on for every message. An obvious "nothing arrived" is a much better failure than a silent per-message one.A batch that holds a single message is sent on the normal operation anyway, so the common trickle case needs no client change at all.
Wolverine's own .NET SignalR client registers the unwrap unconditionally — the server decides whether to coalesce, and a client that only listened when locally configured to would silently drop every batch the moment the server turned it on.
One correction to the issue
Design point 5 says "the TS client under
Wolverine.SignalR/Clientneeds the matching unwrap." There is no TS client in this repository — that directory holds the .NET client, which is updated here. The only TypeScript/JS under the repo is Microsoft's vendoredsignalr.jsinside a sample app. The docs carry a browser-side snippet showing the unwrap instead, so the guidance still lands for browser consumers.Tests
Nine, in two classes:
Full
Wolverine.SignalR.Tests: 37/37. Fullwolverine.slnxbuilds clean pinned tonet9.0.The test double is hand-written rather than substituted, deliberately:
SendAsyncis an extension overIClientProxy.SendCoreAsync, and the indirection makes a mocked setup easy to get subtly wrong in a way that silently records nothing — which is exactly what happened on the first attempt.Out of scope
Domain-level coalescing (accumulating N domain events into one summary message) stays in application code, as the issue specifies. This is delivery coalescing only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JG8Un6iNeyXECKJk3jo5uC