fix(sim): bound the partition count a broker will act on - #61
Merged
Conversation
|
@kevin9327 is attempting to deploy a commit to the whoarrryou's projects Team on Vercel. A member of the Team first needs to authorize it. |
This was referenced Sep 6, 2026
Owner
|
LGTM, thanks! |
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.
What
A design carrying a
partitionscount larger than the inspector can set does not slow the brokerdown, it ends the process.
initStatebuilds one ring buffer per partition:clampInthere already knows this class of value and guards half of it:Infinityis a number and is notNaN, so it passes the guard, survivesMath.floor, and reachesthat loop, which has no end for it. There is no ceiling on a large finite count either.
Measured
Running the decoded topology through the engine for 60 ticks:
The cost is linear in the count, so this is allocation rather than a slow frame:
pubsubreads the same count through the same helper and behaves the same way.Against
mainthe new test file cannot finish. It is not a failed assertion, the worker runs out ofmemory:
Where it comes from
Not the inspector: its number input stops at 64.
isTopologychecks the nine core config numbers —capacity,serviceMs,serviceCv,queueLimit,hitRate,errorRate,timeoutMs,retries,rps— and notpartitions, so a shared link, a.breakscalefile and a restored session carry itthrough untouched.
Fix
clampIntrejects anything that is not a finite number rather than onlyNaN, and takes amax.brokerPartitionspassesMAX_PARTITIONS = 64, which is the maximum the inspector already offers,so nothing a reader can build changes.
The ceiling is the same move the module already makes one line further down:
MAX_TOTAL_RETENTIONcaps the retained-message count "whatever the slider says". This caps the other dimension of the
same structure, which is the one that allocates.
The other three callers of
clampIntpass nomaxand keep their current ceilings, including theMath.min(..., 2000)already wrapped aroundbatchSize. They gain the finite check, which changesnothing measurable — I probed
outlierAfterandmaxConcurrencywith the same values and neithersizes anything, they are compared against.
Tests
src/sim/behaviour-messaging.bounds.test.ts, new:streambrokerwithInfinityand with1e9partitions both run, in well under a secondpubsubwithInfinitypartitions runsreader can reach
How I tested
Windows 11, Bun 1.3.14.
bun run testis 934 passed across 40 files, up from 929 by the five newtests, with nothing else moving.
bun run typecheck,bun run lintandbun run format:checkareclean.
This is the same shape as #58 and sits alongside #60, which does the equivalent for the data tier's
counts. They touch different files and neither depends on the other.