Skip to content

fix(sim): bound the partition count a broker will act on - #61

Merged
xevrion merged 1 commit into
xevrion:mainfrom
kevin9327:fix/partition-bounds
Sep 7, 2026
Merged

fix(sim): bound the partition count a broker will act on#61
xevrion merged 1 commit into
xevrion:mainfrom
kevin9327:fix/partition-bounds

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What

A design carrying a partitions count larger than the inspector can set does not slow the broker
down, it ends the process. initState builds one ring buffer per partition:

for (let p = 0; p < partitions; p++) rings.push(new Int32Array(retention));

clampInt here already knows this class of value and guards half of it:

function clampInt(v: number | undefined, min: number, fallback: number): number {
  if (v === undefined || Number.isNaN(v)) return fallback;
  const n = Math.floor(v);
  return n < min ? min : n;
}

Infinity is a number and is not NaN, so it passes the guard, survives Math.floor, and reaches
that 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:

partitions   what happens
NaN          fine, the existing guard catches it
Infinity     does not return (killed at 15s)
1e9          does not return (killed at 15s)

The cost is linear in the count, so this is allocation rather than a slow frame:

1e3 partitions      8ms
1e5 partitions    133ms
1e6 partitions   1149ms

pubsub reads the same count through the same helper and behaves the same way.

Against main the new test file cannot finish. It is not a failed assertion, the worker runs out of
memory:

1: 00007FF6CC930E3F node::OnFatalError+1343
Caused by: Error: Worker exited unexpectedly
 Test Files   (1)
      Tests   (5)

Where it comes from

Not the inspector: its number input stops at 64. isTopology checks the nine core config numbers —
capacity, serviceMs, serviceCv, queueLimit, hitRate, errorRate, timeoutMs, retries,
rps — and not partitions, so a shared link, a .breakscale file and a restored session carry it
through untouched.

Fix

clampInt rejects anything that is not a finite number rather than only NaN, and takes a max.
brokerPartitions passes MAX_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_RETENTION
caps 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 clampInt pass no max and keep their current ceilings, including the
Math.min(..., 2000) already wrapped around batchSize. They gain the finite check, which changes
nothing measurable — I probed outlierAfter and maxConcurrency with the same values and neither
sizes anything, they are compared against.

Tests

src/sim/behaviour-messaging.bounds.test.ts, new:

  • a streambroker with Infinity and with 1e9 partitions both run, in well under a second
  • a pubsub with Infinity partitions runs
  • an absent count still falls back to four
  • eight partitions are still eight partitions, which is the check that the ceiling changed nothing a
    reader can reach

How I tested

Windows 11, Bun 1.3.14. bun run test is 934 passed across 40 files, up from 929 by the five new
tests, with nothing else moving. bun run typecheck, bun run lint and bun run format:check are
clean.

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.

@kevin9327
kevin9327 requested a review from xevrion as a code owner September 6, 2026 11:45
@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

@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.

@xevrion

xevrion commented Sep 7, 2026

Copy link
Copy Markdown
Owner

LGTM, thanks!

@xevrion
xevrion merged commit 91e06bf into xevrion:main Sep 7, 2026
6 of 7 checks passed
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.

2 participants