⚡ perf: Optimize proxy balancer round-robin - #4549
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe proxy round-robin selector now uses an atomic CAS-based counter instead of a mutex. Tests cover counter wrap-around and concurrent distribution, while benchmarks compare atomic and mutex-protected implementations. ChangesProxy round-robin selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4549 +/- ##
=======================================
Coverage 93.29% 93.29%
=======================================
Files 140 140
Lines 14854 14856 +2
=======================================
+ Hits 13858 13860 +2
Misses 620 620
Partials 376 376
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| // Test_Security_RoundRobin_WrapsAround covers the modulo wrap-around in | ||
| // urlRoundrobin.get when current >= len(pool). | ||
| func Test_Security_RoundRobin_WrapsAround(t *testing.T) { |
There was a problem hiding this comment.
You're right—there was no reason to remove this coverage. I've restored it and expanded it to cover both the normal round-robin sequence and the counter-limit case.
|
@james-yusuke Why you closed it? |
|
Sorry, I closed it by mistake while handling the benchmark failure. The checks are now green, so I’ve reopened it. Could you please take another look? |
5f2ef10 to
4a178bc
Compare
Description
Removes the lock from
BalancerForwardserver selection to reduce contention on the round-robin hot path.The selector now uses an atomic bounded counter while preserving the existing cyclic selection behavior, including counter rollover. Public APIs, proxy validation, and SSRF protections are unchanged.
Changes introduced
uint64boundary.Type of change
Checklist