Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composable rate limiters #6431

Open
mperham opened this issue Sep 17, 2024 · 2 comments
Open

Composable rate limiters #6431

mperham opened this issue Sep 17, 2024 · 2 comments
Milestone

Comments

@mperham
Copy link
Collaborator

mperham commented Sep 17, 2024

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:

  1. Every within_limit call increments a counter to track the limit. If x passes but y fails, we've incremented x unnecessarily.
  2. 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.
  3. 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.

@mperham mperham added this to the 8.0 milestone Sep 22, 2024
@gstokkink
Copy link

gstokkink commented Sep 27, 2024

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants