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

Combination of rate_limiters #156

Open
praetp opened this issue Oct 30, 2022 · 3 comments
Open

Combination of rate_limiters #156

praetp opened this issue Oct 30, 2022 · 3 comments

Comments

@praetp
Copy link

praetp commented Oct 30, 2022

I think there is a use case you want to protect a resource with multiple rate limiters. E.g. one direct and one keyed or multiple keyed.
Example usecase:

  • you only want to have max 10req/s for a particular API call A and only max 5req/s per user group B and only max 1req/s per individual users, except user C, who can have 2 req/s

You cannot just consecutively call check() or check_key() because it will be a mess if you only have partial success. E.g. if you have two rate limiters and the first check() returns a positive outcome, and the second one in a negative outcome, you cannot continue but you have spent some of the first rate limiter's quota regardless.
Morever, the current architecture does not allow you to easily have different quotas (you have to make a different rate limiter then).

I am not sure how to solve it.
Maybe we have to introduce a third type of RateLimiter: the DictionaryRateLimiter.
For a given set of (key, value) pairs, it accepts a Quota, at construction time.
Example

let tuples = vec![
    ("api", Some("API_A"), Quota::per_second(nonzero!(10u32))),
    ("usergroup", Some("B"), Quota::per_second(nonzero!(5u32))),
    ("user", Some("C"), Quota::per_second(nonzero!(2u32))),
    ("user", None, Quota::per_second(nonzero!(1u32)))
];
let lim = RateLimiter::dictionary(tuples);

The usage would then be:

//Maybe use a Map<> instead - not sure
let tuples = vec![
    ("api", "API_A"),
    ("usergroup", "B"),
    ("user", "D")
];
lim.check_dictionary_n(tuples, 4)

This last function call would only have a positive outcome if all quotas for the different 'levels' (i.e. "api", "usergroup", "user") are satisfied.

Let me know what you think.

@antifuchs
Copy link
Collaborator

Thanks for posting this - I've been thinking about something like this all that time (it's a bit similar to #131, but not similar enough to warrant the same solution!).

The idea I had just now was that we could expose an internal method on rate limiters, that allows undoing a positive decision, effectively removing its weight from the bucket & restoring burst capacity. This could be used by a RateLimiterChain type that lets you combine checks against multiple rate limiters in sequence - allowing a decision to proceed only when all limiters allow the decision through on their own. If any one rejects the decision, the RateLimiterChain would call that "undo" method on the limiters that allowed the decision through, and undo the positive decision.

Some trade-offs there:

  • it's not atomic. If you want all these decisions to be made as one un-breakable unit, I don't think we'll support that (not without mutexes, but I'm reluctant to add those, especially in the face of async code).
  • it would be possible to combine rate limiters into multiple RateLimiterChains in different order, so that a check on one can block the other & the other way around. This might be possible to work around by ensuring that all limiters get checked in the same order (maybe by sorting them by smallest burst capacity).

What do you think?

@praetp
Copy link
Author

praetp commented Mar 21, 2023

Not a big fan of having an "undo" method. You want the user of your API to be shielded from this complexity.
Maybe you need to do "undo" internally but it is not something I would expose.

Yeah, I prefer an atomic approach. Not sure if mutexes are that bad in this context. They can be pretty fast nowadays.

@jcgruenhage
Copy link

I agree with @praetp here. An undo method feels like a bit of a footgun, and depending on the context, a mutex might be perfectly fine for this. My usecase here would be ensuring that we're not breaking rate-limits of an API we're talking to, and there are multiple rate-limits to consider that are in the realm of hundreds per hour. That's slow enough, that a mutex is not going to be noticeable.

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

No branches or pull requests

3 participants