Skip to content

[RateLimiter] Fix bucket size reduced when previously created with bigger size #58601

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

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

Orkin
Copy link
Contributor

@Orkin Orkin commented Oct 18, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #
License MIT
foo:
      policy: 'token_bucket'
      limit: 1000
      rate: { interval: '15 minutes', amount: 5 }
      cache_pool: rate_limiter.cache_pool
      lock_factory: 'lock.rate_limiter.factory'

rate_limiter.cache_pool => it's a persistent cache pool like redis

When using previously this configuration and consume the token bucket with 1 token it was save on the storage with 999 tokens available.

If you update the configuration with a lower token limit

foo:
      policy: 'token_bucket'
      limit: 10
      rate: { interval: '15 minutes', amount: 5 }
      cache_pool: rate_limiter.cache_pool
      lock_factory: 'lock.rate_limiter.factory'

You can consume 999 tokens before triggering the bucket limit without flushing the cache. The purpose of this PR is to update and use the new configuration limit.

@carsonbot carsonbot added this to the 6.4 milestone Oct 18, 2024
@Orkin Orkin changed the title [RateLimiter] Fix bucket size reduce when previously created with bigger size [RateLimiter] Fix bucket size reduced when previously created with bigger size Oct 18, 2024
@fabpot
Copy link
Member

fabpot commented Nov 9, 2024

Thank you @Orkin.

@fabpot fabpot merged commit c15a195 into symfony:6.4 Nov 9, 2024
9 of 10 checks passed
This was referenced Nov 13, 2024
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.

4 participants