Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

fix: non-retryable RPCs use totalTimeout #1149

Merged
merged 1 commit into from Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions gax-grpc/src/test/java/com/google/api/gax/grpc/TimeoutTest.java
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
28 changes: 4 additions & 24 deletions gax/src/main/java/com/google/api/gax/rpc/Callables.java
Expand Up @@ -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.
Expand All @@ -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 =
Expand All @@ -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 =
Expand All @@ -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,
Expand Down