From 4dba65bad436f6fa113391e1db13124167637dd3 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 4 Nov 2019 11:33:02 -0800 Subject: [PATCH] api: Rename blockingExecutor to offloadExecutor The API review for #6279 came up with a more meaningful name that better explains the intent. A setter for the old name was left in ManagedChannelBuilder to ease migration to the new name by current users. --- .../java/io/grpc/ForwardingChannelBuilder.java | 7 +++++++ .../main/java/io/grpc/ManagedChannelBuilder.java | 10 ++++++++-- api/src/main/java/io/grpc/NameResolver.java | 8 ++++---- api/src/test/java/io/grpc/NameResolverTest.java | 6 +++--- .../AbstractManagedChannelImplBuilder.java | 8 ++++---- .../java/io/grpc/internal/DnsNameResolver.java | 2 +- .../io/grpc/internal/ManagedChannelImpl.java | 14 +++++++------- .../AbstractManagedChannelImplBuilderTest.java | 16 ++++++++-------- .../io/grpc/internal/DnsNameResolverTest.java | 2 +- .../io/grpc/internal/ManagedChannelImplTest.java | 10 +++++----- 10 files changed, 48 insertions(+), 35 deletions(-) diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java index 4ef944c7ab..cbbe50d1b3 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java @@ -71,6 +71,13 @@ public abstract class ForwardingChannelBuilder> public abstract T executor(Executor executor); /** - * Provides a custom executor that will be used for operations that block. + * Provides a custom executor that will be used for operations that block or are expensive. * *

It's an optional parameter. If the user has not provided an executor when the channel is * built, the builder will use a static cached thread pool. @@ -117,10 +117,16 @@ public abstract class ManagedChannelBuilder> * @since 1.25.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6279") - public T blockingExecutor(Executor executor) { + public T offloadExecutor(Executor executor) { throw new UnsupportedOperationException(); } + @Deprecated + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6279") + public T blockingExecutor(Executor executor) { + return offloadExecutor(executor); + } + /** * Adds interceptors that will be called before the channel performs its real work. This is * functionally equivalent to using {@link ClientInterceptors#intercept(Channel, List)}, but while diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index 459f74fe50..463c44eea9 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -474,7 +474,7 @@ public abstract class NameResolver { */ @Nullable @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6279") - public Executor getBlockingExecutor() { + public Executor getOffloadExecutor() { return executor; } @@ -500,7 +500,7 @@ public abstract class NameResolver { builder.setProxyDetector(proxyDetector); builder.setSynchronizationContext(syncContext); builder.setServiceConfigParser(serviceConfigParser); - builder.setBlockingExecutor(executor); + builder.setOffloadExecutor(executor); return builder; } @@ -569,12 +569,12 @@ public abstract class NameResolver { } /** - * See {@link Args#getBlockingExecutor}. This is an optional field. + * See {@link Args#getOffloadExecutor}. This is an optional field. * * @since 1.25.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6279") - public Builder setBlockingExecutor(Executor executor) { + public Builder setOffloadExecutor(Executor executor) { this.executor = executor; return this; } diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index 09e1e1c20f..3206f4be4a 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -61,14 +61,14 @@ public class NameResolverTest { assertThat(args.getProxyDetector()).isSameInstanceAs(proxyDetector); assertThat(args.getSynchronizationContext()).isSameInstanceAs(syncContext); assertThat(args.getServiceConfigParser()).isSameInstanceAs(parser); - assertThat(args.getBlockingExecutor()).isSameInstanceAs(executor); + assertThat(args.getOffloadExecutor()).isSameInstanceAs(executor); NameResolver.Args args2 = args.toBuilder().build(); assertThat(args2.getDefaultPort()).isEqualTo(defaultPort); assertThat(args2.getProxyDetector()).isSameInstanceAs(proxyDetector); assertThat(args2.getSynchronizationContext()).isSameInstanceAs(syncContext); assertThat(args2.getServiceConfigParser()).isSameInstanceAs(parser); - assertThat(args2.getBlockingExecutor()).isSameInstanceAs(executor); + assertThat(args2.getOffloadExecutor()).isSameInstanceAs(executor); assertThat(args2).isNotSameInstanceAs(args); assertThat(args2).isNotEqualTo(args); @@ -251,7 +251,7 @@ public class NameResolverTest { .setProxyDetector(proxyDetector) .setSynchronizationContext(syncContext) .setServiceConfigParser(parser) - .setBlockingExecutor(executor) + .setOffloadExecutor(executor) .build(); } } diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index 3698eda6fd..cfdabdf2d8 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -95,7 +95,7 @@ public abstract class AbstractManagedChannelImplBuilder ObjectPool executorPool = DEFAULT_EXECUTOR_POOL; - ObjectPool blockingExecutorPool = DEFAULT_EXECUTOR_POOL; + ObjectPool offloadExecutorPool = DEFAULT_EXECUTOR_POOL; private final List interceptors = new ArrayList<>(); final NameResolverRegistry nameResolverRegistry = NameResolverRegistry.getDefaultRegistry(); @@ -220,11 +220,11 @@ public abstract class AbstractManagedChannelImplBuilder } @Override - public final T blockingExecutor(Executor executor) { + public final T offloadExecutor(Executor executor) { if (executor != null) { - this.blockingExecutorPool = new FixedObjectPool<>(executor); + this.offloadExecutorPool = new FixedObjectPool<>(executor); } else { - this.blockingExecutorPool = DEFAULT_EXECUTOR_POOL; + this.offloadExecutorPool = DEFAULT_EXECUTOR_POOL; } return thisT(); } diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index feb6d7d776..2b638327cc 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -185,7 +185,7 @@ final class DnsNameResolver extends NameResolver { this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch"); this.syncContext = Preconditions.checkNotNull(args.getSynchronizationContext(), "syncContext"); - this.executor = args.getBlockingExecutor(); + this.executor = args.getOffloadExecutor(); this.usingExecutorResource = executor == null; this.enableSrv = enableSrv; } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index fed1d660bc..7114c774a9 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -142,7 +142,7 @@ final class ManagedChannelImpl extends ManagedChannel implements private final ObjectPool executorPool; private final ObjectPool balancerRpcExecutorPool; private final ExecutorHolder balancerRpcExecutorHolder; - private final ExecutorHolder blockingExecutorHolder; + private final ExecutorHolder offloadExecutorHolder; private final TimeProvider timeProvider; private final int maxTraceEvents; @@ -566,9 +566,9 @@ final class ManagedChannelImpl extends ManagedChannel implements builder.proxyDetector != null ? builder.proxyDetector : GrpcUtil.DEFAULT_PROXY_DETECTOR; this.retryEnabled = builder.retryEnabled && !builder.temporarilyDisableRetry; this.loadBalancerFactory = new AutoConfiguredLoadBalancerFactory(builder.defaultLbPolicy); - this.blockingExecutorHolder = + this.offloadExecutorHolder = new ExecutorHolder( - checkNotNull(builder.blockingExecutorPool, "blockingExecutorPool")); + checkNotNull(builder.offloadExecutorPool, "offloadExecutorPool")); this.nameResolverRegistry = builder.nameResolverRegistry; this.nameResolverArgs = NameResolver.Args.newBuilder() @@ -581,12 +581,12 @@ final class ManagedChannelImpl extends ManagedChannel implements builder.maxRetryAttempts, builder.maxHedgedAttempts, loadBalancerFactory)) - .setBlockingExecutor( - // Avoid creating the blockingExecutor until it is first used + .setOffloadExecutor( + // Avoid creating the offloadExecutor until it is first used new Executor() { @Override public void execute(Runnable command) { - blockingExecutorHolder.getExecutor().execute(command); + offloadExecutorHolder.getExecutor().execute(command); } }) .build(); @@ -900,7 +900,7 @@ final class ManagedChannelImpl extends ManagedChannel implements terminatedLatch.countDown(); executorPool.returnObject(executor); balancerRpcExecutorHolder.release(); - blockingExecutorHolder.release(); + offloadExecutorHolder.release(); // Release the transport factory so that it can deallocate any resources. transportFactory.close(); } diff --git a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java index fee1685f8a..8b454f974b 100644 --- a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java @@ -100,18 +100,18 @@ public class AbstractManagedChannelImplBuilderTest { } @Test - public void blockingExecutor_normal() { + public void offloadExecutor_normal() { Executor executor = mock(Executor.class); - assertEquals(builder, builder.blockingExecutor(executor)); - assertEquals(executor, builder.blockingExecutorPool.getObject()); + assertEquals(builder, builder.offloadExecutor(executor)); + assertEquals(executor, builder.offloadExecutorPool.getObject()); } @Test - public void blockingExecutor_null() { - ObjectPool defaultValue = builder.blockingExecutorPool; - builder.blockingExecutor(mock(Executor.class)); - assertEquals(builder, builder.blockingExecutor(null)); - assertEquals(defaultValue, builder.blockingExecutorPool); + public void offloadExecutor_null() { + ObjectPool defaultValue = builder.offloadExecutorPool; + builder.offloadExecutor(mock(Executor.class)); + assertEquals(builder, builder.offloadExecutor(null)); + assertEquals(defaultValue, builder.offloadExecutorPool); } @Test diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 6baa87ad0a..4edc2473da 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -331,7 +331,7 @@ public class DnsNameResolverTest { .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) - .setBlockingExecutor( + .setOffloadExecutor( new Executor() { @Override public void execute(Runnable command) { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index b2eb58dc10..e2d7ba70f7 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -266,7 +266,7 @@ public class ManagedChannelImplTest { @Mock private CallCredentials creds; @Mock - private Executor blockingExecutor; + private Executor offloadExecutor; private ChannelBuilder channelBuilder; private boolean requestConnection = true; private BlockingQueue transports; @@ -328,7 +328,7 @@ public class ManagedChannelImplTest { .userAgent(USER_AGENT) .idleTimeout( AbstractManagedChannelImplBuilder.IDLE_MODE_MAX_TIMEOUT_DAYS, TimeUnit.DAYS) - .blockingExecutor(blockingExecutor); + .offloadExecutor(offloadExecutor); channelBuilder.executorPool = executorPool; channelBuilder.binlog = null; channelBuilder.channelz = channelz; @@ -3588,14 +3588,14 @@ public class ManagedChannelImplTest { assertThat(args.getDefaultPort()).isEqualTo(DEFAULT_PORT); assertThat(args.getProxyDetector()).isSameInstanceAs(neverProxy); - verify(blockingExecutor, never()).execute(any(Runnable.class)); - args.getBlockingExecutor() + verify(offloadExecutor, never()).execute(any(Runnable.class)); + args.getOffloadExecutor() .execute( new Runnable() { @Override public void run() {} }); - verify(blockingExecutor, times(1)).execute(any(Runnable.class)); + verify(offloadExecutor, times(1)).execute(any(Runnable.class)); } @Test