diff --git a/stream-android-core/src/main/java/io/getstream/android/core/api/model/retry/StreamRetryPolicy.kt b/stream-android-core/src/main/java/io/getstream/android/core/api/model/retry/StreamRetryPolicy.kt index 84018bc8..3745e6dd 100644 --- a/stream-android-core/src/main/java/io/getstream/android/core/api/model/retry/StreamRetryPolicy.kt +++ b/stream-android-core/src/main/java/io/getstream/android/core/api/model/retry/StreamRetryPolicy.kt @@ -62,9 +62,9 @@ private constructor( /** * Creates an **exponential back-off** policy. * - * Delay grows according to: + * Delay doubles on every retry: * ```text - * nextDelay = prevDelay + retryIndex × backoffStepMillis + * nextDelay = backoffStepMillis × 2^(retryIndex - 1) * ``` * * then clamped to `[backoffStepMillis, maxBackoffMillis]`. @@ -75,6 +75,7 @@ private constructor( * retry 1 → 250 ms * retry 2 → 500 ms * retry 3 → 1 000 ms + * retry 4 → 2 000 ms * … * ``` * @@ -100,10 +101,16 @@ private constructor( maxBackoffMills = maxBackoffMillis, initialDelayMillis = initialDelayMillis, giveUpFunction = giveUp, - nextBackOffDelayFunction = { retry, prev -> - (prev + retry * backoffStepMillis) - .coerceAtMost(maxBackoffMillis) - .coerceIn(backoffStepMillis, maxBackoffMillis) + nextBackOffDelayFunction = { retry, _ -> + val shift = (retry - 1).coerceIn(0, Long.SIZE_BITS - 1) + // Largest step that still fits under the cap once shifted; comparing + // against it keeps `shl` from overflowing on high retry counts. + val largestShiftableStep = maxBackoffMillis shr shift + if (backoffStepMillis > largestShiftableStep) { + maxBackoffMillis + } else { + backoffStepMillis shl shift + } }, ) .also { it.requireValid() } diff --git a/stream-android-core/src/test/java/io/getstream/android/core/api/model/retry/StreamRetryPolicyTest.kt b/stream-android-core/src/test/java/io/getstream/android/core/api/model/retry/StreamRetryPolicyTest.kt index bf98ecb3..08330646 100644 --- a/stream-android-core/src/test/java/io/getstream/android/core/api/model/retry/StreamRetryPolicyTest.kt +++ b/stream-android-core/src/test/java/io/getstream/android/core/api/model/retry/StreamRetryPolicyTest.kt @@ -59,17 +59,18 @@ class StreamRetryPolicyTest { fun `exponential backoff delay calculation increases exponentially`() { val policy = StreamRetryPolicy.exponential(backoffStepMillis = 100, initialDelayMillis = 0) - // First retry: prev=0 + retry=1 * 100 = 100 + // Retry n waits 100 * 2^(n-1) val delay1 = policy.nextBackOffDelayFunction(1, 0) assertEquals(100, delay1) - // Second retry: prev=100 + retry=2 * 100 = 300 val delay2 = policy.nextBackOffDelayFunction(2, delay1) - assertEquals(300, delay2) + assertEquals(200, delay2) - // Third retry: prev=300 + retry=3 * 100 = 600 val delay3 = policy.nextBackOffDelayFunction(3, delay2) - assertEquals(600, delay3) + assertEquals(400, delay3) + + val delay4 = policy.nextBackOffDelayFunction(4, delay3) + assertEquals(800, delay4) } @Test @@ -77,11 +78,28 @@ class StreamRetryPolicyTest { val policy = StreamRetryPolicy.exponential(backoffStepMillis = 1000, maxBackoffMillis = 3000) - // Should exceed max: 0 + 10 * 1000 = 10000, but capped at 3000 + // Should exceed max: 1000 * 2^9 = 512_000, but capped at 3000 val delay = policy.nextBackOffDelayFunction(10, 0) assertEquals(3000, delay) } + @Test + fun `exponential backoff delay does not overflow on very high retry counts`() { + val policy = + StreamRetryPolicy.exponential(backoffStepMillis = 1000, maxBackoffMillis = 30_000) + + assertEquals(30_000, policy.nextBackOffDelayFunction(64, 0)) + assertEquals(30_000, policy.nextBackOffDelayFunction(Int.MAX_VALUE, 0)) + } + + @Test + fun `exponential backoff delay ignores the previous delay`() { + val policy = StreamRetryPolicy.exponential(backoffStepMillis = 100) + + assertEquals(400, policy.nextBackOffDelayFunction(3, 0)) + assertEquals(400, policy.nextBackOffDelayFunction(3, 12_345)) + } + @Test fun `exponential backoff delay is clamped to minimum`() { val policy = StreamRetryPolicy.exponential(backoffStepMillis = 100) diff --git a/stream-android-core/src/test/java/io/getstream/android/core/internal/processing/StreamRetryProcessorImplTest.kt b/stream-android-core/src/test/java/io/getstream/android/core/internal/processing/StreamRetryProcessorImplTest.kt index 4af4cdfb..35e25360 100644 --- a/stream-android-core/src/test/java/io/getstream/android/core/internal/processing/StreamRetryProcessorImplTest.kt +++ b/stream-android-core/src/test/java/io/getstream/android/core/internal/processing/StreamRetryProcessorImplTest.kt @@ -82,6 +82,32 @@ class StreamRetryProcessorImplTest { assertEquals(100, dispatcher.scheduler.currentTime) } + @Test + fun `exponential policy doubles the elapsed delay between attempts`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val scope = TestScope(dispatcher) + + val policy = + StreamRetryPolicy.exponential( + minRetries = 1, + maxRetries = 5, + backoffStepMillis = 250, + initialDelayMillis = 0, + ) + + val counter = AtomicInteger() + val job = + scope.async { retry.retry(policy) { counter.incrementAndGet().also { error("Boom") } } } + + dispatcher.scheduler.advanceUntilIdle() + + assertTrue(job.await().isFailure) + assertEquals(5, counter.get()) + // Four waits between five attempts: 250 + 500 + 1000 + 2000. + // The pre-fix policy accumulated instead, giving 250 + 750 + 1500 + 2500 = 5000. + assertEquals(3750, dispatcher.scheduler.currentTime) + } + @Test fun `returns failure after exhausting maxRetries`() = runTest { val policy = StreamRetryPolicy.linear(maxRetries = 2, minRetries = 1)