google / guava

Google core libraries for Java
Apache License 2.0
49.68k stars 10.8k forks source link

SmoothWarmingUp may not reach max rate in some case? #3552

Open kulallwang opened 4 years ago

kulallwang commented 4 years ago

RateLimiter rateLimiter = RateLimiter.create(10, 500, TimeUnit.MILLISECONDS); for (int i = 0; i < 100; i++) { boolean ret = rateLimiter.tryAcquire(); log.info("tryAcquire result:{}", ret); Thread.sleep(120); }

The log is follow: 15:16:02.816 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:true 15:16:02.938 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:false 15:16:03.060 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:true 15:16:03.186 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:false 15:16:03.311 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:true 15:16:03.433 [main] INFO com.xiaohongshu.kula.test.guava.Test - tryAcquire result:false

Set permitsPerSecond to 10. Every time call tryAcquire, sleep 120ms. The expect tryAcquire result is always true after a while, but the log show that tryAcquire result is true,false,true,false forever. The max rate can reach only 5 per second.

xhd731568849 commented 4 years ago

mark

OLPMO commented 4 years ago

I made some debug and found the reason of the bug.But I have no idea to fix the bug unless I break original structrue of the code. This bug was resulted by the initial value of the storedPermits in SmoothWarmingUp.Although not any stable interval passed by ,the the initial value of the storedPermits equals to maxPermits which can ensure the throttling time follow the image of function below(form high to low).

             ^ throttling
             |
       cold  +                            /
    interval |                          / .
             |                            /    .
             |                          /     .   ← "warmup period" is the area of the trapezoid between
             |                        /       .     thresholdPermits and maxPermits
             |                      /         .
             |                    /          .
             |                  /            .
   stable +---------- /  WARM .
   nterval |                 .   UP  .
             |                 . PERIOD.
             |                 .       .
           0 +----------+-------+--------------→ storedPermits
             0 thresholdPermits maxPermits

However,if the consumption of permit is slower than the production of permit,the SmoothWarmingUp would always run in warm up period regardless of whether it is in the warmup phase. Last but not least,I would be very grateful if someone can give me suggestions for fixing this bug. @kevinb9n @xhd731568849 @kulallwang

OLPMO commented 4 years ago
       RateLimiter rateLimiter = RateLimiter.create(10, 500, TimeUnit.MILLISECONDS);
        rateLimiter.tryAcquire(5);
        Thread.sleep(520);
        for (int i = 0; i < 20; i++) {
            boolean ret = rateLimiter.tryAcquire();
            System.out.println("tryAcquire result:" + ret);
            Thread.sleep(120);
        }

The max rate can reach 10 per second in this case.`

tksarul commented 3 weeks ago

I am facing the same issue. Can you share your fixed code?