flyrain commented on code in PR #3433:
URL: https://github.com/apache/polaris/pull/3433#discussion_r2692662071
##########
runtime/service/src/main/java/org/apache/polaris/service/ratelimiter/TokenBucket.java:
##########
@@ -51,11 +51,11 @@ public synchronized boolean tryAcquire() {
long t = instantSource.millis();
long millisPassed = Math.subtractExact(t, lastTokenGenerationMillis);
lastTokenGenerationMillis = t;
- tokens = Math.min(maxTokens, tokens + ((long) (millisPassed *
tokensPerMilli)));
+ tokens = Math.min((double) maxTokens, tokens + (millisPassed *
tokensPerMilli));
Review Comment:
That's a valid point, but only happen in an extreme case. For IEEE 754
double precision, the relative epsilon is ~2.2e-16. The issue would occur when:
`tokens + increment == tokens`, which happens when increment < tokens * epsilon
For example:
- If maxTokens = 1e15 and tokensPerMilli = 0.001, the increment would be
lost
- But for practical values like maxTokens = 10000, adding 0.001 will
always register
The Java long can reach up to 1e13, which is at the edge of losing the
increment. Mostly likely it won't happen, users may just disable the rate
limiter in that case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
