Make firstFastRetry override minBackoff (#3)

graze · Mar 15, 2017 · 87016ce · 87016ce
1 parent fbe9361
commit 87016ce
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/src/RetryStrategy/ExponentialBackoffStrategy.php b/src/RetryStrategy/ExponentialBackoffStrategy.php
@@ -31,7 +31,7 @@ class ExponentialBackoffStrategy extends AbstractRetryStrategy implements RetryS
     protected $multiplier = 1000;
 
     /**
-     * Whether to retry immediately in the first instance (or after minBackoff if set).
+     * Whether to retry immediately in the first instance (overriding minBackoff if set).
      *
      * @param bool $firstFastRetry
      */
@@ -85,8 +85,12 @@ public function setMultiplier($multiplier)
      */
     public function calculateBackoffPeriod($retryCount)
     {
-        $offset = $this->firstFastRetry ? $this->multiplier : 0;
-        $backoff = $this->minBackoff + rand(0, $this->multiplier * pow(2, $retryCount) - $offset);
+        if ($this->firstFastRetry && $retryCount === 0) {
+            return 0;
+        }
+
+        $upperBound = $this->multiplier * (pow(2, $retryCount) - 1);
+        $backoff = $this->minBackoff + rand(0, $upperBound);
 
         if ($this->maxBackoff) {
             $backoff = min($backoff, $this->maxBackoff);

diff --git a/tests/unit/RetryStrategy/ExponentialBackoffStrategyTest.php b/tests/unit/RetryStrategy/ExponentialBackoffStrategyTest.php
@@ -23,16 +23,14 @@ class ExponentialBackoffStrategyTest extends TestCase
     /**
      * @dataProvider backoffPeriodDataProvider
      * @param int $maxRetries
-     * @param bool $firstFastRetry
      * @param bool $multiplier
      * @param int $minBackoff
      * @param int|null $maxBackoff
      * @param int $retryCount
      */
-    public function testBackoffPeriodIsAboveMinimum($maxRetries, $firstFastRetry, $multiplier, $minBackoff, $maxBackoff, $retryCount)
+    public function testBackoffPeriodIsAboveMinimum($maxRetries, $multiplier, $minBackoff, $maxBackoff, $retryCount)
     {
         $strategy = new ExponentialBackoffStrategy($maxRetries);
-        $strategy->setFirstFastRetry($firstFastRetry);
         $strategy->setMultiplier($multiplier);
         $strategy->setMinBackoff($minBackoff);
         $strategy->setMaxBackoff($maxBackoff);
@@ -50,19 +48,38 @@ public function testMinBackoffAboveMaxBackoff()
         $strategy->setMaxBackoff(4);
     }
 
+    /**
+     * Test that the backoff period is zero for the first retry when firstFastRetry is set to true.
+     *
+     * @dataProvider backoffPeriodDataProvider
+     * @param int $maxRetries
+     * @param bool $multiplier
+     * @param int $minBackoff
+     * @param int|null $maxBackoff
+     */
+    public function testFirstFastRetry($maxRetries, $multiplier, $minBackoff, $maxBackoff)
+    {
+        $strategy = new ExponentialBackoffStrategy($maxRetries);
+        $strategy->setFirstFastRetry(true);
+        $strategy->setMultiplier($multiplier);
+        $strategy->setMinBackoff($minBackoff);
+        $strategy->setMaxBackoff($maxBackoff);
+
+        $this->assertEquals(0, $strategy->calculateBackoffPeriod(0));
+    }
+
     /**
      * @return array
      */
     public function backoffPeriodDataProvider()
     {
         $maxRetries = [0, 1, 10];
-        $firstFastRetry = [true, false];
         $multiplier = [1000];
         $minBackoff = [0, 10];
         $maxBackoff = [10, 1000];
         $retryCount = [0, 10];
 
         $cartesianProduct = new CartesianProduct();
-        return $cartesianProduct->build([$maxRetries, $firstFastRetry, $multiplier, $minBackoff, $maxBackoff, $retryCount]);
+        return $cartesianProduct->build([$maxRetries, $multiplier, $minBackoff, $maxBackoff, $retryCount]);
     }
 }