Fix throwing LBProvider.parseLoadBalancingConfig() impls

LoadBalancers in general should never throw, but
parseLoadBalancingConfig() in particular has a return value to
communicate the error. Throwing can be a bit unpredictable, but at its
most trivial form causes a channel panic. There's no reason to throw
explicitly and calls to JsonUtil have to be protected by a try-catch
because it can throw.
This commit is contained in:
Eric Anderson 2023-05-10 11:04:08 -07:00
parent 6f804331f7
commit 02a4cb5c69
7 changed files with 51 additions and 6 deletions

View File

@ -22,6 +22,7 @@ import io.grpc.LoadBalancer;
import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerProvider;
import io.grpc.NameResolver; import io.grpc.NameResolver;
import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.PickFirstLoadBalancer.PickFirstLoadBalancerConfig; import io.grpc.internal.PickFirstLoadBalancer.PickFirstLoadBalancerConfig;
import java.util.Map; import java.util.Map;
@ -62,9 +63,15 @@ public final class PickFirstLoadBalancerProvider extends LoadBalancerProvider {
public ConfigOrError parseLoadBalancingPolicyConfig( public ConfigOrError parseLoadBalancingPolicyConfig(
Map<String, ?> rawLoadBalancingPolicyConfig) { Map<String, ?> rawLoadBalancingPolicyConfig) {
if (enablePickFirstConfig) { if (enablePickFirstConfig) {
return ConfigOrError.fromConfig( try {
new PickFirstLoadBalancerConfig(JsonUtil.getBoolean(rawLoadBalancingPolicyConfig, return ConfigOrError.fromConfig(
SHUFFLE_ADDRESS_LIST_KEY))); new PickFirstLoadBalancerConfig(JsonUtil.getBoolean(rawLoadBalancingPolicyConfig,
SHUFFLE_ADDRESS_LIST_KEY)));
} catch (RuntimeException e) {
return ConfigOrError.fromError(
Status.UNAVAILABLE.withCause(e).withDescription(
"Failed parsing configuration for " + getPolicyName()));
}
} else { } else {
return ConfigOrError.fromConfig(NO_CONFIG); return ConfigOrError.fromConfig(NO_CONFIG);
} }

View File

@ -59,6 +59,16 @@ public final class OutlierDetectionLoadBalancerProvider extends LoadBalancerProv
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
try {
return parseLoadBalancingPolicyConfigInternal(rawConfig);
} catch (RuntimeException e) {
return ConfigOrError.fromError(
Status.UNAVAILABLE.withCause(e).withDescription(
"Failed parsing configuration for " + getPolicyName()));
}
}
private ConfigOrError parseLoadBalancingPolicyConfigInternal(Map<String, ?> rawConfig) {
// Common configuration. // Common configuration.
Long intervalNanos = JsonUtil.getStringAsDuration(rawConfig, "interval"); Long intervalNanos = JsonUtil.getStringAsDuration(rawConfig, "interval");
Long baseEjectionTimeNanos = JsonUtil.getStringAsDuration(rawConfig, "baseEjectionTime"); Long baseEjectionTimeNanos = JsonUtil.getStringAsDuration(rawConfig, "baseEjectionTime");

View File

@ -25,6 +25,7 @@ import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerProvider;
import io.grpc.LoadBalancerRegistry; import io.grpc.LoadBalancerRegistry;
import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.ServiceConfigUtil.PolicySelection; import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import io.grpc.xds.Bootstrapper.ServerInfo; import io.grpc.xds.Bootstrapper.ServerInfo;
import io.grpc.xds.Endpoints.DropOverload; import io.grpc.xds.Endpoints.DropOverload;
@ -60,7 +61,8 @@ public final class ClusterImplLoadBalancerProvider extends LoadBalancerProvider
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) {
throw new UnsupportedOperationException("not supported as top-level LB policy"); return ConfigOrError.fromError(
Status.INTERNAL.withDescription(getPolicyName() + " cannot be used from service config"));
} }
@Override @Override

View File

@ -24,6 +24,7 @@ import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerProvider;
import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.ServiceConfigUtil.PolicySelection; import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import io.grpc.xds.Bootstrapper.ServerInfo; import io.grpc.xds.Bootstrapper.ServerInfo;
import io.grpc.xds.EnvoyServerProtoData.OutlierDetection; import io.grpc.xds.EnvoyServerProtoData.OutlierDetection;
@ -58,7 +59,8 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) {
throw new UnsupportedOperationException("not supported as top-level LB policy"); return ConfigOrError.fromError(
Status.INTERNAL.withDescription(getPolicyName() + " cannot be used from service config"));
} }
@Override @Override

View File

@ -25,6 +25,7 @@ import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerProvider;
import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.ServiceConfigUtil.PolicySelection; import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
@ -57,7 +58,8 @@ public final class PriorityLoadBalancerProvider extends LoadBalancerProvider {
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
throw new UnsupportedOperationException(); return ConfigOrError.fromError(
Status.INTERNAL.withDescription(getPolicyName() + " cannot be used from service config"));
} }
static final class PriorityLbConfig { static final class PriorityLbConfig {

View File

@ -68,6 +68,17 @@ public final class RingHashLoadBalancerProvider extends LoadBalancerProvider {
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawLoadBalancingPolicyConfig) {
try {
return parseLoadBalancingPolicyConfigInternal(rawLoadBalancingPolicyConfig);
} catch (RuntimeException e) {
return ConfigOrError.fromError(
Status.UNAVAILABLE.withCause(e).withDescription(
"Failed parsing configuration for " + getPolicyName()));
}
}
private ConfigOrError parseLoadBalancingPolicyConfigInternal(
Map<String, ?> rawLoadBalancingPolicyConfig) {
Long minRingSize = JsonUtil.getNumberAsLong(rawLoadBalancingPolicyConfig, "minRingSize"); Long minRingSize = JsonUtil.getNumberAsLong(rawLoadBalancingPolicyConfig, "minRingSize");
Long maxRingSize = JsonUtil.getNumberAsLong(rawLoadBalancingPolicyConfig, "maxRingSize"); Long maxRingSize = JsonUtil.getNumberAsLong(rawLoadBalancingPolicyConfig, "maxRingSize");
long maxRingSizeCap = RingHashOptions.getRingSizeCap(); long maxRingSizeCap = RingHashOptions.getRingSizeCap();

View File

@ -24,6 +24,7 @@ import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerProvider;
import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.JsonUtil; import io.grpc.internal.JsonUtil;
import io.grpc.xds.WeightedRoundRobinLoadBalancer.WeightedRoundRobinLoadBalancerConfig; import io.grpc.xds.WeightedRoundRobinLoadBalancer.WeightedRoundRobinLoadBalancerConfig;
import java.util.Map; import java.util.Map;
@ -62,6 +63,16 @@ public final class WeightedRoundRobinLoadBalancerProvider extends LoadBalancerPr
@Override @Override
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) { public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
try {
return parseLoadBalancingPolicyConfigInternal(rawConfig);
} catch (RuntimeException e) {
return ConfigOrError.fromError(
Status.UNAVAILABLE.withCause(e).withDescription(
"Failed parsing configuration for " + getPolicyName()));
}
}
private ConfigOrError parseLoadBalancingPolicyConfigInternal(Map<String, ?> rawConfig) {
Long blackoutPeriodNanos = JsonUtil.getStringAsDuration(rawConfig, "blackoutPeriod"); Long blackoutPeriodNanos = JsonUtil.getStringAsDuration(rawConfig, "blackoutPeriod");
Long weightExpirationPeriodNanos = Long weightExpirationPeriodNanos =
JsonUtil.getStringAsDuration(rawConfig, "weightExpirationPeriod"); JsonUtil.getStringAsDuration(rawConfig, "weightExpirationPeriod");