Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api,core: add LoadBalancer.Helper#createResolvingOobChannelBuilder api #7136

Merged
merged 2 commits into from Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions api/src/main/java/io/grpc/LoadBalancer.java
Expand Up @@ -1044,15 +1044,33 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG
* {@link ManagedChannelBuilder#forTarget} for the format of a target string.
*
* <p>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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the last sentence is not needed as well. Basically it will use the balancer determined by the service config from the NameResolver or the default config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, removed.

*
* <p>The LoadBalancer is responsible for closing unused OOB channels, and closing all OOB
* channels within {@link #shutdown}.
*
* @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.
*
* <p>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.
*
* <p>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");
}

Expand Down
31 changes: 21 additions & 10 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Expand Up @@ -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;
Expand Down Expand Up @@ -1260,7 +1261,7 @@ public void run() {
}

@Override
public ManagedChannel createResolvingOobChannel(String target) {
public ManagedChannelBuilder<?> createResolvingOobChannelBuilder(String target) {
final class ResolvingOobChannelBuilder
extends AbstractManagedChannelImplBuilder<ResolvingOobChannelBuilder> {
int defaultPort = -1;
Expand All @@ -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
creamsoup marked this conversation as resolved.
Show resolved Hide resolved
return new ManagedChannelImpl(
this,
transportFactory,
backoffPolicyProvider,
balancerRpcExecutorPool,
stopwatchSupplier,
Collections.<ClientInterceptor>emptyList(),
timeProvider);
}
}

checkState(!terminated, "Channel is terminated");
Expand All @@ -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.<ClientInterceptor>emptyList(),
timeProvider);
return builder;
}

@Override
public ManagedChannel createResolvingOobChannel(String target) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need this overriding? Same for below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For below we definitely need.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. for the Forwarding one, i didn't change because the method is not final.

return createResolvingOobChannelBuilder(target).build();
}

@Override
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down