From ebbb48ff2e076b43a5d5db47518fc492495488e2 Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Wed, 17 Jun 2020 11:27:32 -0700 Subject: [PATCH 1/2] api,core: add LoadBalancer.Helper#createResolvingOobChannelBuilder api --- api/src/main/java/io/grpc/LoadBalancer.java | 22 +++++++++++-- .../io/grpc/internal/ManagedChannelImpl.java | 31 +++++++++++++------ .../util/ForwardingLoadBalancerHelper.java | 6 ++++ 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index e97571c8c63..f5f7a8eb397 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -1044,8 +1044,8 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG * {@link ManagedChannelBuilder#forTarget} for the format of a target string. * *

The target string will be resolved by a {@link NameResolver} created according to the - * target string. The out-of-band channel doesn't have load-balancing. If multiple addresses - * are resolved for the target, the first working address will be used. + * target string. If multiple addresses are resolved for the target, the first working address + * will be used. * *

The LoadBalancer is responsible for closing unused OOB channels, and closing all OOB * channels within {@link #shutdown}. @@ -1053,6 +1053,24 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG * @since 1.20.0 */ public ManagedChannel createResolvingOobChannel(String target) { + return createResolvingOobChannelBuilder(target).build(); + } + + /** + * Creates an out-of-band channel builder for LoadBalancer's own RPC needs, e.g., talking to an + * external load-balancer service, that is specified by a target string. See the documentation + * on {@link ManagedChannelBuilder#forTarget} for the format of a target string. + * + *

The target string will be resolved by a {@link NameResolver} created according to the + * target string. If multiple addresses are resolved for the target, the first working address + * will be used. + * + *

The LoadBalancer is responsible for closing unused OOB channels, and closing all OOB + * channels within {@link #shutdown}. + * + * @since 1.31.0 + */ + public ManagedChannelBuilder createResolvingOobChannelBuilder(String target) { throw new UnsupportedOperationException("Not implemented"); } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 05dee80c6ea..cd173172b55 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -60,6 +60,7 @@ import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.LoadBalancer.SubchannelStateListener; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.NameResolver; @@ -1260,7 +1261,7 @@ public void run() { } @Override - public ManagedChannel createResolvingOobChannel(String target) { + public ManagedChannelBuilder createResolvingOobChannelBuilder(String target) { final class ResolvingOobChannelBuilder extends AbstractManagedChannelImplBuilder { int defaultPort = -1; @@ -1278,6 +1279,19 @@ public int getDefaultPort() { protected ClientTransportFactory buildTransportFactory() { throw new UnsupportedOperationException(); } + + @Override + public ManagedChannel build() { + // TODO(creamsoup) prevents main channel to shutdown if oob channel is not terminated + return new ManagedChannelImpl( + this, + transportFactory, + backoffPolicyProvider, + balancerRpcExecutorPool, + stopwatchSupplier, + Collections.emptyList(), + timeProvider); + } } checkState(!terminated, "Channel is terminated"); @@ -1291,15 +1305,12 @@ protected ClientTransportFactory buildTransportFactory() { builder.proxyDetector = nameResolverArgs.getProxyDetector(); builder.defaultPort = nameResolverArgs.getDefaultPort(); builder.userAgent = userAgent; - return - new ManagedChannelImpl( - builder, - transportFactory, - backoffPolicyProvider, - balancerRpcExecutorPool, - stopwatchSupplier, - Collections.emptyList(), - timeProvider); + return builder; + } + + @Override + public ManagedChannel createResolvingOobChannel(String target) { + return createResolvingOobChannelBuilder(target).build(); } @Override diff --git a/core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java b/core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java index 95bf66c0191..85c5c7b125c 100644 --- a/core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java +++ b/core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java @@ -27,6 +27,7 @@ import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.LoadBalancer; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.NameResolver; import io.grpc.NameResolverRegistry; import io.grpc.SynchronizationContext; @@ -68,6 +69,11 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG delegate().updateOobChannelAddresses(channel, eag); } + @Override + public ManagedChannelBuilder createResolvingOobChannelBuilder(String target) { + return delegate().createResolvingOobChannelBuilder(target); + } + @Override public ManagedChannel createResolvingOobChannel(String target) { return delegate().createResolvingOobChannel(target); From 228d0bf8709a5fd1c98e50a26b5cf47857b16c3d Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Wed, 17 Jun 2020 12:04:15 -0700 Subject: [PATCH 2/2] address review comments --- api/src/main/java/io/grpc/LoadBalancer.java | 6 ++---- .../src/main/java/io/grpc/internal/ManagedChannelImpl.java | 7 +------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index f5f7a8eb397..d2ed1ad9368 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -1044,8 +1044,7 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG * {@link ManagedChannelBuilder#forTarget} for the format of a target string. * *

The target string will be resolved by a {@link NameResolver} created according to the - * target string. If multiple addresses are resolved for the target, the first working address - * will be used. + * target string. * *

The LoadBalancer is responsible for closing unused OOB channels, and closing all OOB * channels within {@link #shutdown}. @@ -1062,8 +1061,7 @@ public ManagedChannel createResolvingOobChannel(String target) { * on {@link ManagedChannelBuilder#forTarget} for the format of a target string. * *

The target string will be resolved by a {@link NameResolver} created according to the - * target string. If multiple addresses are resolved for the target, the first working address - * will be used. + * target string. * *

The LoadBalancer is responsible for closing unused OOB channels, and closing all OOB * channels within {@link #shutdown}. diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index cd173172b55..c23bd51e844 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -1282,7 +1282,7 @@ protected ClientTransportFactory buildTransportFactory() { @Override public ManagedChannel build() { - // TODO(creamsoup) prevents main channel to shutdown if oob channel is not terminated + // TODO(creamsoup) prevent main channel to shutdown if oob channel is not terminated return new ManagedChannelImpl( this, transportFactory, @@ -1308,11 +1308,6 @@ public ManagedChannel build() { return builder; } - @Override - public ManagedChannel createResolvingOobChannel(String target) { - return createResolvingOobChannelBuilder(target).build(); - } - @Override public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressGroup eag) { checkArgument(channel instanceof OobChannel,