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

Add a way to check rate limits without using quota #244

Open
dweitzman opened this issue Apr 15, 2021 · 3 comments
Open

Add a way to check rate limits without using quota #244

dweitzman opened this issue Apr 15, 2021 · 3 comments

Comments

@dweitzman
Copy link
Contributor

dweitzman commented Apr 15, 2021

The grpc RateLimitService has only one method ShouldRateLimit and hits_addend is assumed to be 1 when no value has been explicitly set.

Why would a person want to get quota without incrementing it?

One example is if you're dealing with expensive requests but you don't know how expensive they are until after they've run. Maybe you add up the CPU seconds spent serving a request and increment it at the end of the request. You'd still need to check (but not increment) at the beginning of requests whether the CPU-second quota has been exhausted by previous requests during the current time unit.

A hacky workaround today might be to make sure that expensive requests are measured in large numbers (like hundreds of thousands) and check quota by incrementing a very small number (like 1) that might effectively act as if you haven't incremented it at all.

How might this be implemented?

Seems like there are a few possible approaches to getting quota without changing it:

  • Do something clever and subtle like saying that a hits_addend equal to uint32_max means zero
  • Add some kind of a check_only flag to the RateLimitRequest message
  • Make a separate IsRateLimited method on RateLimitService
  • Make a separate non-standard grpc service specific to this rate limiter implementation so the RateLimitService in envoy proxy doesn't need to change
@mattklein123
Copy link
Member

I don't feel strongly about it, but check_only sounds reasonable to me.

@rushabhvaria
Copy link

Is there any update on getting this feature added?

@dweitzman
Copy link
Contributor Author

Is there any update on getting this feature added?

Seems like the maintainers are open to reviewing code, if you're open to implementing it.

I filed this issue to check in on what design approach would most easily get through code review. It was never a high enough priority for me to implement before I left Pinterest. Maybe you're the new me and the priority is higher now 🙂

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