Skip to content

Commit

Permalink
[Finagle] Backoff.takeUntil doesn't call the underlying backoff's next()
Browse files Browse the repository at this point in the history
Problem:

When a new `c.t.f.Backoff`  is created by wrapping an existing instance with `c.t.f.takeUntil` it incorrectly always only returns the underlying Backoff's first backoff

Solution:

Instead it should iterate over the underlying backoff by calling `next()` on it

JIRA Issues: RTC-2278

Differential Revision: https://phabricator.twitter.biz/D1099285
  • Loading branch information
Vishal Palla authored and jenkins committed Sep 19, 2023
1 parent caa7c77 commit ee01085
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ object Backoff {
else Backoff.empty.duration
def next: Backoff =
if (count < maxCumulativeBackoff)
new TakeWhile(backoff, count + duration, maxCumulativeBackoff)
new TakeWhile(backoff.next, count + duration, maxCumulativeBackoff)
else Backoff.empty.next
def isExhausted: Boolean = if (count < maxCumulativeBackoff) false else true
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.twitter.finagle

import com.twitter.conversions.DurationOps._
import com.twitter.finagle.Backoff.{DecorrelatedJittered, EqualJittered, ExponentialJittered}
import com.twitter.finagle.Backoff.DecorrelatedJittered
import com.twitter.finagle.Backoff.EqualJittered
import com.twitter.finagle.Backoff.ExponentialJittered
import com.twitter.finagle.util.Rng
import com.twitter.util.Duration
import org.scalacheck.Gen
Expand Down Expand Up @@ -184,11 +186,14 @@ class BackoffTest extends AnyFunSuite with ScalaCheckDrivenPropertyChecks {
test("takeUntil") {
var backoff: Backoff = Backoff.linear(1.second, 1.second).takeUntil(9.seconds)
var sumBackoff: Duration = Duration.Zero
val backoffs: ArrayBuffer[Duration] = ArrayBuffer.empty[Duration]
while (sumBackoff < 9.seconds) {
assert(!backoff.isExhausted)
sumBackoff += backoff.duration
backoffs += backoff.duration
backoff = backoff.next
}
assert(sumBackoff == 9.seconds)
assert(backoffs == ArrayBuffer[Duration](1.seconds, 2.seconds, 3.seconds, 4.seconds))
assert(backoff.isExhausted)
}

Expand Down

0 comments on commit ee01085

Please sign in to comment.