Skip to content

router: cleanup pending entries once exporter and client are connected#164

Merged
mangelajo merged 1 commit intojumpstarter-dev:mainfrom
bennyz:router-memory-leak
Jan 27, 2026
Merged

router: cleanup pending entries once exporter and client are connected#164
mangelajo merged 1 commit intojumpstarter-dev:mainfrom
bennyz:router-memory-leak

Conversation

@bennyz
Copy link
Copy Markdown
Member

@bennyz bennyz commented Jan 27, 2026

address #161

Summary by CodeRabbit

Bug Fixes

  • Fixed resource cleanup issues in peer connection streaming to prevent memory leaks and ensure stable, reliable operation during concurrent connection scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 27, 2026

Warning

Rate limit exceeded

@bennyz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 43 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The RouterService.Stream method now ensures pending entry cleanup in both code paths: deleting immediately when a second peer connects, and scheduling deferred cleanup when the first peer connects. This guarantees removal of pending map entries that previously could persist longer than intended.

Changes

Cohort / File(s) Summary
Stream Pending Entry Cleanup
controller/internal/service/router_service.go
Added immediate deletion of pending entry when second peer connects and deferred cleanup scheduling when first peer connects, ensuring guaranteed removal of pending map entries on function exit or early termination.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through streams so bright,
Where pending entries fade from sight!
With cleanup swift and deferred care,
No lingering state left hanging there! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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: adding cleanup logic for pending entries in the router service when peers connect.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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


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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@controller/internal/service/router_service.go`:
- Around line 102-103: There is a race where the deferred cleanup
unconditionally calls s.pending.Delete(streamName) and can remove a newer entry;
fix by making the cleanup identity-checked so we only delete if the map still
holds the same streamContext instance (the one stored as sctx). Replace the
unconditional s.pending.Delete(streamName) in the peer cleanup/defer with either
a conditional check that reads the current value and deletes only if it equals
sctx, or (since we target Go 1.24) call s.pending.CompareAndDelete(streamName,
sctx) in the defer; ensure you reference the stored streamContext variable (sctx
/ streamContext) when performing the compare-and-delete to avoid removing newer
entries.

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
@bennyz bennyz force-pushed the router-memory-leak branch from 7d82d7b to e674da7 Compare January 27, 2026 11:31
@bennyz bennyz requested a review from mangelajo January 27, 2026 11:42
Copy link
Copy Markdown
Member

@mangelajo mangelajo left a comment

Choose a reason for hiding this comment

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

ahaaa! :)

@mangelajo mangelajo merged commit 4e3af2f into jumpstarter-dev:main Jan 27, 2026
13 checks passed
@bennyz bennyz deleted the router-memory-leak branch January 28, 2026 08:19
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.

jumpstarter-router memory grows over time, hits the 1GB limit setting and starts failing

2 participants