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 support of rate limit and timeout #1347

Merged
merged 54 commits into from
Aug 11, 2022
Merged

Conversation

bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Jul 4, 2022

close #946
close #1184
close #1275
related to #338

Additions to the traffic shaping plugin:

  • Rate limit - If you want to rate limit requests to a subgraphs or to the router itself.
  • Timeout: - Set a timeout to subgraphs and router requests.

example of configuration:

traffic_shaping:
  router: # Rules applied to requests from clients to the router
    rate_limit: # Accept a maximum of 10 requests per 5 secs. Excess requests must be rejected.
      capacity: 10
      interval: 5s
    timeout: 50sec # If a request to the router takes more than 50 secs then cancel the request (30 sec by default)
  subgraphs: # Rules applied to requests from the router to individual subgraphs
    products:
      rate_limit: # Accept a maximum of 10 requests per 5 secs. Excess requests must be rejected.
        capacity: 10
        interval: 5s
      timeout: 50sec # If a request to the subgraph 'products' takes more than 50 secs then cancel the request (30 sec by default)

Rate limit algorithm is coming from https://dev.to/satrobit/rate-limiting-using-the-sliding-window-algorithm-5fjn and https://medium.com/@intmainco/design-a-scalable-rate-limiting-algorithm-system-design-nlogn-895abba44b77

bnjjj added 4 commits July 4, 2022 17:31
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
@bnjjj bnjjj self-assigned this Jul 4, 2022
Geoffroy Couprie and others added 23 commits July 5, 2022 10:14
…graphql/router into bnjjj/feat_rate_limit_timeout
Signed-off-by: Benjamin Coenen <[email protected]>
…graphql/router into bnjjj/feat_rate_limit_timeout
Signed-off-by: Benjamin Coenen <[email protected]>
…graphql/router into bnjjj/feat_rate_limit_timeout
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
@bnjjj bnjjj marked this pull request as ready for review July 8, 2022 12:33
@bnjjj bnjjj requested a review from StephenBarlow as a code owner July 8, 2022 12:33
@lennyburdette
Copy link
Contributor

lennyburdette commented Jul 29, 2022

in the changelog it says

rate_limit: # Accept a maximum of 10 requests per 5 secs from a given client. Excess requests must be rejected.

how is the client identified? is that configurable? totally fine if these are future features, i was just curious. (are we planning to eventually support something like https://docs.konghq.com/hub/kong-inc/rate-limiting/?)

Signed-off-by: Benjamin Coenen <[email protected]>
@bnjjj
Copy link
Contributor Author

bnjjj commented Aug 1, 2022

@lennyburdette it's mistake it's not rate limited for a specific client. But yes if it's a needed/asked feature we could implement that kind of behavior in the future.

@prasek
Copy link
Contributor

prasek commented Aug 2, 2022

Since this is a global rate limit suggest and we'll likely want to support per client rate limiting in the future suggest using router.global_rate_limit

@bnjjj bnjjj requested a review from Geal August 8, 2022 14:43
@Geal
Copy link
Contributor

Geal commented Aug 8, 2022

the lower (millisecond) and upper(u64::MAX milliseconds) limits of the rate limit window should be documented, no?

Signed-off-by: Benjamin Coenen <[email protected]>
@bnjjj bnjjj requested a review from Geal August 9, 2022 12:50
@bnjjj bnjjj merged commit eab203d into main Aug 11, 2022
@bnjjj bnjjj deleted the bnjjj/feat_rate_limit_timeout branch August 11, 2022 14:54
@Geal Geal mentioned this pull request Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add request_timeout to traffic shaping plugin Subgraph request timeouts Simple rate limiting
7 participants