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

Support Multiple PartitionedRateLimiter Per Endpoint #42691

Open
1 task done
Kahbazi opened this issue Jul 12, 2022 · 23 comments
Open
1 task done

Support Multiple PartitionedRateLimiter Per Endpoint #42691

Kahbazi opened this issue Jul 12, 2022 · 23 comments
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware feature-rate-limit Work related to use of rate limit primitives
Milestone

Comments

@Kahbazi
Copy link
Member

Kahbazi commented Jul 12, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Right now RateLimiting middleware has two PartitionedRateLimiter. A global and an endpoint limiter. This means for each endpoint there could be at most two level of rate limiters and also the global one is somehow limited because it must be same for all endpoints.
So I can not limit my endpoints based on more than two partitions. As an example I need to limit first based on request IP, then current user Id and then based on the current endpoint.
Let's say I need to limit 10 requests per second per IP, no matter which endpoint. And also limit 5 requests per second per User Id.

Also I can't have different window and limit based on one partition. Again let's say I need to limit 10 requests per second per IP and also limit 40 requests per minute per IP.

Describe the solution you'd like

I'm suggesting to create PartitionedRateLimiter<HttpContext> based on policy and have a Dictionary<string, PartitionedRateLimiter<HttpContext> which policy is the key. The endpoints could have multiple IRequireRateLimitMetadata and every one of them is a policy with its own PartitionedRateLimiter. And the middleware would always call Acquire on each of these limiters whether IsAcquired is true or false and only limit the request if one of them has IsAcquired = false.

This way part of the limiter like checking based on User Id could be shared between multiple endpoints too.

Additional context

cc @wtgodbe @BrennanConroy @halter

@TanayParikh TanayParikh added area-runtime feature-rate-limit Work related to use of rate limit primitives labels Jul 12, 2022
@wtgodbe
Copy link
Member

wtgodbe commented Jul 12, 2022

This should be achievable through the CreateChained API in runtime - it allows you to pass in multiple PartitionedRateLimiters which it will combine into 1 PartitionedRateLimiter that runs all of your input limiters in sequence. Then you could apply that single chained limiter to the endpoint.

@wtgodbe
Copy link
Member

wtgodbe commented Jul 12, 2022

Going to close this as I believe it's covered by the above comment - feel free to re-open if there's additional functionality in this request that I'm missing.

@wtgodbe wtgodbe closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2022
@Kahbazi
Copy link
Member Author

Kahbazi commented Jul 13, 2022

@wtgodbe I believe the issue should be re-opened. It seems ChainedPartitionedRateLimiter is the answer on how to implement my requirements (with a small change that I need to call Acquire on every limiter whether IsAcquired is true or false), but I don't see any way to achieve that using the current implementation of the middleware. Is there any sample on how to do that? And also I can't use the global limiter because I would need different ChainedPartitionedRateLimiter per endpoint.

@wtgodbe wtgodbe reopened this Jul 13, 2022
@wtgodbe
Copy link
Member

wtgodbe commented Jul 13, 2022

I see, I think you're right that we don't currently support that (endpoint policies are RateLimitPartition based, not PartitionedRateLimiter based). We'll consider this for future passes

@adityamandaleeka
Copy link
Member

Triage: we're open to this but we'd like to see more use cases for this sort of pattern.

@adityamandaleeka adityamandaleeka added this to the Backlog milestone Jul 13, 2022
@ghost
Copy link

ghost commented Jul 13, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@adityamandaleeka
Copy link
Member

We have another request for this: #44907

@BondarencoM
Copy link

I also want to rate limit an endpoint with one partition but multiple time spans (e.g. allow 3 requests per second but only 100 per hour). As far as I can tell this is currently not possible but feels like a very basic use case.

@stefannikolei
Copy link
Contributor

+100 for @BondarencoM.

ATM it seems not possible to create a policy, which calls PartitionedRateLimiter.CreateChained().

I also have the usecase to create a policy for an endpoint which eg. allows 1 Request per Minute and 10 Requests per Hour. Atm this does not seem to be easy achievable.

@juasanpar
Copy link

Greetings,

I have a very similar requirement of adding a RateLimiting policy of limiting the requests to 50 per minute and also not allowing more that 10 concurrent. At the moment this is only achievable using PartitionedRateLimiter.CreateChained() as you have already mentioned, but only for the global limiter. I have an API with several endpoints that need different limiting policies, so this is not the best option for me. Is there any progress on this request or is there any other way to achieve my purpose?

Thanks for your work and help!

@wtgodbe
Copy link
Member

wtgodbe commented Jan 30, 2023

I agree we should do this for 8. Right now, you apply RateLimitPartitions to endpoints, not PartitionedRateLimiters - the latter would be necessary to solve the problem. I think we could solve it by doing an internal CreateChained for PartitionedRateLimiters added to endpoints

@ghost
Copy link

ghost commented Jan 31, 2023

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@MrChriZ
Copy link

MrChriZ commented Apr 26, 2023

I've just come at this from exactly the same direction as @BondarencoM.

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 2, 2023
@LinqToException
Copy link

I've spent the last hour frustrated looking for a solution for this problem as well.

In my case, I have an endpoint that invokes another, 3rd party service, which has its own rate limiting. As such, I would have liked to create a policy on that single endpoint that

  • limits concurrent requests across all users to 5 (as specified by the 3rd party service),
  • limits requests by individual users using a token bucket or similar to allow burst-usage and
  • limits requests by individual users using a fixed window to a fixed amount per day

@stukalin
Copy link

I'd like to +1 on this one. We'd like to migrate away from the https://github.com/stefanprodan/AspNetCoreRateLimit project and some of our policies will be expressed by a chain of fixed window validators but they should be a custom IRateLimiterPolicy and apparenlty it's not possible to specify a chain as a non-global rate limiter.

@sizzle168
Copy link

We face the same challenge that @stukalin describes.

@GiampaoloGabba
Copy link

GiampaoloGabba commented Nov 5, 2023

+1 for this. i tried to apply multiple policy to an endpoint but only the last one applied work.
Imho this is a very basic feature for a rate limiter, i see that didnt make it for .net8, i hope you consider it for future releases.
I really wanted to use the rate limiting feature bundled with aspnetcore but is not flexible enough for my usecase.

@CreepMania
Copy link

+1

We tried to implement a token bucket policy on top of fixed window policy but hit a wall.

@odin568
Copy link

odin568 commented Dec 22, 2023

Unfortunately did not make it in .NET 8 :(

@MikeNolan678
Copy link

MikeNolan678 commented Mar 9, 2024

+1

Has anyone been able to find a solution to this, which doesn't involve using the AspNetCoreRateLimit NuGet package?

@atomaras
Copy link

Personally planning on building my own and following this guide https://developer.redis.com/develop/dotnet/aspnetcore/rate-limiting/middleware which has the added benefit (aside from the ability to do distributed rate limiting) of sending all rate limit rules that apply to a single endpoint over to Redis in a single request.

@MikeNolan678
Copy link

Personally planning on building my own and following this guide https://developer.redis.com/develop/dotnet/aspnetcore/rate-limiting/middleware which has the added benefit (aside from the ability to do distributed rate limiting) of sending all rate limit rules that apply to a single endpoint over to Redis in a single request.

Nice! Thanks for sharing. I ended up using the ASPNetCoreRateLimit package in the end.

@atomaras
Copy link

atomaras commented Mar 28, 2024

Spent a day so you don't have to. Here's the Redis Lua script (co-written by ChatGPT ofc) that implements the Sliding Window Rate Limit algo with support for multiple rules per invocation AND support for returning the remaining_tokens so you can power the X-Rate-Limit-Remaining header in your web api
https://gist.github.com/atomaras/925a13f07c24df7f15dcc4fb7bc89c81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware feature-rate-limit Work related to use of rate limit primitives
Projects
None yet
Development

No branches or pull requests