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

Feature/refill rate limiter (rate limiter only) #1370

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gkatzioura
Copy link

@gkatzioura gkatzioura commented Mar 13, 2021

This is a pull request for the ticket #1138

As discussed on #1139 this pull request is only for the Refill Rate Limiter implementation. Thus it contains only the rate limiter implementation, its config, unit tests and benchmarks.
The registry and the code to make it feasible to have a more generic registry will be on another branch.

How it works

The main concept is to have a bucket of permissions allocated with a max number of permissions (more permissions than that cannot be accumulated or served).

An integer keeps track of the permissions accumulated over time but the max value is limited by the capacity described above. Also another number acts as a time index - up to which time we calculated the permissions.

At startup we can have some permissions pre-assigned.

When a permission is requested the first step is to subtract the existing permissions from the request ones

If the result is positive the request can be served immediately with the permissions we already have
and the state will be updated with the remaining permissions (the result of the previous calculation).

If the result is negative, we need to see if enough time has passed to fulfil those requests.
We calculate the difference of the current Nanos minus the nanos the state we had back when we last updated (time index).

The time that has passed is caped by the time needed to fill up the maximum number of requests. So no matter how much time has passed we shall be limited by the max permissions (so no overflow or extra permissions given).

If we are on a max permission scenario (we have accumulated the maximum of permissions), we remove the extra permits needed from the max permissions and we update the state with the permissions left and set the current nano time to the time index.

If we have not accumulated the max permissions, we check If the time we have accumulated is bigger or equal to the time needed to serve the extra requests.

If the right time has been accumulated we fulfil the request and the time index is updated with 0 permissions and add to the time-index the time that was needed to accumulate the permissions.

If the time needed is exceeds the accumulated time then we extract the difference and this is the time to wait for the permissions to be released. The state will have 0 permissions and the time index will be at the future. It will be the current index plus the time needed as a whole to fulfil the extra permissions.

If the time needed will cause a timeout no change to the state will happen.

Benchmarks

The benchmarks (can be download from here)

The menckmark-all.*.txt files contain the benchmarks with all the rate limiters on the branch (atomic,refill,semaphore).

Benchmark                                                                         Mode       Cnt      Score     Error   Units
RateLimiterBenchmark.atomicPermission                                            thrpt        20      9.017 ±   0.275  ops/us
RateLimiterBenchmark.refillPermission                                            thrpt        20     11.079 ±   0.071  ops/us
RateLimiterBenchmark.semaphoreBasedPermission                                    thrpt        20     19.173 ±   0.223  ops/us
RateLimiterBenchmark.atomicPermission                                             avgt        20      0.217 ±   0.004   us/op
RateLimiterBenchmark.refillPermission                                             avgt        20      0.180 ±   0.002   us/op
RateLimiterBenchmark.semaphoreBasedPermission                                     avgt        20      0.104 ±   0.001   us/op
RateLimiterBenchmark.atomicPermission                                           sample  13853142      1.452 ±   0.051   us/op

Since the atomic rate limiter had some changes (slightly changed) I did some benchmarks before and after only with the atomic rate limiter.

The atomic-before.*.txt files contain the benchmarks with the atomic rate limiter on the master branch.

Benchmark                                                         Mode      Cnt      Score   Error   Units
RateLimiterBenchmark.atomicPermission                            thrpt       20     12.658 ± 0.135  ops/us
RateLimiterBenchmark.atomicPermission                             avgt       20      0.159 ± 0.002   us/op
RateLimiterBenchmark.atomicPermission                           sample  9312235      1.239 ± 0.056   us/op

The atomic-after.*.txt files contain the benchmarks with the atomic rate limiter on the pr branch.

Benchmark                                                         Mode      Cnt      Score    Error   Units
RateLimiterBenchmark.atomicPermission                            thrpt       20     12.539 ±  0.098  ops/us
RateLimiterBenchmark.atomicPermission                             avgt       20      0.167 ±  0.013   us/op
RateLimiterBenchmark.atomicPermission                           sample  9233578      1.296 ±  0.053   us/op

Due to the Refill Rate limiter sharing same code parts with the atomic rate limiter I have some benchmarks only with the refill rate limiter.

Benchmark                                                         Mode       Cnt      Score   Error   Units
RateLimiterBenchmark.refillPermission                            thrpt        20     15.513 ± 0.081  ops/us
RateLimiterBenchmark.refillPermission                             avgt        20      0.134 ± 0.008   us/op
RateLimiterBenchmark.refillPermission                           sample  11322845      1.153 ± 0.048   us/op

@gkatzioura
Copy link
Author

Hi @RobWin does it make sense to reboot this?

@RobWin
Copy link
Member

RobWin commented Apr 15, 2021

Hi, I'm sorry that this PR is not getting merged faster. I'm currently struggling with work, family home schooling, health. That's why my time is scarce. I still try to answer all issues and review PRs. But I hesitate with this PR, because the RateLimiter is @storozhukBM baby.

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

Successfully merging this pull request may close these issues.

None yet

2 participants