jdevost
15 באוקטובר, 2018, 2:35pm
1
When we retry after a 429, we get another 429.
I suspect a round-down issue.
In our scenario:
we do requests, and eventually get a 429 Too Many Request with Retry-After: 19
we wait 19 seconds, then try again:
—> We get a 429 again, with Retry-After: 0
Now, can it be the actual values were 19.4 and 0.4 for example and they were rounded down for the Retry-After in the headers?
4 לייקים
Probably, should be an easy fix. Another starter task @erlend_sh ?
2 לייקים
dinomite
(Drew Stephens)
28 בנובמבר, 2018, 4:10pm
3
I’m not entirely sure where this is happening, but I suspect it has to do with use of Lua’s tonumber() in rate_limiter.rb —quoth Redis’ EVAL documentation :
Lua has a single numerical type, Lua numbers. There is no distinction between integers and floats. So we always convert Lua numbers into integer replies, removing the decimal part of the number if any.
4 לייקים
andrei
(Andrei Prigorshnev)
11 בינואר, 2021, 5:02pm
4
I’m taking care of this bug and going to submit a pull-request tomorrow.
לייק 1
andrei
(Andrei Prigorshnev)
12 בינואר, 2021, 4:52pm
5
It turned out that it wasn’t a problem with rounding in Lua script. Lua script gets already rounded values.
Moreover, it’s not a problem with rounding at all. Unrounded seconds might be used, and it would be possible to receive a 429 response with Retry-After: 0 anyway.
Here is the fix for the main problem:
master ← AndrewPrigorshnev:fix/sliding-window-end-time-in-rate-limiter
merged 06:26PM - 12 Jan 21 UTC
If the sliding window size is N seconds, then a moment at the Nth second should … be considered as the moment outside of the sliding window.
Otherwise, if the sliding window is already full, at the Nth second, a new call wouldn't be allowed, but a time to wait before the next call would be equal to zero, which is confusing.
In other words, the end of the time range shouldn't be included in the sliding window. Let's say we start at the second 0, and the sliding window size is 10 seconds. In the current version of Rate Limiter, this sliding window will be considered as a time range `[0, 10]` (including the end of the range), which actually is 11 seconds in length.
After this fix, the time range will be considered as `[0, 10)` (excluding the end of the range), which is exactly 10 seconds in length.
And here is the fix for an additional problem that can occasionally cause the same error:
master ← AndrewPrigorshnev:fix/use-the-same-time-moment-in-redis-calls
merged 08:09PM - 12 Jan 21 UTC
3 לייקים