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

getAvailableTokensPerEachBandwidth() #279

Closed
kiril-toshev opened this issue Jul 25, 2022 · 8 comments
Closed

getAvailableTokensPerEachBandwidth() #279

kiril-toshev opened this issue Jul 25, 2022 · 8 comments

Comments

@kiril-toshev
Copy link

Hello,
I am using bucket4j with Redis:

<dependencies>
    <dependency>
        <groupId>com.giffing.bucket4j.spring.boot.starter</groupId>
        <artifactId>bucket4j-spring-boot-starter</artifactId>
        <version>0.7.0</version>
    </dependency>
    <dependency>
        <groupId>org.redisson</groupId>
        <artifactId>redisson-spring-boot-starter</artifactId>
        <version>3.17.5</version>
    </dependency>
<dependencies>

When I configure a bucket with the following bandwidths:

Bandwidth.classic(4, Refill.intervally(4, Duration.ofMinutes(20)))
Bandwidth.classic(10, Refill.intervally(10, Duration.ofMinutes(60)))

and consume 1 token multiple times using the verbose API:

VerboseResult<ConsumptionProbe> verboseResult = bucket.asVerbose().tryConsumeAndReturnRemaining(1);

I get incorrect results reported by the getAvailableTokensPerEachBandwidth() method as shown bellow:

First consumption (the bucket is created and stored in Redis for the first time):

verboseResult.getValue().getRemainingTokens() -> 3
verboseResult.getDiagnostics().getAvailableTokensPerEachBandwidth() -> [3, 9]

Second consumption:

verboseResult.getValue().getRemainingTokens() -> 2
verboseResult.getDiagnostics().getAvailableTokensPerEachBandwidth() -> [3, 9]

Third consumption:

verboseResult.getValue().getRemainingTokens() -> 1
verboseResult.getDiagnostics().getAvailableTokensPerEachBandwidth() -> [2, 8]

Fourth consumption:

verboseResult.getValue().getRemainingTokens() -> 0
verboseResult.getDiagnostics().getAvailableTokensPerEachBandwidth() -> [1, 7]
@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Jul 25, 2022

Hello,

First of all, it worth to mention that bucket4j-spring-boot-starter is separated project and is maintained by another guys.
I am unable to reproduce the problem with my code, you can run the proofs from this commit 9481bfa

The test behaves as expected:

Remaining tokens = 3
Tokens per bandwidth = [3, 9]

Remaining tokens = 2
Tokens per bandwidth = [2, 8]

Remaining tokens = 1
Tokens per bandwidth = [1, 7]

Remaining tokens = 0
Tokens per bandwidth = [0, 6]

Looks that you are using https://github.com/MarcGiffing/bucket4j-spring-boot-starter/blob/master/bucket4j-spring-boot-starter/src/main/java/com/giffing/bucket4j/spring/boot/starter/config/cache/redis/RedisProxyManager.java from bucket4j-spring-boot-starter and there are problems in implementation. Currently I have no time to debug and find out which concrete problem in this class, but it worth to mention that since bucket4j release 7.6.0 there is no reasons to continue using of RedisProxyManager from bucket4j-spring-boot-starter because support for Letucce, Jedis and SpringDataRedis was directly added to Bucket4j.

@iliyanvelchevio
Copy link

@vladimir-bukhtoyarov I managed to reproduce the issue using JCache, here is the PR with the configuration I used.

@iliyanvelchevio
Copy link

@vladimir-bukhtoyarov That change fixes the issue for this particular case.

@vladimir-bukhtoyarov
Copy link
Collaborator

@iliyanvelchevio hello, I am going to close both merge requests without merge because of I will provide more optimal fix, as well as more optimal test.

@iliyanvelchevio
Copy link

@vladimir-bukhtoyarov Great, I just wanted to give you more information on how to reproduce the issue and a potential fix.

@vladimir-bukhtoyarov
Copy link
Collaborator

The problem was in inacurate implementation of VerboseCommand, it was not working correctly when implementation of MutableBucketEntry does not cache the result of get/set in memory, it is at least all JCache and similar integrations like Hazelcast/Ignite/Coherence.

Has been fixed by commit 4c3ea89
Will be released at weekend with version 8.0.1, maybe faster, it depends from progress on bucket4j/bucket4j.github.io#1

vladimir-bukhtoyarov added a commit that referenced this issue Jul 31, 2022
@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Jul 31, 2022

Fix has been included to release 8.0.1
Also, groupId has been changed from com.github.vladimir-bukhtoyarov to com.bucket4j.

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

No branches or pull requests

3 participants