From 61624519fb04ac9e2d0c3651b65cc3a3cf40a26f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B0=B7=E6=BA=AA?= <guxi99@gmail.com>
Date: Wed, 21 Oct 2020 15:09:25 +0800
Subject: [PATCH] Fixed unexpected behavior in retry budget for `hyperf/retry`.
 (#2693)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: retry budget bug

* fix: fix travis

* Update CHANGELOG-2.0.md

Co-authored-by: 李铭昕 <715557344@qq.com>
---
 CHANGELOG-2.0.md                    |  1 +
 src/retry/src/RetryBudget.php       | 16 +++++++++++++---
 src/retry/tests/RetryBudgetTest.php | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG-2.0.md b/CHANGELOG-2.0.md
index 72b01dc95..d5b1a97cd 100644
--- a/CHANGELOG-2.0.md
+++ b/CHANGELOG-2.0.md
@@ -8,6 +8,7 @@
 
 - [#2680](https://github.com/hyperf/hyperf/pull/2680) Fixed Type error for `CastsValue`, because `$isSynchronized` don't have default value.
 - [#2680](https://github.com/hyperf/hyperf/pull/2680) Fixed default value in `$items` will be replaced by `__construct` for `CastsValue`.
+- [#2693](https://github.com/hyperf/hyperf/pull/2693) Fixed unexpected behavior in retry budget for `hyperf/retry`.
 - [#2695](https://github.com/hyperf/hyperf/pull/2695) Fixed method `Container::define()` does not works when the class has been resolved.
 
 ## Optimized
diff --git a/src/retry/src/RetryBudget.php b/src/retry/src/RetryBudget.php
index 15eea954d..89ced885c 100644
--- a/src/retry/src/RetryBudget.php
+++ b/src/retry/src/RetryBudget.php
@@ -42,11 +42,17 @@ class RetryBudget implements RetryBudgetInterface
      */
     private $timerId;
 
+    /**
+     * @var float|int
+     */
+    private $maxToken;
+
     public function __construct(int $ttl, int $minRetriesPerSec, float $percentCanRetry)
     {
         $this->ttl = $ttl;
         $this->minRetriesPerSec = $minRetriesPerSec;
         $this->percentCanRetry = $percentCanRetry;
+        $this->maxToken = ($this->minRetriesPerSec / $this->percentCanRetry) * $this->ttl;
         $this->budget = new SplQueue();
     }
 
@@ -70,9 +76,7 @@ class RetryBudget implements RetryBudgetInterface
             for ($i = 0; $i < $this->minRetriesPerSec / $this->percentCanRetry; ++$i) {
                 $this->produce();
             }
-            while (! $this->budget->isEmpty()
-                && $this->budget->top() <= microtime(true)
-            ) {
+            while ($this->hasOverflown()) {
                 $this->budget->dequeue();
             }
         });
@@ -98,4 +102,10 @@ class RetryBudget implements RetryBudgetInterface
         $t = microtime(true) + $this->ttl;
         $this->budget->push($t);
     }
+
+    public function hasOverflown(): bool
+    {
+        return (! $this->budget->isEmpty() && $this->budget->bottom() <= microtime(true))
+            || $this->budget->count() > $this->maxToken;
+    }
 }
diff --git a/src/retry/tests/RetryBudgetTest.php b/src/retry/tests/RetryBudgetTest.php
index 5369b1c18..51811b700 100644
--- a/src/retry/tests/RetryBudgetTest.php
+++ b/src/retry/tests/RetryBudgetTest.php
@@ -68,5 +68,20 @@ class RetryBudgetTest extends TestCase
         $this->assertTrue($budget->consume());
         $this->assertTrue($budget->consume());
         $this->assertTrue(! $budget->consume());
+
+        // Retry budget should never have more than 1 token in this test
+        $budget = new RetryBudget(
+            1,
+            1,
+            1
+        );
+        $budget->init();
+        $ref = new \ReflectionClass(RetryBudget::class);
+        $prop = $ref->getProperty('budget');
+        $prop->setAccessible(true);
+        System::sleep(1.2);
+        $this->assertLessThanOrEqual(1, $prop->getValue($budget)->count());
+        System::sleep(1.2);
+        $this->assertLessThanOrEqual(1, $prop->getValue($budget)->count());
     }
 }