Re: [PR] ZOOKEEPER-4933: Fixed the issue where all connections were rejected w… [zookeeper]

["kezhuw (via GitHub)" <gi...@apache.org>]

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-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org