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

[Epic]: Rate/Resource Limiting #37380

Closed
7 tasks done
rafikiassumani-msft opened this issue Oct 8, 2021 · 11 comments
Closed
7 tasks done

[Epic]: Rate/Resource Limiting #37380

rafikiassumani-msft opened this issue Oct 8, 2021 · 11 comments
Assignees
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares feature-rate-limit Work related to use of rate limit primitives

Comments

@rafikiassumani-msft
Copy link
Contributor

rafikiassumani-msft commented Oct 8, 2021

This issue is intended to track work items for Rate or Resource Limiting feature.

Additional information

https://github.com/dotnet/designs/blob/main/proposed/rate-limit.md#dotnetruntime-pocs

@rafikiassumani-msft rafikiassumani-msft added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Oct 8, 2021
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Oct 8, 2021
@rafikiassumani-msft rafikiassumani-msft added the feature-rate-limit Work related to use of rate limit primitives label Oct 8, 2021
@rafikiassumani-msft rafikiassumani-msft changed the title Epic: Rate/Resource Limiting [EPIC]: Rate/Resource Limiting Dec 27, 2021
@rafikiassumani-msft rafikiassumani-msft changed the title [EPIC]: Rate/Resource Limiting [Epic]: Rate/Resource Limiting Dec 27, 2021
@rafikiassumani-msft rafikiassumani-msft modified the milestones: .NET 7 Planning, .NET 7.0 Jan 6, 2022
@rafikiassumani-msft rafikiassumani-msft modified the milestones: .NET 7.0, .NET 7 Planning Jan 25, 2022
@ghost
Copy link

ghost commented Jan 25, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 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.

@brijeshb
Copy link

brijeshb commented May 25, 2022

Hey there,

Learnt about the ratelimiter finally happening in .net 7, so came here to re-enforce an ask.
Please ensure that the core ratelimiter is domain agnostic and can be used in non server environments, for example in console apps / workers / other hosted services etc.

One of the constant issues with most rate limiter implementations that i see is an innate assumption that it will only be used in traditional rest api scenarios and this severely limits it's usability.

If you ensure that the base implementation consumes a set of variables and then throttles based on them, it would allow far more re-use. For example in graphql, we don't see much variation in basic http request context, but mid way through processing we're able to extract information from bodies and pass that into rate limiting checks.

Or in message processors / works, where there is a massive variation in implementation details, at the end of the day we're able to bubble up some key variables and we just have to enforce limits on combinations of them.

I do see the feature tracked above to bubble some parts of this back down into base .net. But wanted to help re-enforce that it would be really good if that can be achieved as part of the initial release.

@halter73
Copy link
Member

One of the constant issues with most rate limiter implementations that i see is an innate assumption that it will only be used in traditional rest api scenarios and this severely limits it's usability.

Check out the work we're doing in the runtime repo to address non-web scenarios: dotnet/runtime#65400

If you take a look at the rate limiting middleware sample in our .NET 7 preview 4 blog post, you can see that PartitionedRateLimiter.Create<TResource, TPartitionKey>() can take any resource, not just HttpContext. HttpContext makes sense as a TResource for rate limiting requests, but for the graphql it might be GraphQL.ExecutionOptions or something else.

PartitionedRateLimiter.Create allows you to easily define and apply different rules based on whatever aspect of TResource is important to the application. Maybe it's the user that's making a request, or the id of the resource, or some combination of these.

If this interests you @brijeshb, I recommend trying this API out! It's already available in .NET 7 Preview 4. You can see how we use this API for middleware in this AWESOME video by @Elfocrash

@brijeshb
Copy link

Hey Stephen,

Coincidently, that is the exact video that led me to come by :), but I think in my quick skim of that video, I had assumed that HttpContext is the only thing that can be injected in.

Glad to see the work happening in runtime, I think we'll be able to give it a realistic shot as soon as we get to prototype it for the non-http scenarios with a distributed store backing it, and maybe take it to prod a month or two after 7's release.

For the distributed scenario, I have a hypothesis that an implementation that relies on either being purely in a backing store like redis / or some other non-volatile storage may not be sufficient at scale. And that we could potentially reduce latency overhead of throttling further by splitting the tracking by having local counters that update the centralized store only when they cross some threshold or periodically.

Curious if you've had any discussions / brainstorms along these lines.

@halter73
Copy link
Member

Curious if you've had any discussions / brainstorms along these lines.

We have, but you've encouraged me to file a public issue at #41861.

@tpeczek
Copy link

tpeczek commented Jul 26, 2022

Hi,

I've been playing with RateLimiting middleware in .NET 7 Preview 6 and it seems that retry-after metadata for FixedWindowRateLimiter always provides the window size. I'm not sure if this is a bug or by desing, but would it be possible for it to provide the actual time remaining till replenishing?

@rwkarg
Copy link

rwkarg commented Jul 27, 2022

Hi,

I've been playing with RateLimiting middleware in .NET 7 Preview 6 and it seems that retry-after metadata for FixedWindowRateLimiter always provides the window size. I'm not sure if this is a bug or by desing, but would it be possible for it to provide the actual time remaining till replenishing?

I don’t have a spec to reference but it would make sense to give the window size otherwise you’d have all of the throttled clients retrying at the same time when the “actual time” refreshes.

The Token limiter, at least, has the same behavior for retry-after.

@tpeczek
Copy link

tpeczek commented Jul 27, 2022

Hi,
I've been playing with RateLimiting middleware in .NET 7 Preview 6 and it seems that retry-after metadata for FixedWindowRateLimiter always provides the window size. I'm not sure if this is a bug or by desing, but would it be possible for it to provide the actual time remaining till replenishing?

I don’t have a spec to reference but it would make sense to give the window size otherwise you’d have all of the throttled clients retrying at the same time when the “actual time” refreshes.

The Token limiter, at least, has the same behavior for retry-after.

In fact this is becoming quite sophisticated very quickly. In web context it's quite common to have different rate limits for different clients (based for example on client identifiers or IP addresses). I'm assuming that RateLimiting middleware is designed to handle similar cases, this is way it seems to be centralized around PartitionedRateLimiter.

This might be more of an opinion than a fact, but from my experience the usual expectation in the web context is that Retry-After HTTP header contains the actual remaining time to which clients add jitter. But as I said, I might be have been exposed to specific set of projects and maybe it's not like this in general.

Also I see a potential risk of "unlucky client" if entire window length is always returned. If a client makes his request close to the end of the window, gets rate limited, and then waits the entire window length, it becomes likely that he will be rate limited again as the whole quota has been already used by others. This way client might end up being constantly rate limited, so a jitter on the client side may be required anyway and returning of the whole window just makes client unnecessary wait longer.

@rf-0
Copy link

rf-0 commented Aug 18, 2022

Will this work for scale-out scenarios? ie: saved shared state externally like in Redis so rate limiting will work per user per request across multiple proxies/gateway instances?

I was looking into the combination of Yarp + Rate limiting (dotnet 7) for a microservices architecture.

@BrennanConroy
Copy link
Member

See #41861

@ghost
Copy link

ghost commented Oct 11, 2022

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.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2022
@amcasey amcasey added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jun 2, 2023
@danmoseley danmoseley removed the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares feature-rate-limit Work related to use of rate limit primitives
Projects
None yet
Development

No branches or pull requests

9 participants