Skip to content

Commit

Permalink
core, grpclb: change policy selection strategy for Grpclb policy (tak…
Browse files Browse the repository at this point in the history 
…e one: eliminate special logic for deciding grpclb policy in core) (#6637)

First take for grpclb selection stabilization: 

1. Changed DnsNameResolver to return balancer addresses as a GrpcAttributes.ATTR_LB_ADDRS attribute in ResolutionResult, instead of among the addresses.

2. AutoConfiguredLoadBalancerFactory decides LB policy solely based on parsed service config without looking at resolved addresses. Behavior changes:
  - If no LB policy is specified in service config, default to pick_first, even if there exist balancer addresses (in attributes).
  - If grpclb specified but not available and no other specified policies available, it will fail without fallback to round_robin.

3. GrpclbLoadBalancer populates balancer addresses from ResolvedAddresses's attribute (GrpclbConstants.ATTR_LB_ADDRS) instead of sieving from addresses.
  • Loading branch information
@voidzcy
voidzcy committed Jan 31, 2020
1 parent 3e6a77a commit c0f37e5
Show file tree
Hide file tree
Showing 13 changed files with 484 additions and 611 deletions.
1 change: 1 addition & 0 deletions alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java
View file
Expand Up @@ -223,6 +223,7 @@ public AsciiString scheme() {
return SCHEME;
}

@SuppressWarnings("deprecation")
@Override
public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
ChannelHandler gnh = InternalProtocolNegotiators.grpcNegotiationHandler(grpcHandler);
Expand Down
139 changes: 23 additions & 116 deletions core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java
View file
Expand Up @@ -41,18 +41,13 @@
import io.grpc.Status;
import io.grpc.internal.ServiceConfigUtil.LbConfig;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
import javax.annotation.Nullable;

// TODO(creamsoup) fully deprecate LoadBalancer.ATTR_LOAD_BALANCING_CONFIG
@SuppressWarnings("deprecation")
public final class AutoConfiguredLoadBalancerFactory {
private static final Logger logger =
Logger.getLogger(AutoConfiguredLoadBalancerFactory.class.getName());
private static final String GRPCLB_POLICY_NAME = "grpclb";

private final LoadBalancerRegistry registry;
private final String defaultPolicy;
Expand Down Expand Up @@ -92,7 +87,6 @@ public final class AutoConfiguredLoadBalancer {
private final Helper helper;
private LoadBalancer delegate;
private LoadBalancerProvider delegateProvider;
private boolean roundRobinDueToGrpclbDepMissing;

AutoConfiguredLoadBalancer(Helper helper) {
this.helper = helper;
Expand Down Expand Up @@ -125,48 +119,53 @@ Status tryHandleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
}
PolicySelection policySelection =
(PolicySelection) resolvedAddresses.getLoadBalancingPolicyConfig();
ResolvedPolicySelection resolvedSelection;

try {
resolvedSelection = resolveLoadBalancerProvider(servers, policySelection);
} catch (PolicyException e) {
Status s = Status.INTERNAL.withDescription(e.getMessage());
helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new FailingPicker(s));
delegate.shutdown();
delegateProvider = null;
delegate = new NoopLoadBalancer();
return Status.OK;
if (policySelection == null) {
LoadBalancerProvider defaultProvider;
try {
defaultProvider = getProviderOrThrow(defaultPolicy, "using default policy");
} catch (PolicyException e) {
Status s = Status.INTERNAL.withDescription(e.getMessage());
helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new FailingPicker(s));
delegate.shutdown();
delegateProvider = null;
delegate = new NoopLoadBalancer();
return Status.OK;
}
policySelection =
new PolicySelection(defaultProvider, /* rawConfig= */ null, /* config= */ null);
}
PolicySelection selection = resolvedSelection.policySelection;

if (delegateProvider == null
|| !selection.provider.getPolicyName().equals(delegateProvider.getPolicyName())) {
|| !policySelection.provider.getPolicyName().equals(delegateProvider.getPolicyName())) {
helper.updateBalancingState(ConnectivityState.CONNECTING, new EmptyPicker());
delegate.shutdown();
delegateProvider = selection.provider;
delegateProvider = policySelection.provider;
LoadBalancer old = delegate;
delegate = delegateProvider.newLoadBalancer(helper);
helper.getChannelLogger().log(
ChannelLogLevel.INFO, "Load balancer changed from {0} to {1}",
old.getClass().getSimpleName(), delegate.getClass().getSimpleName());
}
Object lbConfig = selection.config;
Object lbConfig = policySelection.config;
if (lbConfig != null) {
helper.getChannelLogger().log(
ChannelLogLevel.DEBUG, "Load-balancing config: {0}", selection.config);
ChannelLogLevel.DEBUG, "Load-balancing config: {0}", policySelection.config);
attributes =
attributes.toBuilder().set(ATTR_LOAD_BALANCING_CONFIG, selection.rawConfig).build();
attributes.toBuilder()
.set(ATTR_LOAD_BALANCING_CONFIG, policySelection.rawConfig)
.build();
}

LoadBalancer delegate = getDelegate();
if (resolvedSelection.serverList.isEmpty()
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(resolvedSelection.serverList)
.setAddresses(resolvedAddresses.getAddresses())
.setAttributes(attributes)
.setLoadBalancingPolicyConfig(lbConfig)
.build());
Expand Down Expand Up @@ -206,78 +205,6 @@ void setDelegate(LoadBalancer lb) {
LoadBalancerProvider getDelegateProvider() {
return delegateProvider;
}

/**
* Resolves a load balancer based on given criteria. If policySelection is {@code null} and
* given servers contains any gRPC LB addresses, it will fall back to "grpclb". If no gRPC LB
* addresses are not present, it will fall back to {@link #defaultPolicy}.
*
* @param servers The list of servers reported
* @param policySelection the selected policy from raw service config
* @return the resolved policy selection
*/
@VisibleForTesting
ResolvedPolicySelection resolveLoadBalancerProvider(
List<EquivalentAddressGroup> servers, @Nullable PolicySelection policySelection)
throws PolicyException {
// Check for balancer addresses
boolean haveBalancerAddress = false;
List<EquivalentAddressGroup> backendAddrs = new ArrayList<>();
for (EquivalentAddressGroup s : servers) {
if (s.getAttributes().get(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY) != null) {
haveBalancerAddress = true;
} else {
backendAddrs.add(s);
}
}

if (policySelection != null) {
String policyName = policySelection.provider.getPolicyName();
return new ResolvedPolicySelection(
policySelection, policyName.equals(GRPCLB_POLICY_NAME) ? servers : backendAddrs);
}

if (haveBalancerAddress) {
// This is a special case where the existence of balancer address in the resolved address
// selects "grpclb" policy if the service config couldn't select a policy
LoadBalancerProvider grpclbProvider = registry.getProvider(GRPCLB_POLICY_NAME);
if (grpclbProvider == null) {
if (backendAddrs.isEmpty()) {
throw new PolicyException(
"Received ONLY balancer addresses but grpclb runtime is missing");
}
if (!roundRobinDueToGrpclbDepMissing) {
// We don't log the warning every time we have an update.
roundRobinDueToGrpclbDepMissing = true;
String errorMsg = "Found balancer addresses but grpclb runtime is missing."
+ " Will use round_robin. Please include grpc-grpclb in your runtime dependencies.";
helper.getChannelLogger().log(ChannelLogLevel.ERROR, errorMsg);
logger.warning(errorMsg);
}
return new ResolvedPolicySelection(
new PolicySelection(
getProviderOrThrow(
"round_robin", "received balancer addresses but grpclb runtime is missing"),
/* rawConfig = */ null,
/* config= */ null),
backendAddrs);
}
return new ResolvedPolicySelection(
new PolicySelection(
grpclbProvider, /* rawConfig= */ null, /* config= */ null), servers);
}
// No balancer address this time. If balancer address shows up later, we want to make sure
// the warning is logged one more time.
roundRobinDueToGrpclbDepMissing = false;

// No config nor balancer address. Use default.
return new ResolvedPolicySelection(
new PolicySelection(
getProviderOrThrow(defaultPolicy, "using default policy"),
/* rawConfig= */ null,
/* config= */ null),
servers);
}
}

private LoadBalancerProvider getProviderOrThrow(String policy, String choiceReason)
Expand Down Expand Up @@ -406,26 +333,6 @@ public String toString() {
}
}

@VisibleForTesting
static final class ResolvedPolicySelection {
final PolicySelection policySelection;
final List<EquivalentAddressGroup> serverList;

ResolvedPolicySelection(
PolicySelection policySelection, List<EquivalentAddressGroup> serverList) {
this.policySelection = checkNotNull(policySelection, "policySelection");
this.serverList = Collections.unmodifiableList(checkNotNull(serverList, "serverList"));
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("policySelection", policySelection)
.add("serverList", serverList)
.toString();
}
}

private static final class EmptyPicker extends SubchannelPicker {

@Override
Expand Down
21 changes: 8 additions & 13 deletions core/src/main/java/io/grpc/internal/DnsNameResolver.java
View file
Expand Up @@ -298,14 +298,12 @@ public void run() {
for (InetAddress inetAddr : resolutionResults.addresses) {
servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port)));
}
servers.addAll(resolutionResults.balancerAddresses);
if (servers.isEmpty()) {
savedListener.onError(Status.UNAVAILABLE.withDescription(
"No DNS backend or balancer addresses found for " + host));
return;
}

ResolutionResult.Builder resultBuilder = ResolutionResult.newBuilder().setAddresses(servers);
Attributes.Builder attributesBuilder = Attributes.newBuilder();
if (!resolutionResults.balancerAddresses.isEmpty()) {
attributesBuilder.set(GrpcAttributes.ATTR_LB_ADDRS, resolutionResults.balancerAddresses);
}
if (!resolutionResults.txtRecords.isEmpty()) {
ConfigOrError rawServiceConfig =
parseServiceConfig(resolutionResults.txtRecords, random, getLocalHostname());
Expand All @@ -319,17 +317,14 @@ public void run() {
Map<String, ?> verifiedRawServiceConfig = (Map<String, ?>) rawServiceConfig.getConfig();
ConfigOrError parsedServiceConfig =
serviceConfigParser.parseServiceConfig(verifiedRawServiceConfig);
resultBuilder
.setAttributes(
Attributes.newBuilder()
.set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, verifiedRawServiceConfig)
.build())
.setServiceConfig(parsedServiceConfig);
resultBuilder.setServiceConfig(parsedServiceConfig);
attributesBuilder
.set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, verifiedRawServiceConfig);
}
} else {
logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host});
}
savedListener.onResult(resultBuilder.build());
savedListener.onResult(resultBuilder.setAttributes(attributesBuilder.build()).build());
}
}

Expand Down
14 changes: 14 additions & 0 deletions core/src/main/java/io/grpc/internal/GrpcAttributes.java
View file
Expand Up @@ -21,6 +21,7 @@
import io.grpc.Grpc;
import io.grpc.NameResolver;
import io.grpc.SecurityLevel;
import java.util.List;
import java.util.Map;

/**
Expand All @@ -37,10 +38,23 @@ public final class GrpcAttributes {
public static final Attributes.Key<Map<String, ?>> NAME_RESOLVER_SERVICE_CONFIG =
Attributes.Key.create("service-config");

/**
* Attribute key for gRPC LB server addresses.
*
* <p>Deprecated: this will be used for grpclb specific logic, which will be moved out of core.
*/
@Deprecated
@NameResolver.ResolutionResultAttr
public static final Attributes.Key<List<EquivalentAddressGroup>> ATTR_LB_ADDRS =
Attributes.Key.create("io.grpc.grpclb.lbAddrs");

/**
* The naming authority of a gRPC LB server address. It is an address-group-level attribute,
* present when the address group is a LoadBalancer.
*
* <p>Deprecated: this will be used for grpclb specific logic, which will be moved out of core.
*/
@Deprecated
@EquivalentAddressGroup.Attr
public static final Attributes.Key<String> ATTR_LB_ADDR_AUTHORITY =
Attributes.Key.create("io.grpc.grpclb.lbAddrAuthority");
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java
View file
Expand Up @@ -129,6 +129,7 @@ public List<String> resolveTxt(String serviceConfigHostname) throws NamingExcept
return Collections.unmodifiableList(serviceConfigTxtRecords);
}

@SuppressWarnings("deprecation")
@Override
public List<EquivalentAddressGroup> resolveSrv(
AddressResolver addressResolver, String grpclbHostname) throws Exception {
Expand Down

0 comments on commit c0f37e5

Please sign in to comment.