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 for fixed interval refill #39

Closed
vladimir-bukhtoyarov opened this issue Jun 22, 2017 · 16 comments
Closed

Add support for fixed interval refill #39

vladimir-bukhtoyarov opened this issue Jun 22, 2017 · 16 comments
Labels

Comments

@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Jun 22, 2017

In opposite to smooth refill which adds token as soon as possible, interval refill should regenerate tokens periodically.

Proposed API:

public static Refill fixedInterval(long tokens, Duration period) {
    // ...
}

public static Refill fixedInterval(Instant timeOfFirstRefill, long tokens, Duration period) {
     // ...
}
@vtahlani
Copy link

vtahlani commented Oct 5, 2017

When this feature will be available?

@vladimir-bukhtoyarov
Copy link
Collaborator Author

Most likely in January 2018

@vtahlani
Copy link

vtahlani commented Oct 5, 2017

I need this feature soon, can you try for oct end?

@vladimir-bukhtoyarov
Copy link
Collaborator Author

Yes, I can try, please hire me via upwork. I am agree to fixed price project 1000 US dollars.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

@vtahlani do you still need in this feature?

@vtahlani
Copy link

vtahlani commented Mar 29, 2018 via email

@vladimir-bukhtoyarov
Copy link
Collaborator Author

@vtahlani I have a little time frame to implement new release 3.2, #12 and #65 already implemented, you have the good chance that "fixed interval refill" will be included to 3.2, to achieve this you need to describe how do you plan to use this feature from business point of view. In other words I expect the descriptions of use-cases, which can be added to javadocs and wiki-pages.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

@vtahlani I remember that this feature will not be included to current release if you will not provide use cases.

@vtahlani
Copy link

vtahlani commented Apr 12, 2018

Thanks for implementing the feature.

With current smooth refill tokens are regenerated in millisecond so it is hard to say that a token was consumed as I get same value for X-RateLimit-Remaining header in subsequent requests if throttling number is big (e.g. 500 and it would become even more hard as this number become bigger). Also as tokens get refilled in milliseconds then in a minute I would be serving more request than my limit. I also faced another challenge what should be the value for X-RateLimit-Reset when should i say it would be rest.

With fixed rate interval, i believe, we can solve these problems - i would serve only request equivalent to my limit, i would be able to tell my client when it is going to reset.

Please let me know if you need more details.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

vladimir-bukhtoyarov commented Apr 12, 2018

Also as tokens get refilled in milliseconds then in a minute I would be serving more request than my limit.

This is because the token-bucket allows local burst by design. You can not avoid the burst, independently of smoot or interval refill style. For example:

  • Having 100 tokens per minute limit.
  • You decided to refresh limit each 1 minute.
  • In time 1 second before refreash bucket is full. User acquires all 100 token within this second. So bucket is empty.
  • After second, bucket is full again because of interval refill.
  • User come again and consume a next 100 tokens for next second.
  • In conclusion user consumed 200 tokens for 2 seconds.
    Redefine contract as 50 tokens per 30 second, would be single possible way to provide guarantee that user will be unable to consume more then 100 tokens per 1 minute, and stay in same time in token-bucket model which takes minimal memory footprint and does not store each event independently from each other.

I also faced another challenge what should be the value for X-RateLimit-Reset when should i say it would be rest.

It is interesting option, thanks for pointing out. I will add support for this for this option to the method tryConsumeAndReturnRemaining, it will work for both, smooth and interval refill.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

vladimir-bukhtoyarov commented Apr 12, 2018

The proposed API to specifying interval refill:

// 1000 tokens per minute, refill happens with one second interval
Bandwidth limit = Bandwidth.simple(1000, Duration.ofMinutes(1))
                                .withFixedRefillInterval(Duration.ofSecond(1));

Bucket bucket = Bucket4j.builder()
    .addBandwith(limit)
    .build();

I am not sure that withFixedRefillInterval is best name for method(resilience4j uses limitRefreshPeriod), feel free to propose more suitable.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

will be released soon

@vladimir-bukhtoyarov
Copy link
Collaborator Author

@vtahlani I have created dedicated issue #68 for X-RateLimit-Reset header

@vtahlani
Copy link

vtahlani commented Apr 16, 2018

Thanks for implementing the change, so if i change duration to one minute then refill would happen after one minute, right?

// 1000 tokens per minute, refill happens with after 1 minute interval
Bandwidth limit = Bandwidth.simple(1000, Duration.ofMinutes(1))
.withFixedRefillInterval(Duration.ofMinutes(1));

Bucket bucket = Bucket4j.builder()
.addBandwith(limit)
.build();

Now in every one minute i would be able to consume only 1000 toke, right?

@vladimir-bukhtoyarov
Copy link
Collaborator Author

if i change duration to one minute then refill would happen after one minute, right?

right

Now in every one minute i would be able to consume only 1000 toke, right?

In general case no, see my previous comment that describes the burst.

@vladimir-bukhtoyarov
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants