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

Commit

Permalink
feat: add ability to partially update retry settings (#993)
Browse files Browse the repository at this point in the history 
* feat: add ability to partially update retry settings

* Fix mutability of ServerStreamingCallSettings.retryCodes

* fix tests

* address feedback

* doc

* fix bad rebase
  • Loading branch information
@igorbernstein2
igorbernstein2 committed Apr 6, 2020
1 parent 909bf1b commit e57452a
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 48 deletions.
8 changes: 5 additions & 3 deletions gax-grpc/src/test/java/com/google/api/gax/grpc/SettingsTest.java
View file
Expand Up @@ -634,7 +634,7 @@ private static void assertIsReflectionEqual(
UnaryCallSettings.Builder<?, ?> builderB) {
assertIsReflectionEqual(path, builderA, builderB, new String[] {"retrySettingsBuilder"});
assertIsReflectionEqual(
path + ".getRetrySettings", builderA.getRetrySettings(), builderB.getRetrySettings());
path + ".retrySettingsBuilder", builderA.getRetrySettings(), builderB.getRetrySettings());
}

private static <RequestT, ResponseT> void assertIsReflectionEqual(
Expand Down Expand Up @@ -664,10 +664,12 @@ private static <RequestT, ResponseT> void assertIsReflectionEqual(
path,
builderA,
builderB,
new String[] {"retrySettings", "batchingDescriptor", "batchingSettings", "flowController"});
new String[] {
"retrySettingsBuilder", "batchingDescriptor", "batchingSettings", "flowController"
});

assertIsReflectionEqual(
path + ".getRetrySettings", builderA.getRetrySettings(), builderB.getRetrySettings());
path + ".retrySettingsBuilder", builderA.getRetrySettings(), builderB.getRetrySettings());
assertIsReflectionEqual(
path + ".getBatchingSettings",
builderA.getBatchingSettings(),
Expand Down
35 changes: 27 additions & 8 deletions gax/src/main/java/com/google/api/gax/rpc/ServerStreamingCallSettings.java
View file
Expand Up @@ -83,7 +83,7 @@ public final class ServerStreamingCallSettings<RequestT, ResponseT>

private ServerStreamingCallSettings(Builder<RequestT, ResponseT> builder) {
this.retryableCodes = ImmutableSet.copyOf(builder.retryableCodes);
this.retrySettings = builder.retrySettings;
this.retrySettings = builder.retrySettingsBuilder.build();
this.resumptionStrategy = builder.resumptionStrategy;
this.idleTimeout = builder.idleTimeout;
}
Expand Down Expand Up @@ -135,15 +135,15 @@ public static <RequestT, ResponseT> Builder<RequestT, ResponseT> newBuilder() {
public static class Builder<RequestT, ResponseT>
extends StreamingCallSettings.Builder<RequestT, ResponseT> {
@Nonnull private Set<StatusCode.Code> retryableCodes;
@Nonnull private RetrySettings retrySettings;
@Nonnull private RetrySettings.Builder retrySettingsBuilder;
@Nonnull private StreamResumptionStrategy<RequestT, ResponseT> resumptionStrategy;

@Nonnull private Duration idleTimeout;

/** Initialize the builder with default settings */
private Builder() {
this.retryableCodes = ImmutableSet.of();
this.retrySettings = RetrySettings.newBuilder().build();
this.retrySettingsBuilder = RetrySettings.newBuilder();
this.resumptionStrategy = new SimpleStreamResumptionStrategy<>();

this.idleTimeout = Duration.ZERO;
Expand All @@ -152,7 +152,7 @@ private Builder() {
private Builder(ServerStreamingCallSettings<RequestT, ResponseT> settings) {
super(settings);
this.retryableCodes = settings.retryableCodes;
this.retrySettings = settings.retrySettings;
this.retrySettingsBuilder = settings.retrySettings.toBuilder();
this.resumptionStrategy = settings.resumptionStrategy;

this.idleTimeout = settings.idleTimeout;
Expand Down Expand Up @@ -183,18 +183,37 @@ public Set<Code> getRetryableCodes() {
}

/**
* See the class documentation of {@link ServerStreamingCallSettings} for a description of what
* retrySettings do.
* Returns the underlying {@link RetrySettings.Builder}, which allows callers to augment the
* existing {@link RetrySettings}.
*/
public RetrySettings.Builder retrySettings() {
return this.retrySettingsBuilder;
}

/**
* Replaces the {@link RetrySettings} for the associated {@link ServerStreamingCallable}.
*
* <p>When using the method, make sure that the {@link RetrySettings} are complete. For example,
* the following code will disable retries because the retry delay is not set:
*
* <pre>{@code
* stubSettings.setRetrySettings(
* RetrySettings.newBuilder()
* .setTotalTimeout(Duration.ofSeconds(10)
* );
* }</pre>
*
* @see #retrySettings()
*/
public Builder<RequestT, ResponseT> setRetrySettings(@Nonnull RetrySettings retrySettings) {
Preconditions.checkNotNull(retrySettings);
this.retrySettings = retrySettings;
this.retrySettingsBuilder = retrySettings.toBuilder();
return this;
}

@Nonnull
public RetrySettings getRetrySettings() {
return retrySettings;
return retrySettingsBuilder.build();
}

/** Disables retries and sets the overall timeout. */
Expand Down
39 changes: 33 additions & 6 deletions gax/src/main/java/com/google/api/gax/rpc/UnaryCallSettings.java
View file
Expand Up @@ -31,7 +31,6 @@

import com.google.api.core.InternalExtensionOnly;
import com.google.api.gax.retrying.RetrySettings;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.util.Set;
Expand Down Expand Up @@ -85,7 +84,7 @@ public Builder<RequestT, ResponseT> toBuilder() {

protected UnaryCallSettings(Builder<RequestT, ResponseT> builder) {
this.retryableCodes = ImmutableSet.copyOf(builder.retryableCodes);
this.retrySettings = builder.retrySettings;
this.retrySettings = builder.retrySettingsBuilder.build();
}

/**
Expand All @@ -98,11 +97,11 @@ protected UnaryCallSettings(Builder<RequestT, ResponseT> builder) {
public static class Builder<RequestT, ResponseT> {

private Set<StatusCode.Code> retryableCodes;
private RetrySettings retrySettings;
private RetrySettings.Builder retrySettingsBuilder;

protected Builder() {
retryableCodes = Sets.newHashSet();
retrySettings = RetrySettings.newBuilder().build();
retrySettingsBuilder = RetrySettings.newBuilder();
}

protected Builder(UnaryCallSettings<RequestT, ResponseT> unaryCallSettings) {
Expand Down Expand Up @@ -130,9 +129,32 @@ public UnaryCallSettings.Builder<RequestT, ResponseT> setRetryableCodes(
return this;
}

/**
* Returns the underlying {@link RetrySettings.Builder}, which allows callers to augment the
* existing {@link RetrySettings}.
*/
public RetrySettings.Builder retrySettings() {
return this.retrySettingsBuilder;
}

/**
* Replaces the {@link RetrySettings} for the associated {@link UnaryCallable}.
*
* <p>When using the method, make sure that the {@link RetrySettings} are complete. For example,
* the following code will disable retries because the retry delay is not set:
*
* <pre>{@code
* stubSettings.setRetrySettings(
* RetrySettings.newBuilder()
* .setTotalTimeout(Duration.ofSeconds(10)
* );
* }</pre>
*
* @see #retrySettings()
*/
public UnaryCallSettings.Builder<RequestT, ResponseT> setRetrySettings(
RetrySettings retrySettings) {
this.retrySettings = Preconditions.checkNotNull(retrySettings);
this.retrySettingsBuilder = retrySettings.toBuilder();
return this;
}

Expand Down Expand Up @@ -162,8 +184,13 @@ public Set<StatusCode.Code> getRetryableCodes() {
return this.retryableCodes;
}

/**
* Returns an immutable {@link RetrySettings} currently set in this Builder.
*
* <p>Unlike {@link #retrySettings()}, objects returned by this method are frozen in time.
*/
public RetrySettings getRetrySettings() {
return this.retrySettings;
return this.retrySettingsBuilder.build();
}

/**
Expand Down
31 changes: 26 additions & 5 deletions gax/src/test/java/com/google/api/gax/rpc/BatchingCallSettingsTest.java
View file
Expand Up @@ -40,6 +40,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
import org.threeten.bp.Duration;

@RunWith(JUnit4.class)
public class BatchingCallSettingsTest {
Expand Down Expand Up @@ -82,7 +83,17 @@ public void testBuilder() {
BatchingSettings.newBuilder().setElementCountThreshold(1L).build();
FlowController flowController = Mockito.mock(FlowController.class);
Set<StatusCode.Code> retryCodes = Sets.newHashSet(Code.UNAVAILABLE);
RetrySettings retrySettings = RetrySettings.newBuilder().build();
RetrySettings retrySettings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(5))
.setMaxRetryDelay(Duration.ofSeconds(1))
.setRetryDelayMultiplier(2)
.setInitialRpcTimeout(Duration.ofMillis(100))
.setMaxRpcTimeout(Duration.ofMillis(200))
.setRpcTimeoutMultiplier(1.1)
.setJittered(true)
.setMaxAttempts(10)
.build();

builder.setBatchingSettings(batchingSettings);
builder.setFlowController(flowController);
Expand All @@ -93,15 +104,15 @@ public void testBuilder() {
Truth.assertThat(builder.getBatchingSettings()).isSameInstanceAs(batchingSettings);
Truth.assertThat(builder.getFlowController()).isSameInstanceAs(flowController);
Truth.assertThat(builder.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(builder.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(builder.getRetrySettings()).isEqualTo(retrySettings);

BatchingCallSettings settings = builder.build();

Truth.assertThat(settings.getBatchingDescriptor()).isSameInstanceAs(batchingDescriptor);
Truth.assertThat(settings.getBatchingSettings()).isSameInstanceAs(batchingSettings);
Truth.assertThat(settings.getFlowController()).isSameInstanceAs(flowController);
Truth.assertThat(settings.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(settings.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(settings.getRetrySettings()).isEqualTo(retrySettings);
}

@Test
Expand All @@ -116,7 +127,17 @@ public void testBuilderFromSettings() throws Exception {
BatchingSettings.newBuilder().setElementCountThreshold(1L).build();
FlowController flowController = Mockito.mock(FlowController.class);
Set<StatusCode.Code> retryCodes = Sets.newHashSet(Code.UNAVAILABLE);
RetrySettings retrySettings = RetrySettings.newBuilder().build();
RetrySettings retrySettings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(5))
.setMaxRetryDelay(Duration.ofSeconds(1))
.setRetryDelayMultiplier(2)
.setInitialRpcTimeout(Duration.ofMillis(100))
.setMaxRpcTimeout(Duration.ofMillis(200))
.setRpcTimeoutMultiplier(1.1)
.setJittered(true)
.setMaxAttempts(10)
.build();

builder.setBatchingSettings(batchingSettings);
builder.setFlowController(flowController);
Expand All @@ -130,7 +151,7 @@ public void testBuilderFromSettings() throws Exception {
Truth.assertThat(newBuilder.getBatchingSettings()).isSameInstanceAs(batchingSettings);
Truth.assertThat(newBuilder.getFlowController()).isSameInstanceAs(flowController);
Truth.assertThat(newBuilder.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(newBuilder.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(newBuilder.getRetrySettings()).isEqualTo(retrySettings);
}

@Test
Expand Down
33 changes: 27 additions & 6 deletions gax/src/test/java/com/google/api/gax/rpc/PagedCallSettingsTest.java
View file
Expand Up @@ -38,6 +38,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
import org.threeten.bp.Duration;

@RunWith(JUnit4.class)
public class PagedCallSettingsTest {
Expand Down Expand Up @@ -72,18 +73,28 @@ public void testBuilder() {
PagedCallSettings.newBuilder(pagedListResponseFactory);

Set<StatusCode.Code> retryCodes = Sets.newHashSet(Code.UNAVAILABLE);
RetrySettings retrySettings = RetrySettings.newBuilder().build();
RetrySettings retrySettings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(5))
.setMaxRetryDelay(Duration.ofSeconds(1))
.setRetryDelayMultiplier(2)
.setInitialRpcTimeout(Duration.ofMillis(100))
.setMaxRpcTimeout(Duration.ofMillis(200))
.setRpcTimeoutMultiplier(1.1)
.setJittered(true)
.setMaxAttempts(10)
.build();

builder.setRetryableCodes(retryCodes);
builder.setRetrySettings(retrySettings);

Truth.assertThat(builder.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(builder.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(builder.getRetrySettings()).isEqualTo(retrySettings);

PagedCallSettings settings = builder.build();

Truth.assertThat(settings.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(settings.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(settings.getRetrySettings()).isEqualTo(retrySettings);
}

@Test
Expand All @@ -95,18 +106,28 @@ public void testBuilderFromSettings() throws Exception {
PagedCallSettings.newBuilder(pagedListResponseFactory);

Set<StatusCode.Code> retryCodes = Sets.newHashSet(Code.UNAVAILABLE);
RetrySettings retrySettings = RetrySettings.newBuilder().build();
RetrySettings retrySettings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(5))
.setMaxRetryDelay(Duration.ofSeconds(1))
.setRetryDelayMultiplier(2)
.setInitialRpcTimeout(Duration.ofMillis(100))
.setMaxRpcTimeout(Duration.ofMillis(200))
.setRpcTimeoutMultiplier(1.1)
.setJittered(true)
.setMaxAttempts(10)
.build();

builder.setRetryableCodes(retryCodes);
builder.setRetrySettings(retrySettings);

Truth.assertThat(builder.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(builder.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(builder.getRetrySettings()).isEqualTo(retrySettings);

PagedCallSettings settings = builder.build();
PagedCallSettings.Builder newBuilder = settings.toBuilder();

Truth.assertThat(newBuilder.getRetryableCodes().size()).isEqualTo(1);
Truth.assertThat(newBuilder.getRetrySettings()).isSameInstanceAs(retrySettings);
Truth.assertThat(newBuilder.getRetrySettings()).isEqualTo(retrySettings);
}
}

0 comments on commit e57452a

Please sign in to comment.