feat: non-retryable RPCs use totalTimeout (#1149)

Author: noahdietz

gax-grpc/src/test/java/com/google/api/gax/grpc/TimeoutTest.java

@@ -102,9 +102,9 @@ public void testNonRetryUnarySettings() {
     // Verify that the gRPC channel used the CallOptions with our custom timeout of ~2 Days.
     assertThat(callOptionsUsed.getDeadline()).isNotNull();
     assertThat(callOptionsUsed.getDeadline())
-        .isGreaterThan(Deadline.after(DEADLINE_IN_SECONDS - 1, TimeUnit.SECONDS));
+        .isGreaterThan(Deadline.after(DEADLINE_IN_DAYS - 1, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getDeadline())
-        .isLessThan(Deadline.after(DEADLINE_IN_SECONDS, TimeUnit.SECONDS));
+        .isLessThan(Deadline.after(DEADLINE_IN_DAYS, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getAuthority()).isEqualTo(CALL_OPTIONS_AUTHORITY);
   }
 
@@ -126,9 +126,9 @@ public void testNonRetryUnarySettingsWithoutInitialRpcTimeout() {
     // Verify that the gRPC channel used the CallOptions with our custom timeout of ~2 Days.
     assertThat(callOptionsUsed.getDeadline()).isNotNull();
     assertThat(callOptionsUsed.getDeadline())
-        .isGreaterThan(Deadline.after(DEADLINE_IN_MINUTES - 1, TimeUnit.MINUTES));
+        .isGreaterThan(Deadline.after(DEADLINE_IN_DAYS - 1, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getDeadline())
-        .isLessThan(Deadline.after(DEADLINE_IN_MINUTES, TimeUnit.MINUTES));
+        .isLessThan(Deadline.after(DEADLINE_IN_DAYS, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getAuthority()).isEqualTo(CALL_OPTIONS_AUTHORITY);
   }
 
@@ -175,9 +175,9 @@ public void testNonRetryServerStreamingSettings() {
     // Verify that the gRPC channel used the CallOptions with our custom timeout of ~2 Days.
     assertThat(callOptionsUsed.getDeadline()).isNotNull();
     assertThat(callOptionsUsed.getDeadline())
-        .isGreaterThan(Deadline.after(DEADLINE_IN_SECONDS - 1, TimeUnit.SECONDS));
+        .isGreaterThan(Deadline.after(DEADLINE_IN_DAYS - 1, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getDeadline())
-        .isLessThan(Deadline.after(DEADLINE_IN_SECONDS, TimeUnit.SECONDS));
+        .isLessThan(Deadline.after(DEADLINE_IN_DAYS, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getAuthority()).isEqualTo(CALL_OPTIONS_AUTHORITY);
   }
 
@@ -199,9 +199,9 @@ public void testNonRetryServerStreamingSettingsWithoutInitialRpcTimeout() {
     // Verify that the gRPC channel used the CallOptions with our custom timeout of ~2 Days.
     assertThat(callOptionsUsed.getDeadline()).isNotNull();
     assertThat(callOptionsUsed.getDeadline())
-        .isGreaterThan(Deadline.after(DEADLINE_IN_MINUTES - 1, TimeUnit.MINUTES));
+        .isGreaterThan(Deadline.after(DEADLINE_IN_DAYS - 1, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getDeadline())
-        .isLessThan(Deadline.after(DEADLINE_IN_MINUTES, TimeUnit.MINUTES));
+        .isLessThan(Deadline.after(DEADLINE_IN_DAYS, TimeUnit.DAYS));
     assertThat(callOptionsUsed.getAuthority()).isEqualTo(CALL_OPTIONS_AUTHORITY);
   }
 

gax/src/main/java/com/google/api/gax/rpc/Callables.java

@@ -39,7 +39,6 @@
 import com.google.api.gax.retrying.ScheduledRetryingExecutor;
 import com.google.api.gax.retrying.StreamingRetryAlgorithm;
 import java.util.Collection;
-import org.threeten.bp.Duration;
 
 /**
  * Class with utility methods to create callable objects using provided settings.
@@ -58,12 +57,11 @@ public static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> retrying(
       ClientContext clientContext) {
 
     if (areRetriesDisabled(callSettings.getRetryableCodes(), callSettings.getRetrySettings())) {
-      // When retries are disabled, the choose a timeout from the retrying settings to use as the
-      // timeout for the single rpc call.
+      // When retries are disabled, the total timeout can be treated as the rpc timeout.
       return innerCallable.withDefaultCallContext(
           clientContext
               .getDefaultCallContext()
-              .withTimeout(singleRpcCallTimeout(callSettings.getRetrySettings())));
+              .withTimeout(callSettings.getRetrySettings().getTotalTimeout()));
     }
 
     RetryAlgorithm<ResponseT> retryAlgorithm =
@@ -84,12 +82,11 @@ public static <RequestT, ResponseT> ServerStreamingCallable<RequestT, ResponseT>
       ClientContext clientContext) {
 
     if (areRetriesDisabled(callSettings.getRetryableCodes(), callSettings.getRetrySettings())) {
-      // When retries are disabled, the choose a timeout from the retrying settings to use as the
-      // timeout for the single rpc call.
+      // When retries are disabled, the total timeout can be treated as the rpc timeout.
       return innerCallable.withDefaultCallContext(
           clientContext
               .getDefaultCallContext()
-              .withTimeout(singleRpcCallTimeout(callSettings.getRetrySettings())));
+              .withTimeout(callSettings.getRetrySettings().getTotalTimeout()));
     }
 
     StreamingRetryAlgorithm<Void> retryAlgorithm =
@@ -105,23 +102,6 @@ public static <RequestT, ResponseT> ServerStreamingCallable<RequestT, ResponseT>
         innerCallable, retryingExecutor, callSettings.getResumptionStrategy());
   }
 
-  /*
-   * Returns the default Duration for a single RPC call given a Callable's RetrySettings. This
-   * configuration most likely does not belong in retry settings and may change in the future.
-   */
-  static Duration singleRpcCallTimeout(RetrySettings retrySettings) {
-    // Prefer initialRpcTimeout, then maxRpcTimeout, then totalTimeout
-    Duration duration = retrySettings.getInitialRpcTimeout();
-    if (!duration.isZero()) {
-      return duration;
-    }
-    duration = retrySettings.getMaxRpcTimeout();
-    if (!duration.isZero()) {
-      return duration;
-    }
-    return retrySettings.getTotalTimeout();
-  }
-
   @BetaApi("The surface for streaming is not stable yet and may change in the future.")
   public static <RequestT, ResponseT> ServerStreamingCallable<RequestT, ResponseT> watched(
       ServerStreamingCallable<RequestT, ResponseT> callable,