Add LeakyBucket-based throttling

[jeroenvisser101]

Using the leaky bucket algorithm, you can get a better distrubution that allows a more human behaviour and that should (in theory) decrease false-positives.

It does that because it throttles differently. Let's take the old one, and concider you have a limit of 3 for 5 mins. If the user then did 3 requests, it'll have to wait for the next 5-min slot to do more requests.

Using the leaky bucket algorithm, doing 3 requests adds 3 drops in the bucket, and the bucket is full. The bucket then leaks consistently, and when leaked enough, it'll allow 1 more request (and then it will be full again).

Shopify uses and explains this too here.

We might want to add a custom throttled_response for the leaky bucket algorithm, so that it has the limits in there too.

Fixes #205.

[jeroenvisser101]

Sorry, this doesn't work yet, just tried it out in my app, and it's not blocking. Will investigate & fix.

[jeroenvisser101]

It works now—turns out not rounding the :expires_at will cause it not to be persisted at all. It's now being rounded up, and works fine!

[jeroenvisser101]

@ktheory I've also added documentation to the README, if you have time, could you check this out? Meanwhile, I'll push this into staging/prod to try out.

[ktheory]

Hi @jeroenvisser101,

Thanks for putting this together. It's highlighted a design constraint that I wasn't particularly clear on...

I want Rack::Attack throttle state to be written atomically.

The current leaky bucket implementation is not atomic: it's possible for two simultaneous requests to read the same value from cache, compute the updated value, and both write the same updated value; effectively not counting one request.

While it's generally unlikely that such a race condition occurs, it becomes more likely the more concurrent requests an application has (when you need throttling the most).

Rack::Attack.throttle is careful to use the redis/memcache atomic increment features to avoid uncounted requests.

One atomic leaky bucket implementation that I'd like to caution against is using compare-and-swap (cas) in a loop until it succeeds. The problem with that implementation is that it's non-deterministic, i.e., we don't know how many calls to our cache store we need in order to get a result.

If you can think of an atomic, deterministic leaky-bucket implementation that works in redis and memcached, I'd welcome this feature.

[ktheory]

@jeroenvisser101 Since the cached value is read in line 34, and written in line 37, this implementation is not atomic, and can lead to uncounted requests.

[jeroenvisser101]

Hey @ktheory, you're right, I didn't think about being atomic here.

I'll have a look tonight to see if I can make it atomic in a deterministic fashion. Thanks for checking this out!

[jeroenvisser101]

So I can do the following:

1. Deterministic and atomic increases of the bucket's value. (Using INCR just like throttle)
2. Non-atomic timestamp updates, but since these don't change so much based on the amount of requests, so atomic updates will not make that much of a difference?

This will mean that for each client, 2 keys will be stored. I'll also change the TTL logic to be the amount of seconds it takes to drain the full capacity (rather than the current value), since it wouldn't be possible to update that in an atomic way.

Update: This won't work, since it's not counting down, let me think some more about this

[ktheory]

Gonna close this out as it's stalled. Feel free to re-open with updates.

[jeroenvisser101]

@ktheory yeah, I think this idea is a little dead, I've not been able to come up with a solution that's fully atomic, but it's semi-atomic. If I have time I'll set up a new PR, thanks for all the feedback so far!