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

feat: support retry settings and retryable codes in call context #1238

Merged
merged 37 commits into from
Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
8eeefdc
feat: support retry settings and retryable codes in call context
olavloite Nov 9, 2020
83c2f16
chore: fix formatting and license headers
olavloite Nov 9, 2020
aabd3b0
feat: use context retry settings for streaming calls
olavloite Nov 11, 2020
d73e7b3
Merge branch 'master' into context-retry-settings
olavloite Nov 11, 2020
c58f9c4
Merge branch 'master' into context-retry-settings
olavloite Jan 3, 2021
ed22eaf
fix: process review comments
olavloite Jan 7, 2021
80230e1
fix: missed one Thread.sleep
olavloite Jan 7, 2021
8ef6e6f
fix: fix style
olavloite Jan 7, 2021
19ed8d0
Merge branch 'master' into context-retry-settings
olavloite Jan 7, 2021
b2999d6
Merge branch 'master' into context-retry-settings
olavloite Jan 19, 2021
3eb9fa7
Merge branch 'master' into context-retry-settings
olavloite Jan 26, 2021
73b26c7
Merge branch 'master' into context-retry-settings
olavloite Feb 4, 2021
111f8f5
fix: address review comments
olavloite Feb 5, 2021
a58eb10
refactor: remove ContextAwareResultRetryAlgorithm
olavloite Feb 5, 2021
51b670b
refactor: remove ContextAwareTimedRetryAlgorithm
olavloite Feb 5, 2021
41fdc4e
fix: make package-private + add ignored
olavloite Feb 5, 2021
68b2c69
revert: revert to always using global settings for jittered
olavloite Feb 5, 2021
f6beeef
test: add tests for NoopRetryingContext
olavloite Feb 8, 2021
54f9f70
Merge branch 'master' into context-retry-settings
olavloite Feb 22, 2021
da9cc81
Merge branch 'master' into context-retry-settings
olavloite Feb 24, 2021
8c5090d
chore: cleanup 'this' usage
olavloite Feb 24, 2021
131da46
Merge branch 'master' into context-retry-settings
olavloite Mar 1, 2021
395d7cf
refactor: merge context aware classes with base classes
olavloite Mar 1, 2021
5c90e70
chore: cleanup and remove unnecessary methods
olavloite Mar 1, 2021
301069d
test: add safety margin as the retry settings are now jittered
olavloite Mar 1, 2021
f322549
docs: add javadoc
olavloite Mar 1, 2021
e0a61c5
chore: address review comments
olavloite Mar 5, 2021
0925ea2
Merge branch 'master' into context-retry-settings
olavloite Mar 5, 2021
e02f951
fix: address review comments
olavloite Mar 10, 2021
a0118c3
Merge branch 'context-retry-settings' of github.com:olavloite/gax-jav…
olavloite Mar 10, 2021
eafe239
fix: add tests + fix potential NPE
olavloite Mar 10, 2021
ffa05a1
Merge branch 'master' into context-retry-settings
olavloite Mar 10, 2021
8ae3280
fix: remove unused method + add tests
olavloite Mar 10, 2021
3c0d719
Merge branch 'context-retry-settings' of github.com:olavloite/gax-jav…
olavloite Mar 10, 2021
5da364a
test: add additional test calls
olavloite Mar 10, 2021
a42ea60
fix: deprecation
olavloite Mar 17, 2021
8eab974
Merge branch 'master' into context-retry-settings
olavloite Mar 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 137 additions & 43 deletions gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,17 @@
package com.google.api.gax.grpc;

import com.google.api.core.BetaApi;
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.rpc.StatusCode;
import com.google.api.gax.rpc.TransportChannel;
import com.google.api.gax.rpc.internal.Headers;
import com.google.api.gax.tracing.ApiTracer;
import com.google.api.gax.tracing.NoopApiTracer;
import com.google.auth.Credentials;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import io.grpc.CallCredentials;
import io.grpc.CallOptions;
import io.grpc.CallOptions.Key;
Expand All @@ -48,6 +51,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;
Expand All @@ -70,18 +74,36 @@ public final class GrpcCallContext implements ApiCallContext {
@Nullable private final Duration streamWaitTimeout;
@Nullable private final Duration streamIdleTimeout;
@Nullable private final Integer channelAffinity;
@Nullable private final RetrySettings retrySettings;
@Nullable private final ImmutableSet<StatusCode.Code> retryableCodes;
private final ImmutableMap<String, List<String>> extraHeaders;

/** Returns an empty instance with a null channel and default {@link CallOptions}. */
public static GrpcCallContext createDefault() {
return new GrpcCallContext(
null, CallOptions.DEFAULT, null, null, null, null, ImmutableMap.<String, List<String>>of());
null,
CallOptions.DEFAULT,
null,
null,
null,
null,
ImmutableMap.<String, List<String>>of(),
null,
null);
}

/** Returns an instance with the given channel and {@link CallOptions}. */
public static GrpcCallContext of(Channel channel, CallOptions callOptions) {
return new GrpcCallContext(
channel, callOptions, null, null, null, null, ImmutableMap.<String, List<String>>of());
channel,
callOptions,
null,
null,
null,
null,
ImmutableMap.<String, List<String>>of(),
null,
null);
}

private GrpcCallContext(
Expand All @@ -91,14 +113,18 @@ private GrpcCallContext(
@Nullable Duration streamWaitTimeout,
@Nullable Duration streamIdleTimeout,
@Nullable Integer channelAffinity,
ImmutableMap<String, List<String>> extraHeaders) {
ImmutableMap<String, List<String>> extraHeaders,
@Nullable RetrySettings retrySettings,
@Nullable Set<StatusCode.Code> retryableCodes) {
this.channel = channel;
this.callOptions = Preconditions.checkNotNull(callOptions);
this.timeout = timeout;
this.streamWaitTimeout = streamWaitTimeout;
this.streamIdleTimeout = streamIdleTimeout;
this.channelAffinity = channelAffinity;
this.extraHeaders = Preconditions.checkNotNull(extraHeaders);
this.retrySettings = retrySettings;
this.retryableCodes = retryableCodes == null ? null : ImmutableSet.copyOf(retryableCodes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this nullable? per Effective Java, empty collections are preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a difference in this case:

  • null means 'do not override the default settings`.
  • An empty set means 'override the default settings by disabling retrying'.

I've added that to the documentation of ApiCallContext#getRetryableCodes().

}

/**
Expand Down Expand Up @@ -160,7 +186,9 @@ public GrpcCallContext withTimeout(@Nullable Duration timeout) {
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders);
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

@Nullable
Expand All @@ -177,13 +205,15 @@ public GrpcCallContext withStreamWaitTimeout(@Nullable Duration streamWaitTimeou
}

return new GrpcCallContext(
channel,
callOptions,
timeout,
this.channel,
this.callOptions,
this.timeout,
streamWaitTimeout,
streamIdleTimeout,
channelAffinity,
extraHeaders);
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

@Override
Expand All @@ -194,25 +224,29 @@ public GrpcCallContext withStreamIdleTimeout(@Nullable Duration streamIdleTimeou
}

return new GrpcCallContext(
channel,
callOptions,
timeout,
streamWaitTimeout,
this.channel,
this.callOptions,
this.timeout,
this.streamWaitTimeout,
streamIdleTimeout,
channelAffinity,
extraHeaders);
this.channelAffinity,
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

@BetaApi("The surface for channel affinity is not stable yet and may change in the future.")
public GrpcCallContext withChannelAffinity(@Nullable Integer affinity) {
return new GrpcCallContext(
channel,
callOptions,
timeout,
streamWaitTimeout,
streamIdleTimeout,
this.channel,
this.callOptions,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
affinity,
extraHeaders);
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

@BetaApi("The surface for extra headers is not stable yet and may change in the future.")
Expand All @@ -222,13 +256,53 @@ public GrpcCallContext withExtraHeaders(Map<String, List<String>> extraHeaders)
ImmutableMap<String, List<String>> newExtraHeaders =
Headers.mergeHeaders(this.extraHeaders, extraHeaders);
return new GrpcCallContext(
channel,
callOptions,
timeout,
streamWaitTimeout,
streamIdleTimeout,
channelAffinity,
newExtraHeaders);
this.channel,
this.callOptions,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
newExtraHeaders,
this.retrySettings,
this.retryableCodes);
}

@Override
public RetrySettings getRetrySettings() {
return this.retrySettings;
}

@Override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between the retry settings here, and those in UnaryCallSettings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The RetrySettings in UnaryCallSettings are the default retry settings that are generated as part of the gapic client code generation. These settings are used if the client application does not set any RetrySettings on the CallContext that is used for an invocation of the RPC.
  • The RetrySettings that are set on the CallContext will override the settings in UnaryCallSettings for the invocation(s) where the CallContext is used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Furthermore, UnaryCallSettings are available at "client-creation", not at run time. As @olavloite mentioned, the UnaryCallSettings supply the default settings for a Callable at creation. The RetrySettings are extracted from the UnaryCallSettings and used throughout the chain independently.

public GrpcCallContext withRetrySettings(RetrySettings retrySettings) {
return new GrpcCallContext(
this.channel,
this.callOptions,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders,
retrySettings,
this.retryableCodes);
}

@Override
public Set<StatusCode.Code> getRetryableCodes() {
return this.retryableCodes;
}

@Override
public GrpcCallContext withRetryableCodes(Set<StatusCode.Code> retryableCodes) {
return new GrpcCallContext(
this.channel,
this.callOptions,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders,
this.retrySettings,
retryableCodes);
}

@Override
Expand Down Expand Up @@ -283,8 +357,18 @@ public ApiCallContext merge(ApiCallContext inputCallContext) {
newChannelAffinity = this.channelAffinity;
}

RetrySettings newRetrySettings = grpcCallContext.retrySettings;
if (newRetrySettings == null) {
newRetrySettings = this.retrySettings;
}

Set<StatusCode.Code> newRetryableCodes = grpcCallContext.retryableCodes;
if (newRetryableCodes == null) {
newRetryableCodes = this.retryableCodes;
}

ImmutableMap<String, List<String>> newExtraHeaders =
Headers.mergeHeaders(extraHeaders, grpcCallContext.extraHeaders);
Headers.mergeHeaders(this.extraHeaders, grpcCallContext.extraHeaders);

CallOptions newCallOptions =
grpcCallContext
Expand All @@ -303,7 +387,9 @@ public ApiCallContext merge(ApiCallContext inputCallContext) {
newStreamWaitTimeout,
newStreamIdleTimeout,
newChannelAffinity,
newExtraHeaders);
newExtraHeaders,
newRetrySettings,
newRetryableCodes);
}

/** The {@link Channel} set on this context. */
Expand Down Expand Up @@ -357,23 +443,27 @@ public GrpcCallContext withChannel(Channel newChannel) {
return new GrpcCallContext(
newChannel,
this.callOptions,
timeout,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders);
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

/** Returns a new instance with the call options set to the given call options. */
public GrpcCallContext withCallOptions(CallOptions newCallOptions) {
return new GrpcCallContext(
this.channel,
newCallOptions,
timeout,
this.timeout,
this.streamWaitTimeout,
this.streamIdleTimeout,
this.channelAffinity,
this.extraHeaders);
this.extraHeaders,
this.retrySettings,
this.retryableCodes);
}

public GrpcCallContext withRequestParamsDynamicHeaderOption(String requestParams) {
Expand Down Expand Up @@ -410,7 +500,9 @@ public int hashCode() {
streamWaitTimeout,
streamIdleTimeout,
channelAffinity,
extraHeaders);
extraHeaders,
retrySettings,
retryableCodes);
}

@Override
Expand All @@ -423,13 +515,15 @@ public boolean equals(Object o) {
}

GrpcCallContext that = (GrpcCallContext) o;
return Objects.equals(channel, that.channel)
&& Objects.equals(callOptions, that.callOptions)
&& Objects.equals(timeout, that.timeout)
&& Objects.equals(streamWaitTimeout, that.streamWaitTimeout)
&& Objects.equals(streamIdleTimeout, that.streamIdleTimeout)
&& Objects.equals(channelAffinity, that.channelAffinity)
&& Objects.equals(extraHeaders, that.extraHeaders);
return Objects.equals(this.channel, that.channel)
&& Objects.equals(this.callOptions, that.callOptions)
&& Objects.equals(this.timeout, that.timeout)
&& Objects.equals(this.streamWaitTimeout, that.streamWaitTimeout)
&& Objects.equals(this.streamIdleTimeout, that.streamIdleTimeout)
&& Objects.equals(this.channelAffinity, that.channelAffinity)
&& Objects.equals(this.extraHeaders, that.extraHeaders)
&& Objects.equals(this.retrySettings, that.retrySettings)
&& Objects.equals(this.retryableCodes, that.retryableCodes);
}

Metadata getMetadata() {
Expand Down
Loading