kezhuw commented on code in PR #2264:
URL: https://github.com/apache/zookeeper/pull/2264#discussion_r2120001122
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/BlueThrottle.java:
##########
@@ -327,6 +327,9 @@ public synchronized boolean checkLimit(int need) {
if (diff > fillTime) {
int refill = (int) (diff * fillCount / fillTime);
tokens = Math.min(tokens + refill, maxTokens);
+ if (tokens < 0) {
+ tokens = maxTokens;
+ }
Review Comment:
```suggestion
long refill = diff * fillCount / fillTime;
tokens = (int) Math.min(tokens + refill, maxTokens);
if (tokens < 0) {
tokens = maxTokens;
LOG.error("Throttle config values {}({}) and {}({}) are
insane and cause long integer overflow after {}ms",
CONNECTION_THROTTLE_FILL_TIME, fillTime,
CONNECTION_THROTTLE_FILL_COUNT, fillCount, diff);
}
```
> fillCount is 3 and fillTime is 1，this case is triggered when there is no
request for more than 8 days.
In case of `long` and above settings, it will take 90 million years to
overflow. Even for `fillCount 30000, filleTime 1`(which is insane) it takes 9
thousand years.
So I think we could combine [the two
methods](https://github.com/apache/zookeeper/pull/2264#discussion_r2113804414)
together to solve both overflow and insane/incorrect configurations.
--
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: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
