Composable rate limiters

[mperham]

Several customers have asked for support for basic composition with rate limiters, e.g. (limit1 AND limit2 AND limit3). Imagine a scenario where a service has an official rate limit of "10 per second, 100 per minute":

x = Sidekiq::Limiter.window(:svc_sec, 10, :second)
y = Sidekiq::Limiter.window(:svc_min, 100, :minute)

x.within_limit do
  y.within_limit do
    service.call ...
  end
end

There's a number of issues with this:

0. Every within_limit call increments a counter to track the limit. If x passes but y fails, we've incremented x unnecessarily.
1. Limiters are typically realtime-sensitive so things like the current bucket can change from millisecond to millisecond. x and y could use different buckets/windows/etc.
2. We don't want Lua executing a long chain of logic within Redis, leading to latency spikes.

I think the correct approach is for optimistic transactions: each limiter call will make a round trip to Redis and consume a token, but that call will return an object which can be used to rollback that consumption if a later limiter raises OverLimit. In the code above, if x passes ok but y raises OverLimit, we want to rollback the token consumed by x since we didn't actually make the call to the service.

Something like this:

Sidekiq::Limiter.within_all_limits(x, y) do
  service.call
end

# or

Sidekiq::Limiter.and(x, y) do
  service.call
end

Internally that API would maintain a list of rollback objects and roll back any earlier limits if a later limiter raises OverLimit. Is a simple AND(x, y) sufficient? Would we ever want OR(x, y)?

Ideas, comments and feedback welcome.

[gstokkink]

Haha, this is indeed what I was looking for when I e-mailed you (Gerjan here). Thanks for creating an issue 👍 For now we have worked around this limitation by simply enforcing a maximum of two limiters, and requiring that the outer limiter is always a concurrent limiter. That way it should work correctly. Ideally, however, we would be able to nest more than 2 limiters.

I would go for the within_all_limits approach. I cannot see a use case for the OR approach.

[rubendinho]

We ran into this issue as well. The first syntax within_all_limits(x, y) feels very readable and clear.

[mperham]

I don't believe this change is worth the pretty dramatic increase in code complexity. Most people don't need it so the cost/benefit ratio doesn't pencil for me.

I'd suggest people using multiple rate limiters to push back pretty hard against that idea. It's just not a good idea to have multiple different rate limits enforced simultaneously. A leaky bucket limiter with a generous burst is extremely flexible and the recommended solution if you are choosing your own rate limiting mechanism.