fix(streaming): bound NATS producer selection - #11287
Merged
ReubenBond merged 2 commits intoSep 17, 2026
Merged
ReubenBond merged 2 commits into
ReubenBond merged 2 commits into
Conversation
(cherry picked from commit 210e0ba)
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Fixes NATS producer selection for int.MinValue hash codes using unsigned modulo arithmetic.
Changes:
- Adds bounded producer-index calculation.
- Routes publishing through the helper.
- Adds boundary regression tests.
| File | Description |
|---|---|
test/Extensions/Orleans.Streaming.NATS.Tests/NatsOptionsTests.cs |
Covers hash and producer-count boundary cases. |
src/Orleans.Streaming.NATS/Providers/NatsConnectionManager.cs |
Uses safe unsigned-modulo producer selection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
Contributor
Code coverage
Report-only conclusion: current-main baseline stale. The newest successful coverage run tested 68f1f47, not current main 2e40fa8. Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
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.
NATS producer selection currently calls
Math.Abson the stream key's hash code. A hash ofint.MinValuethrowsOverflowException, preventing the message from being published.Route selection through an internal
GetProducerIndexhelper using unsigned modulo:(int)((uint)hashCode % (uint)producerCount). For positive producer counts, every hash maps to a valid producer index. This selects a publishing connection while the existing NATS subject determines stream partitioning.The regression theory covers
int.MinValue,-1,0, andint.MaxValuewith producer counts1,3,8, andint.MaxValue, asserting exact indices and array bounds.Extracted from #10798 into a standalone two-file fix based on current upstream
main. The original commit,210e0ba4ca94cd15b34691fb49bb80df945288ef, is preserved withcherry-pick -xand its author attribution to Reuben Bond.Microsoft Reviewers: Open in CodeFlow