Skip to content

Commit

Permalink
api,core: Add LoadBalancer.acceptResolvedAddresses() (#9498)
Browse files Browse the repository at this point in the history
Introduces a new acceptResolvedAddresses() method in LoadBalancer that
is to ultimately replace the existing handleResolvedAddresses(). The new
method allows the LoadBalancer implementation to reject the given
addresses by returning false from the method.

The long deprecated handleResolvedAddressGroups is also removed.
  • Loading branch information
temawi committed Aug 31, 2022
1 parent 8dbff5b commit 4b4cb0b
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 196 deletions.
49 changes: 28 additions & 21 deletions api/src/main/java/io/grpc/LoadBalancer.java
Expand Up @@ -124,39 +124,46 @@ public abstract class LoadBalancer {
*
* <p>Implementations should not modify the given {@code servers}.
*
* @param servers the resolved server addresses, never empty.
* @param attributes extra information from naming system.
* @deprecated override {@link #handleResolvedAddresses(ResolvedAddresses) instead}
* @since 1.2.0
* @param resolvedAddresses the resolved server addresses, attributes, and config.
* @since 1.21.0
*/
@Deprecated
public void handleResolvedAddressGroups(
List<EquivalentAddressGroup> servers,
@NameResolver.ResolutionResultAttr Attributes attributes) {
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
if (recursionCount++ == 0) {
handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build());
// Note that the information about the addresses actually being accepted will be lost
// if you rely on this method for backward compatibility.
acceptResolvedAddresses(resolvedAddresses);
}
recursionCount = 0;
}

/**
* Handles newly resolved server groups and metadata attributes from name resolution system.
* {@code servers} contained in {@link EquivalentAddressGroup} should be considered equivalent
* but may be flattened into a single list if needed.
* Accepts newly resolved addresses from the name resolution system. The {@link
* EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a
* single list if needed.
*
* <p>Implementations should not modify the given {@code servers}.
* <p>Implementations can choose to reject the given addresses by returning {@code false}.
*
* <p>Implementations should not modify the given {@code addresses}.
*
* @param resolvedAddresses the resolved server addresses, attributes, and config.
* @since 1.21.0
* @return {@code true} if the resolved addresses were accepted. {@code false} if rejected.
* @since 1.49.0
*/
@SuppressWarnings("deprecation")
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
if (recursionCount++ == 0) {
handleResolvedAddressGroups(
resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes());
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
if (resolvedAddresses.getAddresses().isEmpty()
&& !canHandleEmptyAddressListFromNameResolution()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
"NameResolver returned no usable address. addrs=" + resolvedAddresses.getAddresses()
+ ", attrs=" + resolvedAddresses.getAttributes()));
return false;
} else {
if (recursionCount++ == 0) {
handleResolvedAddresses(resolvedAddresses);
}
recursionCount = 0;

return true;
}
recursionCount = 0;
}

/**
Expand Down
54 changes: 26 additions & 28 deletions api/src/test/java/io/grpc/LoadBalancerTest.java
Expand Up @@ -235,13 +235,14 @@ public void createSubchannelArgs_toString() {

@Deprecated
@Test
public void handleResolvedAddressGroups_delegatesToHandleResolvedAddresses() {
public void handleResolvedAddresses_delegatesToAcceptResolvedAddresses() {
final AtomicReference<ResolvedAddresses> resultCapture = new AtomicReference<>();

LoadBalancer balancer = new LoadBalancer() {
@Override
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
resultCapture.set(resolvedAddresses);
return true;
}

@Override
Expand All @@ -260,23 +261,22 @@ public void shutdown() {
List<EquivalentAddressGroup> servers = Arrays.asList(
new EquivalentAddressGroup(new SocketAddress(){}),
new EquivalentAddressGroup(new SocketAddress(){}));
balancer.handleResolvedAddressGroups(servers, attrs);
ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers)
.setAttributes(attrs).build();
balancer.handleResolvedAddresses(addresses);
assertThat(resultCapture.get()).isEqualTo(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build());
}

@Deprecated
@Test
public void handleResolvedAddresses_delegatesToHandleResolvedAddressGroups() {
final AtomicReference<List<EquivalentAddressGroup>> serversCapture = new AtomicReference<>();
final AtomicReference<Attributes> attrsCapture = new AtomicReference<>();
public void acceptResolvedAddresses_delegatesToHandleResolvedAddressGroups() {
final AtomicReference<ResolvedAddresses> addressesCapture = new AtomicReference<>();

LoadBalancer balancer = new LoadBalancer() {
@Override
public void handleResolvedAddressGroups(
List<EquivalentAddressGroup> servers, Attributes attrs) {
serversCapture.set(servers);
attrsCapture.set(attrs);
public void handleResolvedAddresses(ResolvedAddresses addresses) {
addressesCapture.set(addresses);
}

@Override
Expand All @@ -295,25 +295,23 @@ public void shutdown() {
List<EquivalentAddressGroup> servers = Arrays.asList(
new EquivalentAddressGroup(new SocketAddress(){}),
new EquivalentAddressGroup(new SocketAddress(){}));
balancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build());
assertThat(serversCapture.get()).isEqualTo(servers);
assertThat(attrsCapture.get()).isEqualTo(attrs);
ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers)
.setAttributes(attrs).build();
balancer.handleResolvedAddresses(addresses);
assertThat(addressesCapture.get().getAddresses()).isEqualTo(servers);
assertThat(addressesCapture.get().getAttributes()).isEqualTo(attrs);
}

@Deprecated
@Test
public void handleResolvedAddresses_noInfiniteLoop() {
final List<List<EquivalentAddressGroup>> serversCapture = new ArrayList<>();
final List<Attributes> attrsCapture = new ArrayList<>();
public void acceptResolvedAddresses_noInfiniteLoop() {
final List<ResolvedAddresses> addressesCapture = new ArrayList<>();

LoadBalancer balancer = new LoadBalancer() {
@Override
public void handleResolvedAddressGroups(
List<EquivalentAddressGroup> servers, Attributes attrs) {
serversCapture.add(servers);
attrsCapture.add(attrs);
super.handleResolvedAddressGroups(servers, attrs);
public void handleResolvedAddresses(ResolvedAddresses addresses) {
addressesCapture.add(addresses);
super.handleResolvedAddresses(addresses);
}

@Override
Expand All @@ -328,12 +326,12 @@ public void shutdown() {
List<EquivalentAddressGroup> servers = Arrays.asList(
new EquivalentAddressGroup(new SocketAddress(){}),
new EquivalentAddressGroup(new SocketAddress(){}));
balancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build());
assertThat(serversCapture).hasSize(1);
assertThat(attrsCapture).hasSize(1);
assertThat(serversCapture.get(0)).isEqualTo(servers);
assertThat(attrsCapture.get(0)).isEqualTo(attrs);
ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers)
.setAttributes(attrs).build();
balancer.handleResolvedAddresses(addresses);
assertThat(addressesCapture).hasSize(1);
assertThat(addressesCapture.get(0).getAddresses()).isEqualTo(servers);
assertThat(addressesCapture.get(0).getAttributes()).isEqualTo(attrs);
}

private static class NoopHelper extends LoadBalancer.Helper {
Expand Down
Expand Up @@ -20,11 +20,9 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import io.grpc.Attributes;
import io.grpc.ChannelLogger.ChannelLogLevel;
import io.grpc.ConnectivityState;
import io.grpc.ConnectivityStateInfo;
import io.grpc.EquivalentAddressGroup;
import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancer.PickResult;
Expand Down Expand Up @@ -67,10 +65,14 @@ private static final class NoopLoadBalancer extends LoadBalancer {

@Override
@Deprecated
public void handleResolvedAddressGroups(List<EquivalentAddressGroup> s, Attributes a) {}
@SuppressWarnings("InlineMeSuggester")
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
}

@Override
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {}
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
return true;
}

@Override
public void handleNameResolutionError(Status error) {}
Expand All @@ -97,14 +99,10 @@ public final class AutoConfiguredLoadBalancer {
}

/**
* Returns non-OK status if resolvedAddresses is empty and delegate lb requires address ({@link
* LoadBalancer#canHandleEmptyAddressListFromNameResolution()} returns {@code false}). {@code
* AutoConfiguredLoadBalancer} doesn't expose {@code
* canHandleEmptyAddressListFromNameResolution} because it depends on the delegated LB.
* Returns non-OK status if the delegate rejects the resolvedAddresses (e.g. if it does not
* support an empty list).
*/
Status tryHandleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
List<EquivalentAddressGroup> servers = resolvedAddresses.getAddresses();
Attributes attributes = resolvedAddresses.getAttributes();
boolean tryAcceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
PolicySelection policySelection =
(PolicySelection) resolvedAddresses.getLoadBalancingPolicyConfig();

Expand All @@ -118,7 +116,7 @@ Status tryHandleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
delegate.shutdown();
delegateProvider = null;
delegate = new NoopLoadBalancer();
return Status.OK;
return true;
}
policySelection =
new PolicySelection(defaultProvider, /* config= */ null);
Expand All @@ -141,20 +139,12 @@ Status tryHandleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
ChannelLogLevel.DEBUG, "Load-balancing config: {0}", policySelection.config);
}

LoadBalancer delegate = getDelegate();
if (resolvedAddresses.getAddresses().isEmpty()
&& !delegate.canHandleEmptyAddressListFromNameResolution()) {
return Status.UNAVAILABLE.withDescription(
"NameResolver returned no usable address. addrs=" + servers + ", attrs=" + attributes);
} else {
delegate.handleResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(resolvedAddresses.getAddresses())
.setAttributes(attributes)
.setLoadBalancingPolicyConfig(lbConfig)
.build());
return Status.OK;
}
return getDelegate().acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(resolvedAddresses.getAddresses())
.setAttributes(resolvedAddresses.getAttributes())
.setLoadBalancingPolicyConfig(lbConfig)
.build());
}

void handleNameResolutionError(Status error) {
Expand Down
9 changes: 5 additions & 4 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Expand Up @@ -1852,16 +1852,17 @@ public void run() {
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
.build();
}
Attributes attributes = attrBuilder.build();

Status handleResult = helper.lb.tryHandleResolvedAddresses(
boolean addressesAccepted = helper.lb.tryAcceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers)
.setAttributes(attrBuilder.build())
.setAttributes(attributes)
.setLoadBalancingPolicyConfig(effectiveServiceConfig.getLoadBalancingConfig())
.build());

if (!handleResult.isOk()) {
handleErrorInSyncContext(handleResult.augmentDescription(resolver + " was used"));
if (!addressesAccepted) {
scheduleExponentialBackOffInSyncContext();
}
}
}
Expand Down
15 changes: 4 additions & 11 deletions core/src/main/java/io/grpc/util/ForwardingLoadBalancer.java
Expand Up @@ -17,14 +17,10 @@
package io.grpc.util;

import com.google.common.base.MoreObjects;
import io.grpc.Attributes;
import io.grpc.ConnectivityStateInfo;
import io.grpc.EquivalentAddressGroup;
import io.grpc.ExperimentalApi;
import io.grpc.LoadBalancer;
import io.grpc.NameResolver;
import io.grpc.Status;
import java.util.List;

@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771")
public abstract class ForwardingLoadBalancer extends LoadBalancer {
Expand All @@ -34,16 +30,13 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer {
protected abstract LoadBalancer delegate();

@Override
@Deprecated
public void handleResolvedAddressGroups(
List<EquivalentAddressGroup> servers,
@NameResolver.ResolutionResultAttr Attributes attributes) {
delegate().handleResolvedAddressGroups(servers, attributes);
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
delegate().handleResolvedAddresses(resolvedAddresses);
}

@Override
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
delegate().handleResolvedAddresses(resolvedAddresses);
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
return delegate().acceptResolvedAddresses(resolvedAddresses);
}

@Override
Expand Down

0 comments on commit 4b4cb0b

Please sign in to comment.