fix stale entries before the each pick operation #3690
Conversation
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
shenqidebaozi
previously approved these changes
Jul 31, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a memory leak issue in the weighted round-robin (WRR) load balancer by implementing cleanup logic for stale entries in the currentWeight map. When service discovery updates the node list, old entries were persisting in the map and could affect future weight calculations.
Key changes:
- Added cleanup logic to remove stale entries from the
currentWeightmap before each pick operation - Moved mutex locking to encompass the entire cleanup and selection process
- Added comprehensive test coverage to verify the cleanup functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| selector/wrr/wrr.go | Implements cleanup logic for stale entries and reorganizes mutex usage |
| selector/wrr/wrr_test.go | Adds test coverage for the cleanup functionality with mock weighted nodes |
|
@tonybase PTAL |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
shenqidebaozi
approved these changes
Aug 5, 2025
|
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.
The original implementation had a critical flaw in the
currentWeightmap management:currentWeightmap accumulated entries for every node address that was ever encounteredDescription (what this PR does / why we need it):
Which issue(s) this PR fixes (resolves / be part of):
Other special notes for the reviewers: