Skip to content

Unify subscription channel overflow defaults#1181

Merged
mtmk merged 1 commit into
release/3.0from
unify-sub-overflow-defaults
Jun 12, 2026
Merged

Unify subscription channel overflow defaults#1181
mtmk merged 1 commit into
release/3.0from
unify-sub-overflow-defaults

Conversation

@mtmk

@mtmk mtmk commented Jun 8, 2026

Copy link
Copy Markdown
Member

All entry points now share one subscriber-channel overflow policy. NatsClient and the DI builder previously forced Wait while NatsConnection used DropNewest; Wait can stall the socket read loop and get the client disconnected as a slow consumer. They now inherit the NatsOpts default DropNewest, and the default channel capacity goes from 1024 to 16384 so overflow stays rare. Breaking for NatsClient and DI users under sustained overload: newest messages are dropped (surfaced via MessageDropped) instead of blocking.

Fixes #1161

NatsClient and the DI builder forced SubPendingChannelFullMode to Wait,
while NatsConnection defaulted to DropNewest, so overflow behavior
differed by entry point. Wait applies backpressure to the socket read
loop and risks the server disconnecting the client as a slow consumer.

Make all entry points inherit the NatsOpts default DropNewest and raise
SubPendingChannelCapacity from 1024 to 16384 so overflow stays rare.
Drop the now-redundant pending parameter from the NatsClient(NatsOpts)
constructor.

Breaking for NatsClient and DI users under sustained overload: messages
are dropped (surfaced via MessageDropped) instead of blocking.
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 2m 35s —— View job


PR Review: Unify subscription channel overflow defaults

Tasks:

  • Gather PR context
  • Read changed files and surrounding code
  • Analyze correctness, security, and performance
  • Post inline comments for specific issues
  • Post review summary

Review

The motivation is sound: BoundedChannelFullMode.Wait on the socket-read path blocks the reader goroutine and can cause the server to disconnect the client as a slow consumer — exactly the failure mode it was meant to prevent. Unifying to DropNewest with a much larger default buffer is the right trade-off for an at-most-once messaging system.

Correctness

No bugs found. The mechanics are correct:

  • NatsConnection (NatsConnection.cs:106-112) builds _defaultSubscriptionChannelOpts directly from NatsOpts, so it always inherits whatever SubPendingChannelFullMode is set on the opts.
  • NatsClient now passes opts through unchanged (after the serializer-registry override) — the per-connection channel opts are therefore set correctly.
  • NatsBuilder previously force-set Wait via a Configure action; that line is now removed, so the builder inherits NatsOpts.Default (i.e., DropNewest), and WithSubPendingChannelFullMode remains available to opt in to Wait explicitly.
  • The MessageDropped event path (NatsConnection.cs:292-295) is unchanged and fires correctly when the channel is full.

Breaking change (expected for v3.0)

Removing the pending parameter from NatsClient(NatsOpts opts, BoundedChannelFullMode pending = ...) is a source-breaking change: any callers that passed pending: BoundedChannelFullMode.Wait explicitly will get a compile error. Because this targets release/3.0, that's appropriate — but worth calling out in release notes or a migration guide. The replacement pattern (new NatsOpts { SubPendingChannelFullMode = ... }) is straightforward and tested in Client_opts_pending_respected.

Capacity bump: 1 024 → 16 384 (16×)

BoundedChannel<T> in .NET doesn't pre-allocate the full ring buffer; it grows as messages arrive. So the increase doesn't inflate baseline memory. However, under a genuine burst a subscription can now hold up to 16× more messages before dropping, which is the intended goal. Users with thousands of JetStream consumers should be aware the per-subscription ceiling is now 16 384 messages; at a few hundred bytes each this can be several hundred MB of live buffered data in pathological cases. A sentence in the docs' "Tuning Channel Capacity" section (already updated in slow-consumers.md) covers this adequately.

Doc example (IntroPage.cs:84)

SubPendingChannelFullMode = BoundedChannelFullMode.DropNewest,

This is redundant (it matches the new default), but it's intentionally illustrative — showing that the field is the correct place to set the policy. Fine as-is.

Test coverage

All assertions updated correctly. The renamed test Client_opts_pending_respected cleanly verifies that an explicit user-supplied value in NatsOpts survives the NatsClient constructor unchanged.


Overall: Clean and correct. No functional issues. The PR achieves consistent behavior across all entry points, eliminates a latent footgun, and documents the behavioral change clearly.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

@mtmk
mtmk requested a review from scottf June 9, 2026 10:03

@scottf scottf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtmk
mtmk merged commit 41fa0d3 into release/3.0 Jun 12, 2026
32 of 33 checks passed
@mtmk
mtmk deleted the unify-sub-overflow-defaults branch June 12, 2026 11:41
@mtmk mtmk linked an issue Jun 15, 2026 that may be closed by this pull request
@mtmk mtmk mentioned this pull request Jun 15, 2026
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.

Unify sub channel overflow defaults, raise capacity to 16K

2 participants