chore: bring backoff behavior inline with RetryHelper

BenWhitehead · BenWhitehead · commit 3e38109a7343 · 2025-03-14T17:38:20.000-04:00
Specifically, only return exhausted when the timeout has elapsed.
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Backoff.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Backoff.java
@@ -81,14 +81,16 @@ private Backoff(
    *
    * <ol>
    *   <li>If the existing {@link #cumulativeBackoff} + {@code elapsed} is >= {@link #timeout}
-   *   <li>If the existing {@link #cumulativeBackoff} + {@code elapsed} + {@code
-   *       nextBackoffDuration} is >= {@link #timeout}
    * </ol>
    */
   BackoffResult nextBackoff(Duration elapsed) {
     checkArgument(
         Durations.gtEq(elapsed, ZERO), "elapsed must be >= PT0S (%s >= %s)", elapsed, ZERO);
     Duration cumulativeAndElapsed = cumulativeBackoff.plus(elapsed);
+    if (Durations.gtEq(cumulativeAndElapsed, timeout)) {
+      cumulativeBackoff = cumulativeAndElapsed;
+      return BackoffResults.EXHAUSTED;
+    }
 
     Duration nextDelay =
         Duration.ofNanos(Math.round(previousBackoff.toNanos() * retryDelayMultiplier));
@@ -97,15 +99,10 @@ BackoffResult nextBackoff(Duration elapsed) {
     }
     Duration nextBackoffWithJitter = jitterer.jitter(nextDelay);
     Duration cappedBackoff = Durations.min(nextBackoffWithJitter, maxBackoff);
-    Duration newCumulativeElapsed = cumulativeAndElapsed.plus(cappedBackoff);
-    cumulativeBackoff = newCumulativeElapsed;
+    cumulativeBackoff = cumulativeAndElapsed.plus(cappedBackoff);
     previousBackoff = cappedBackoff;
 
-    if (Durations.gtEq(newCumulativeElapsed, timeout)) {
-      return BackoffResults.EXHAUSTED;
-    } else {
-      return BackoffDuration.of(cappedBackoff);
-    }
+    return BackoffDuration.of(cappedBackoff);
   }
 
   /**
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BackoffTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BackoffTest.java
@@ -18,6 +18,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static java.time.Duration.ZERO;
+import static java.time.Duration.ofSeconds;
 import static org.junit.Assert.assertThrows;
 
 import com.google.cloud.storage.Backoff.BackoffDuration;
@@ -83,15 +84,17 @@ public void simple() {
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(57)));
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(57)));
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(57)));
+    assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(57)));
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffResults.EXHAUSTED);
   }
 
   @Test
-  public void happyPath() {
+  public void backoffDuration_min_of_backoff_maxBackoff_remainingFromTimeout() {
     Backoff backoff = defaultBackoff();
 
     Duration elapsed = Duration.ofMinutes(6).plusSeconds(58);
-    assertThat(backoff.nextBackoff(elapsed)).isEqualTo(BackoffResults.EXHAUSTED);
+    assertThat(backoff.nextBackoff(elapsed)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(2)));
+    assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffResults.EXHAUSTED);
   }
 
   @Test
@@ -116,7 +119,8 @@ public void resetWorks() {
             .setRetryDelayMultiplier(2.0)
             .build();
 
-    assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(2)));
+    assertThat(backoff.nextBackoff(Duration.ofSeconds(4)))
+        .isEqualTo(BackoffDuration.of(Duration.ofSeconds(2)));
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffResults.EXHAUSTED);
     backoff.reset();
     assertThat(backoff.nextBackoff(Duration.ofSeconds(10))).isEqualTo(BackoffResults.EXHAUSTED);
@@ -139,8 +143,14 @@ public void onceExhaustedStaysExhaustedUntilReset() {
     assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffDuration.of(Duration.ofSeconds(2)));
   }
 
+  /**
+   * If a next computed backoff would exceed the timeout, truncate the backoff to the amount of time
+   * remaining until timeout.
+   *
+   * <p>This is primarily here to preserve behavior of {@link com.google.cloud.RetryHelper}.
+   */
   @Test
-  public void ifANextBackoffWouldExceedTheTimeoutItShouldBeConsideredExhausted() {
+  public void ifANextBackoffWouldExceedTheTimeoutTheBackoffDurationShouldBeTruncated() {
     Backoff backoff =
         Backoff.newBuilder()
             .setInitialBackoff(Duration.ofSeconds(2))
@@ -150,10 +160,9 @@ public void ifANextBackoffWouldExceedTheTimeoutItShouldBeConsideredExhausted() {
             .setRetryDelayMultiplier(2.0)
             .build();
 
-    assertThat(backoff.nextBackoff(Duration.ofSeconds(5)))
-        .isEqualTo(BackoffDuration.of(Duration.ofSeconds(2)));
-
-    assertThat(backoff.nextBackoff(Duration.ofSeconds(15))).isEqualTo(BackoffResults.EXHAUSTED);
+    assertThat(backoff.nextBackoff(Duration.ofSeconds(22)))
+        .isEqualTo(BackoffDuration.of(ofSeconds(2)));
+    assertThat(backoff.nextBackoff(ZERO)).isEqualTo(BackoffResults.EXHAUSTED);
   }
 
   @Test
