Skip to content

fix: limit reconnecting pool known ids to 1000#558

Merged
rolznz merged 1 commit into
masterfrom
fix/reconnecting-pool-limit-known-ids
May 14, 2026
Merged

fix: limit reconnecting pool known ids to 1000#558
rolznz merged 1 commit into
masterfrom
fix/reconnecting-pool-limit-known-ids

Conversation

@rolznz

@rolznz rolznz commented May 14, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes
    • Optimized event deduplication to prevent unbounded memory growth in long-lived subscriptions. The deduplication mechanism now enforces a size limit and removes oldest entries when the threshold is exceeded, improving stability during extended operations and reconnect bursts.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d8e952d-8867-426f-b3ba-ac5ef11c108d

📥 Commits

Reviewing files that changed from the base of the PR and between 367f585 and 8705b97.

📒 Files selected for processing (1)
  • src/nwc/ReconnectingPool.ts

📝 Walkthrough

Walkthrough

The ReconnectingPool.subscribe() method now bounds memory usage of its event de-duplication tracking. A new MAX_KNOWN_IDS constant (1000) caps the knownIds set size, and when exceeded, the oldest tracked id is evicted to prevent unbounded growth during long-lived connections.

Changes

Memory-Bounded Event De-duplication

Layer / File(s) Summary
Add knownIds size limit and eviction
src/nwc/ReconnectingPool.ts
MAX_KNOWN_IDS constant (1000) caps the knownIds set used for event de-duplication. When the set exceeds this limit, the oldest entry is removed via values().next() to prevent unbounded memory growth during long-lived subscriptions and reconnect bursts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A pool of connections, once ever-growing,
Now bounded by a thousand, the oldest ids flowing—
Away like autumn leaves in a careful hand,
Where memory stays tame across the reconnecting land. 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: limiting the reconnecting pool's known IDs to 1000, which directly matches the summary of adding a MAX_KNOWN_IDS constant to bound the knownIds set size.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reconnecting-pool-limit-known-ids

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rolznz rolznz merged commit e4864e0 into master May 14, 2026
4 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.

1 participant