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

Commit

Permalink
Keep default http json executor the same, use boolean to track
Browse files Browse the repository at this point in the history 
deprecated executor
  • Loading branch information
@mutianf
mutianf committed Jul 27, 2021
1 parent d323f11 commit ea12683
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 42 deletions.
14 changes: 3 additions & 11 deletions gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java
View file
Expand Up @@ -37,15 +37,14 @@
import com.google.api.core.BetaApi;
import com.google.api.core.SettableApiFuture;
import com.google.api.gax.core.BackgroundResource;
import com.google.api.gax.core.InstantiatingExecutorProvider;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;

Expand All @@ -54,11 +53,7 @@
public class ManagedHttpJsonChannel implements HttpJsonChannel, BackgroundResource {
private static final JsonFactory JSON_FACTORY = GsonFactory.getDefaultInstance();
private static final ExecutorService DEFAULT_EXECUTOR =
Executors.newCachedThreadPool(
new ThreadFactoryBuilder()
.setDaemon(true)
.setNameFormat("http-default-executor-%d")
.build());
InstantiatingExecutorProvider.newBuilder().build().getExecutor();

private final Executor executor;
private final String endpoint;
Expand Down Expand Up @@ -158,7 +153,7 @@ public static class Builder {
private Builder() {}

public Builder setExecutor(Executor executor) {
this.executor = executor;
this.executor = Preconditions.checkNotNull(executor);
return this;
}

Expand All @@ -179,9 +174,6 @@ public Builder setHttpTransport(HttpTransport httpTransport) {

public ManagedHttpJsonChannel build() {
Preconditions.checkNotNull(endpoint);
if (executor == null) {
this.executor = DEFAULT_EXECUTOR;
}
return new ManagedHttpJsonChannel(
executor, endpoint, jsonFactory, headerEnhancers, httpTransport);
}
Expand Down
20 changes: 5 additions & 15 deletions gax/src/main/java/com/google/api/gax/rpc/ClientContext.java
View file
Expand Up @@ -169,10 +169,6 @@ public static ClientContext create(StubSettings settings) throws IOException {
ExecutorProvider backgroundExecutorProvider = settings.getBackgroundExecutorProvider();
final ScheduledExecutorService backgroundExecutor = backgroundExecutorProvider.getExecutor();

ExecutorProvider executorProvider = settings.getExecutorProvider();
final ScheduledExecutorService executor =
executorProvider == null ? null : executorProvider.getExecutor();

Credentials credentials = settings.getCredentialsProvider().getCredentials();

if (settings.getQuotaProjectId() != null) {
Expand All @@ -184,11 +180,11 @@ public static ClientContext create(StubSettings settings) throws IOException {
}

TransportChannelProvider transportChannelProvider = settings.getTransportChannelProvider();
// After needsExecutor and StubSettings#setExecutor are deprecated, transport channel executor
// can only be set from TransportChannelProvider#withExecutor directly, and a provider will
// have a default executor if it needs one.
if (transportChannelProvider.needsExecutor() && executor != null) {
transportChannelProvider = transportChannelProvider.withExecutor(executor);
// After needsExecutor and StubSettings#setExecutorProvider are deprecated, transport channel
// executor can only be set from TransportChannelProvider#withExecutor directly, and a provider
// will have a default executor if it needs one.
if (transportChannelProvider.needsExecutor() && settings.getExecutorProvider() != null) {
transportChannelProvider = transportChannelProvider.withExecutor(backgroundExecutor);
}
Map<String, String> headers = getHeadersFromSettings(settings);
if (transportChannelProvider.needsHeaders()) {
Expand Down Expand Up @@ -242,12 +238,6 @@ public static ClientContext create(StubSettings settings) throws IOException {
if (watchdogProvider != null && watchdogProvider.shouldAutoClose()) {
backgroundResources.add(watchdog);
}
if (executorProvider != null
&& executor != null
&& executorProvider.shouldAutoClose()
&& executor != backgroundExecutor) {
backgroundResources.add(new ExecutorAsBackgroundResource(executor));
}

return newBuilder()
.setBackgroundResources(backgroundResources.build())
Expand Down
32 changes: 16 additions & 16 deletions gax/src/main/java/com/google/api/gax/rpc/StubSettings.java
View file
Expand Up @@ -64,7 +64,6 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {

static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project";

private final ExecutorProvider executorProvider;
private final ExecutorProvider backgroundExecutorProvider;
private final CredentialsProvider credentialsProvider;
private final HeaderProvider headerProvider;
Expand All @@ -77,6 +76,8 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {
@Nullable private final WatchdogProvider streamWatchdogProvider;
@Nonnull private final Duration streamWatchdogCheckInterval;
@Nonnull private final ApiTracerFactory tracerFactory;
// Track if deprecated setExecutorProvider is called
private boolean deprecatedExecutorProviderSet;

/**
* Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the
Expand All @@ -88,7 +89,6 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {

/** Constructs an instance of StubSettings. */
protected StubSettings(Builder builder) {
this.executorProvider = builder.executorProvider;
this.backgroundExecutorProvider = builder.backgroundExecutorProvider;
this.transportChannelProvider = builder.transportChannelProvider;
this.credentialsProvider = builder.credentialsProvider;
Expand All @@ -102,12 +102,13 @@ protected StubSettings(Builder builder) {
this.streamWatchdogProvider = builder.streamWatchdogProvider;
this.streamWatchdogCheckInterval = builder.streamWatchdogCheckInterval;
this.tracerFactory = builder.tracerFactory;
this.deprecatedExecutorProviderSet = builder.deprecatedExecutorProviderSet;
}

/** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */
@Deprecated
public final ExecutorProvider getExecutorProvider() {
return executorProvider;
return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null;
}

public final ExecutorProvider getBackgroundExecutorProvider() {
Expand Down Expand Up @@ -177,7 +178,6 @@ public ApiTracerFactory getTracerFactory() {

public String toString() {
return MoreObjects.toStringHelper(this)
.add("executorProvider", executorProvider)
.add("backgroundExecutorProvider", backgroundExecutorProvider)
.add("transportChannelProvider", transportChannelProvider)
.add("credentialsProvider", credentialsProvider)
Expand All @@ -199,7 +199,6 @@ public String toString() {
public abstract static class Builder<
SettingsT extends StubSettings<SettingsT>, B extends Builder<SettingsT, B>> {

private ExecutorProvider executorProvider;
private ExecutorProvider backgroundExecutorProvider;
private CredentialsProvider credentialsProvider;
private HeaderProvider headerProvider;
Expand All @@ -212,6 +211,7 @@ public abstract static class Builder<
@Nullable private WatchdogProvider streamWatchdogProvider;
@Nonnull private Duration streamWatchdogCheckInterval;
@Nonnull private ApiTracerFactory tracerFactory;
private boolean deprecatedExecutorProviderSet;

/**
* Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the
Expand All @@ -223,7 +223,6 @@ public abstract static class Builder<

/** Create a builder from a StubSettings object. */
protected Builder(StubSettings settings) {
this.executorProvider = settings.executorProvider;
this.backgroundExecutorProvider = settings.backgroundExecutorProvider;
this.transportChannelProvider = settings.transportChannelProvider;
this.credentialsProvider = settings.credentialsProvider;
Expand All @@ -237,6 +236,7 @@ protected Builder(StubSettings settings) {
this.streamWatchdogProvider = settings.streamWatchdogProvider;
this.streamWatchdogCheckInterval = settings.streamWatchdogCheckInterval;
this.tracerFactory = settings.tracerFactory;
this.deprecatedExecutorProviderSet = settings.deprecatedExecutorProviderSet;
}

/** Get Quota Project ID from Client Context * */
Expand All @@ -258,7 +258,6 @@ private static String getQuotaProjectIdFromClientContext(ClientContext clientCon

protected Builder(ClientContext clientContext) {
if (clientContext == null) {
this.executorProvider = null;
this.backgroundExecutorProvider = InstantiatingExecutorProvider.newBuilder().build();
this.transportChannelProvider = null;
this.credentialsProvider = NoCredentialsProvider.create();
Expand All @@ -271,10 +270,11 @@ protected Builder(ClientContext clientContext) {
this.streamWatchdogProvider = InstantiatingWatchdogProvider.create();
this.streamWatchdogCheckInterval = Duration.ofSeconds(10);
this.tracerFactory = BaseApiTracerFactory.getInstance();
this.deprecatedExecutorProviderSet = false;
} else {
ExecutorProvider fixedExecutorProvider =
FixedExecutorProvider.create(clientContext.getExecutor());
this.executorProvider = fixedExecutorProvider;
this.deprecatedExecutorProviderSet = true;
this.backgroundExecutorProvider = fixedExecutorProvider;
this.transportChannelProvider =
FixedTransportChannelProvider.create(clientContext.getTransportChannel());
Expand Down Expand Up @@ -317,13 +317,13 @@ protected B self() {
*/
@Deprecated
public B setExecutorProvider(ExecutorProvider executorProvider) {
// For backward compatibility, this will set both executorProvider and
// backgroundExecutorProvider. ExecutorProvider is null by default. In ClientContext#create(),
// if TransportChannelProvider doesn't have an executor, executorProvider will be used as
// TransportChannelProvider's executor. After this method is deprecated,
// TransportChannelProvider's executor can only be set with
// For backward compatibility, this will set backgroundExecutorProvider and mark
// deprecatedExecutorProviderSet to true. In ClientContext#create(), if
// TransportChannelProvider doesn't have an executor, and deprecatedExecutorProviderSet is
// true, backgroundExecutorProvider will be used as TransportChannelProvider's executor.
// After this method is deprecated, TransportChannelProvider's executor can only be set with
// TransportChannelProvider#withExecutor.
this.executorProvider = executorProvider;
this.deprecatedExecutorProviderSet = true;
this.backgroundExecutorProvider = executorProvider;
return self();
}
Expand Down Expand Up @@ -457,7 +457,8 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) {
/** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */
@Deprecated
public ExecutorProvider getExecutorProvider() {
return executorProvider;
// return executorProvider;
return deprecatedExecutorProviderSet ? backgroundExecutorProvider : null;
}

/** Gets the ExecutorProvider that was previously set on this Builder. */
Expand Down Expand Up @@ -537,7 +538,6 @@ protected static void applyToAllUnaryMethods(

public String toString() {
return MoreObjects.toStringHelper(this)
.add("executorProvider", executorProvider)
.add("backgroundExecutorProvider", backgroundExecutorProvider)
.add("transportChannelProvider", transportChannelProvider)
.add("credentialsProvider", credentialsProvider)
Expand Down

0 comments on commit ea12683

Please sign in to comment.