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

[Proposal] Distributed outbound rate-limiting backed by state store #6935

Open
sujitdmello opened this issue Sep 18, 2023 · 15 comments
Open

Comments

@sujitdmello
Copy link

sujitdmello commented Sep 18, 2023

Dapr Maintainer Comment / Description

We should explore the creation of a distributed outbound rate limiter that can be backed by any state store. While the original request received was specific to the HTTP Binding - this could likely be done as HTTP middleware such that it can be used in other scenarios such as external service invocation as well.

Original Description

In a recent DAPR project, we implemented a distributed rate-limiting feature using DAPR state to throttle calls made by the HTTP binding to internal services, so as to honor service limits and budgets. This proved really key in the message processing scenario as it prevented these existing services from being overwhelmed by numerous API calls. The project was implemented in Java using the token-bucket library Bucket4j.

It seemed like this may be a useful addition to the HTTP Binding itself, so we did some spikes and created a sample approach and design proposal for integrating a distributed token bucket into DAPR using DAPR State. An existing rate limiting library in Golang was forked and a sample created. You can find it here along with a potential proposal for the HTTP binding (summarized below).

  • maxCallsPerPeriod - the maximum number of calls allowed per period
  • period - the period of time in which the maximum number of calls is allowed
  • rateLimitStore - the name of the DAPR state store to use for the rate-limiting
apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: http-person-service
spec:
  type: bindings.http
  version: v1
  metadata:
    - name: url
      value: http://localhost:8089/participant
    - name: maxCallsPerPeriod
      value: "10"
    - name: period
      value: "1s"
    - name: rateLimitStore
      value: "redis-rate-limit"
@berndverst
Copy link
Member

I like what you are getting at, however I think this makes more sense as a HTTP middleware or a new component type entirely. It cannot simply be added to the binding component.

Ideally that middleware can be used for both service invocation (external service invocation) as well as with the HTTP output binding.

This proposal is complex and actually will require Dapr runtime changes. We would not want the component implementation making adhoc requests to the state store or sidecar.

Ideally the rate distributed limiter can be applied to more than the HTTP binding, and ideally any Dapr State Store can be used.

Of course Redis shines in this scenario due to their handy key increment function - ironically Dapr does not use the key increment functionality, and Redis isn't transactional itself - so concurrent write problems might be experienced (which of course is generally acceptably even with distributed rate limiters).

I'll rename this issue and add on to the description, then will transfer it to the Dapr runtime to be considered there.

@berndverst berndverst changed the title [Proposal] Distributed outbound rate-limiting [Proposal] Distributed outbound rate-limiting backed by state store Sep 19, 2023
@berndverst berndverst transferred this issue from dapr/components-contrib Sep 19, 2023
@berndverst
Copy link
Member

cc @ItalyPaleAle

@ItalyPaleAle
Copy link
Contributor

This should be implemented with an internal actor. We're already considering using internal actors for other things such as middlewares (rate-limiting and OAuth2).

@sujitdmello
Copy link
Author

This should be implemented with an internal actor. We're already considering using internal actors for other things such as middlewares (rate-limiting and OAuth2).

FWIW, in our original Java implementation, we used the DAPR Actor framework to process messages and the Actor method was grabbing a token from the distributed bucket before making the HTTP binding call. That approached worked really well.

@ItalyPaleAle
Copy link
Contributor

Nice! Thanks for sharing @sujitdmello

Yes, that's the idea. As long as you can tolerate that the current rate-limiter state can be lost at any time (generally that is fine), you don't even need to persist the actor state's in the actor state store, and keeping the state in-memory is enough.

@sujitdmello
Copy link
Author

@stuartleeks and myself are planning to pick up this effort and do some analysis/design around it. Will share when available.

@stuartleeks
Copy link
Contributor

@ItalyPaleAle I just wanted to check in with some assumptions/questions....

Firstly, from this thread it seems that the intention is to implement this using internal actors.

Secondly, should this just apply to the HTTP binding and service invocation, or more bindings more generally?
I'm trying to work out whether the expectation is that this would add maxCallsPerPeriod and period properties to bindings:

apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: http-person-service
spec:
  type: bindings.http
  version: v1
  metadata:
    - name: url
      value: http://localhost:8089/participant
    - name: maxCallsPerPeriod
      value: "10"
    - name: period
      value: "1s"

... or be a new component type that can be applied to bindings via config:

apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: ratelimit
spec:
  type: middleware.binding.ratelimit
  version: v1
  metadata:
    - name: maxCallsPerPeriod
      value: 10
    - name: period
      value: "1s"

@ItalyPaleAle
Copy link
Contributor

Firstly, from this thread it seems that the intention is to implement this using internal actors.

That would be my strong preference, as it's the one that would leverage the existing Dapr building blocks best.

should this just apply to the HTTP binding and service invocation, or more bindings more generally

The idea would be to make this a feature of the ratelimit middleware: https://docs.dapr.io/reference/components-reference/supported-middleware/middleware-rate-limit/

@stuartleeks
Copy link
Contributor

One of the patterns I've seen on various occasions is building a service (serviceA) that needs to communicate with ServiceB and ServiceC, but only ServiceB has rate-limits.

With the HTTP middleware, we can configure it to apply to outbound calls via the appHttpPipeline configuration (link).

One challenge with this is that it applies to all outbound service calls.
So in the example above, there would not be a way to rate-limit outbound calls to just ServiceB.

@jjcollinge
Copy link
Contributor

hey @stuartleeks (long time!), I was quickly scanning this so may have missed some context, but just in case it's useful, this is a very old proposal I had for adding CEL expressions for middleware selectors #4167 to make them more flexible. Could this be used (with the right data exposed to the expression) so you can apply rate limits to specific services?

@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale Issues and PRs without response label Jan 19, 2024
@ItalyPaleAle ItalyPaleAle removed the stale Issues and PRs without response label Jan 19, 2024
@yaron2
Copy link
Member

yaron2 commented Jan 19, 2024

keepalive

@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale Issues and PRs without response label Mar 19, 2024
@ItalyPaleAle ItalyPaleAle added pinned and removed stale Issues and PRs without response pinned labels Mar 20, 2024
@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale Issues and PRs without response label May 19, 2024
@sujitdmello
Copy link
Author

Requesting this be kept alive as the need is certainly there but an approach needs to be decided.

  1. Extend existing rate-limiting middleware
  2. Use internal actors for token bucket

@dapr-bot dapr-bot removed the stale Issues and PRs without response label May 20, 2024
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

7 participants