chriso / redback

A high-level Redis library
MIT License
541 stars 46 forks source link

Ratelimit issue #38

Open laino opened 7 years ago

laino commented 7 years ago

There's cases in which an "expired" bucket may get re-used before it was reset to 0.

This will happen for any client that didn't increment the hitcount for the duration of at least 2 "buckets" before incrementing a bucket again.

For the default settings (bucket span of 10 minutes) picture a client that sends requests only every 10 minutes. They will end up in the same bucket everytime, which will never be cleared. This bug would affect any client that has pauses for longer than 10 seconds though.

A possible fix (afaik) for this would be to save a string with "timestamp:count" in each bucket instead and use a lua script to increment the bucket instead of using HINCRYBY. Another fix would be to keep the timestamp in a separate field "bucketNumber:timestamp = timestamp", which would still require a lua script for incrementing (because we can't get around checking the timestamp before incrementing).

Edit: A third solution (which is how I'm doing rate limiting right now) is to use individual redis keys for buckets which each have their own TTL. This might not be the optimal solution if you have many active buckets though (I only use 2 with an ever-increasing bucket counter).

laino commented 7 years ago

Another solution: Keep twice the amount of buckets required, use a sliding window of half the amount of buckets and clear the other half of buckets on each INCR. Then set the TTL equal to the size of the window.

chaoran commented 7 years ago

This is what I did in my repo: https://github.com/chaoran/node-throttle

Check it out.

Chaoran

On Apr 20, 2017, at 4:15 PM, Nils Kuhnhenn notifications@github.com wrote:

Another solution: Keep twice the amount of buckets required, use a sliding window of half the amount of buckets and clear the other half of buckets on each INCR. Then set the TTL equal to the size of the window.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.