Minor global ratelimiter fix: don't reduce values when "boosting" #6281
Conversation
Groxx
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
| "usage over target": { | ||
| targetLimit: 10, | ||
| fallbackLimit: 3, | ||
| weight: 0.1, | ||
| usedRPS: 100, | ||
| resultLimit: 1, // should be weight-based at worst, and not be reduced by excess usage | ||
| }, |
There was a problem hiding this comment.
Error: Max difference between 0.89 and -89.11 allowed is 0.01, but difference was 90
Test: TestBoostRPS/usage_over_target
Messages: computed RPS does not match closely enough, should be 0.89
--- FAIL: TestBoostRPS/usage_over_target (0.00s)
yea that's not good. fixed now!
this would probably be a good target for a small fuzz test, now that I think about it. there are clear min/max values: target*weight and target.
(I was curious so I checked: negative values just act like zero, so this isn't risking crashes or anything currently)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Previously, if the target RPS was below the cluster's usage (say it was being reduced, or cluster usage was over target),
unusedcould go below zero and set the RPS to below the fair-weighted value, and possibly even to a negative value.That's unintended, the fair-weighted value is what we want even in an excess-load scenario.
Boosting was only intended to increase based on spare capacity, not decrease.