Skip to content

Commit

Permalink
api: Rename blockingExecutor to offloadExecutor
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ejona86 committed Nov 4, 2019
1 parent 44c4fee commit 4dba65b
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 35 deletions.
7 changes: 7 additions & 0 deletions api/src/main/java/io/grpc/ForwardingChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ public T executor(Executor executor) {
return thisT();
}

@Override
public T offloadExecutor(Executor executor) {
delegate().offloadExecutor(executor);
return thisT();
}

@Deprecated
@Override
public T blockingExecutor(Executor executor) {
delegate().blockingExecutor(executor);
Expand Down
10 changes: 8 additions & 2 deletions api/src/main/java/io/grpc/ManagedChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
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.
*
* <p>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.
Expand All @@ -117,10 +117,16 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
* @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
Expand Down
8 changes: 4 additions & 4 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ public ServiceConfigParser getServiceConfigParser() {
*/
@Nullable
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/6279")
public Executor getBlockingExecutor() {
public Executor getOffloadExecutor() {
return executor;
}

Expand All @@ -500,7 +500,7 @@ public Builder toBuilder() {
builder.setProxyDetector(proxyDetector);
builder.setSynchronizationContext(syncContext);
builder.setServiceConfigParser(serviceConfigParser);
builder.setBlockingExecutor(executor);
builder.setOffloadExecutor(executor);
return builder;
}

Expand Down Expand Up @@ -569,12 +569,12 @@ public Builder setServiceConfigParser(ServiceConfigParser parser) {
}

/**
* 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;
}
Expand Down
6 changes: 3 additions & 3 deletions api/src/test/java/io/grpc/NameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ public void args() {
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);
Expand Down Expand Up @@ -251,7 +251,7 @@ private NameResolver.Args createArgs() {
.setProxyDetector(proxyDetector)
.setSynchronizationContext(syncContext)
.setServiceConfigParser(parser)
.setBlockingExecutor(executor)
.setOffloadExecutor(executor)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public static ManagedChannelBuilder<?> forTarget(String target) {

ObjectPool<? extends Executor> executorPool = DEFAULT_EXECUTOR_POOL;

ObjectPool<? extends Executor> blockingExecutorPool = DEFAULT_EXECUTOR_POOL;
ObjectPool<? extends Executor> offloadExecutorPool = DEFAULT_EXECUTOR_POOL;

private final List<ClientInterceptor> interceptors = new ArrayList<>();
final NameResolverRegistry nameResolverRegistry = NameResolverRegistry.getDefaultRegistry();
Expand Down Expand Up @@ -220,11 +220,11 @@ public final T executor(Executor executor) {
}

@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();
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/internal/DnsNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
private final ObjectPool<? extends Executor> executorPool;
private final ObjectPool<? extends Executor> balancerRpcExecutorPool;
private final ExecutorHolder balancerRpcExecutorHolder;
private final ExecutorHolder blockingExecutorHolder;
private final ExecutorHolder offloadExecutorHolder;
private final TimeProvider timeProvider;
private final int maxTraceEvents;

Expand Down Expand Up @@ -566,9 +566,9 @@ ClientStream newSubstream(ClientStreamTracer.Factory tracerFactory, Metadata new
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()
Expand All @@ -581,12 +581,12 @@ ClientStream newSubstream(ClientStreamTracer.Factory tracerFactory, Metadata new
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();
Expand Down Expand Up @@ -900,7 +900,7 @@ private void maybeTerminateChannel() {
terminatedLatch.countDown();
executorPool.returnObject(executor);
balancerRpcExecutorHolder.release();
blockingExecutorHolder.release();
offloadExecutorHolder.release();
// Release the transport factory so that it can deallocate any resources.
transportFactory.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,18 @@ public void directExecutor() {
}

@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<? extends Executor> defaultValue = builder.blockingExecutorPool;
builder.blockingExecutor(mock(Executor.class));
assertEquals(builder, builder.blockingExecutor(null));
assertEquals(defaultValue, builder.blockingExecutorPool);
public void offloadExecutor_null() {
ObjectPool<? extends Executor> defaultValue = builder.offloadExecutorPool;
builder.offloadExecutor(mock(Executor.class));
assertEquals(builder, builder.offloadExecutor(null));
assertEquals(defaultValue, builder.offloadExecutorPool);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ public void testExecutor_custom() throws Exception {
.setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR)
.setSynchronizationContext(syncContext)
.setServiceConfigParser(mock(ServiceConfigParser.class))
.setBlockingExecutor(
.setOffloadExecutor(
new Executor() {
@Override
public void execute(Runnable command) {
Expand Down
10 changes: 5 additions & 5 deletions core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public String getPolicyName() {
@Mock
private CallCredentials creds;
@Mock
private Executor blockingExecutor;
private Executor offloadExecutor;
private ChannelBuilder channelBuilder;
private boolean requestConnection = true;
private BlockingQueue<MockClientTransportInfo> transports;
Expand Down Expand Up @@ -328,7 +328,7 @@ public void setUp() throws Exception {
.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;
Expand Down Expand Up @@ -3588,14 +3588,14 @@ public String getDefaultScheme() {
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
Expand Down

0 comments on commit 4dba65b

Please sign in to comment.