diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java index ca765aa90..af3e9bbd7 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ManagedHttpJsonChannel.java @@ -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; @@ -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; @@ -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; } @@ -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); } diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index b11548f4e..ff18e77d3 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -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) { @@ -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 headers = getHeadersFromSettings(settings); if (transportChannelProvider.needsHeaders()) { @@ -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()) diff --git a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index c082b5079..8ef161efa 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -64,7 +64,6 @@ public abstract class StubSettings> { 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; @@ -77,6 +76,8 @@ public abstract class StubSettings> { @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 @@ -88,7 +89,6 @@ public abstract class StubSettings> { /** 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; @@ -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() { @@ -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) @@ -199,7 +199,6 @@ public String toString() { public abstract static class Builder< SettingsT extends StubSettings, B extends Builder> { - private ExecutorProvider executorProvider; private ExecutorProvider backgroundExecutorProvider; private CredentialsProvider credentialsProvider; private HeaderProvider headerProvider; @@ -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 @@ -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; @@ -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 * */ @@ -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(); @@ -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()); @@ -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(); } @@ -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. */ @@ -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)