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

Rate Limiting works incorrectly if PeriodTimespan value is greater than Period #1590

Open
sergio-str opened this issue Jul 28, 2022 · 10 comments · May be fixed by #1592
Open

Rate Limiting works incorrectly if PeriodTimespan value is greater than Period #1590

sergio-str opened this issue Jul 28, 2022 · 10 comments · May be fixed by #1592
Assignees
Labels
2023 Annual 2023 release accepted Bug or feature would be accepted as a PR or is being worked on bug Identified as a potential bug Rate Limiting Ocelot feature: Rate Limiting
Milestone

Comments

@sergio-str
Copy link

sergio-str commented Jul 28, 2022

Expected Behavior

Rate Limiting works as documented for such parameter values:
Period: 1 second
PeriodTimeSpan: 30 seconds
Limit: 100

Scenario:
Request flow, 10-20 requests per sec, should be allowed.

Status: not equal to 429

Actual Behavior / Motivation for New Feature

With the same parameter values from Expected Behavior Ocelot returns status 429 aka "Rate limit exceeded".
It seems this issue caused by code in RateLimitCore.cs lines 35-43

// entry has not expired
if (entry.Value.Timestamp + TimeSpan.FromSeconds(rule.PeriodTimespan) >= DateTime.UtcNow)
{
    // increment request count
    var totalRequests = entry.Value.TotalRequests + 1;

    // deep copy
    counter = new RateLimitCounter(entry.Value.Timestamp, totalRequests);
}

Ocelot increments request counter if request has been received in interval [FirstRequestTime : FirstRequestTime + PeriodTimeSpan]
According to the documentation counter should be incremented if request is in interval [FirstRequestTime : FirstRequestTime + Period].

Steps to Reproduce the Problem

  1. Set Rate Limiting configuration parameters:
  • Period: 1 second
  • PeriodTimeSpan: 30 seconds
  • Limit: 100
  1. Generate a consequent request flow (10-20 requests per second) for more than 12 seconds
  2. Ensure Ocelot returns status 429

Specifications

@sergio-str sergio-str linked a pull request Aug 1, 2022 that will close this issue
@sergio-str sergio-str changed the title RateLimiting works incorrect if PeriodTimespan value way more than Period RateLimiting works incorrect if PeriodTimespan value way more exceeds Period Aug 9, 2022
@raman-m raman-m added bug Identified as a potential bug needs validation Issue has not been replicated or verified yet in progress Someone is working on the issue. Could be someone on the team or off. waiting Waiting for answer to question or feedback from issue raiser labels May 11, 2023
@raman-m
Copy link
Member

raman-m commented May 11, 2023

Hi Sergii!

Do not you mind if we collaborate on the issue description first?

@raman-m raman-m changed the title RateLimiting works incorrect if PeriodTimespan value way more exceeds Period Rate Limiting works incorrectly if PeriodTimespan value is greater than Period May 11, 2023
@raman-m
Copy link
Member

raman-m commented May 11, 2023

Check the issue Title please!
Is this title correct?

@raman-m
Copy link
Member

raman-m commented May 11, 2023

Sergii,
I've changed Expected Behavior description.
Could you confirm it's correct please?
I need to know we are on the same page.

@sergio-str
Copy link
Author

I think yes. But give me a little time, please, to dive into the context. Also I apologize for inacurate title.

@raman-m
Copy link
Member

raman-m commented May 11, 2023

So, bug description has been changed and updated.
Could you review it plz?

@raman-m raman-m linked a pull request May 11, 2023 that will close this issue
@raman-m raman-m pinned this issue May 11, 2023
@raman-m
Copy link
Member

raman-m commented May 12, 2023

@sergio-str Hi Sergii!
Do you accept current description?

@raman-m raman-m removed the waiting Waiting for answer to question or feedback from issue raiser label May 29, 2023
@raman-m
Copy link
Member

raman-m commented Jun 1, 2023

@sergio-str Sergii

I would like your checking this bug for the latest build/version which is 19.0.2 and .NET 7.
Your reported Specifications info with "Ocelot Version: 18" is quite outdated.
Please, use the latest package and/or just use top commits from develop branch version!

Do not you mind if I change Ocelot Version to 19.0.2 in Specifications paragraph?
That's how I and team can accept this bug after our common validation.
And the team will not accept bugs for old versions, because of outdating.

@raman-m raman-m added accepted Bug or feature would be accepted as a PR or is being worked on and removed needs validation Issue has not been replicated or verified yet in progress Someone is working on the issue. Could be someone on the team or off. labels Sep 7, 2023
@raman-m
Copy link
Member

raman-m commented Sep 7, 2023

The issue has been accepted due to open PR #1592

@MithunChopda
Copy link

Also, is it possible to configure RateLimiting globally? Looks like the limits work only when specified at Route level. It's inconvenient to configure Limits for large number of routes.
Limits configured at Route level can then take precedence over Global configuration.

@raman-m
Copy link
Member

raman-m commented Nov 3, 2023

@MithunChopda commented on Nov 3

Hi Mithun!
No, impossible! You have to configure Rate Limiter for each route.
Look at this PR #1659 for global Headers Transformation. If you will be inspired by this PR, you could open new PR especially for global Rate Liming.

@raman-m raman-m added the 2023 Annual 2023 release label Nov 29, 2023
@raman-m raman-m added this to the December'23 milestone Nov 29, 2023
@raman-m raman-m added the Rate Limiting Ocelot feature: Rate Limiting label Dec 4, 2023
@raman-m raman-m unpinned this issue Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2023 Annual 2023 release accepted Bug or feature would be accepted as a PR or is being worked on bug Identified as a potential bug Rate Limiting Ocelot feature: Rate Limiting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants