Skip to content

Commit dfb22ba

Browse files
ejona86ejona86
committed
xds: Avoid switchTo in ClusterImplLb and ClusterResolverLb
1 parent 749b2e0 commit dfb22ba

8 files changed

+111
-103
lines changed

‎xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,12 @@
2323
import com.google.common.annotations.VisibleForTesting;
2424
import io.grpc.InternalLogId;
2525
import io.grpc.LoadBalancer;
26-
import io.grpc.LoadBalancerProvider;
2726
import io.grpc.LoadBalancerRegistry;
2827
import io.grpc.NameResolver;
2928
import io.grpc.Status;
3029
import io.grpc.SynchronizationContext;
3130
import io.grpc.internal.ObjectPool;
32-
import io.grpc.internal.ServiceConfigUtil;
33-
import io.grpc.internal.ServiceConfigUtil.LbConfig;
34-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
31+
import io.grpc.util.GracefulSwitchLoadBalancer;
3532
import io.grpc.xds.CdsLoadBalancerProvider.CdsConfig;
3633
import io.grpc.xds.ClusterResolverLoadBalancerProvider.ClusterResolverConfig;
3734
import io.grpc.xds.ClusterResolverLoadBalancerProvider.ClusterResolverConfig.DiscoveryMechanism;
@@ -43,6 +40,7 @@
4340
import io.grpc.xds.client.XdsLogger.XdsLogLevel;
4441
import java.util.ArrayDeque;
4542
import java.util.ArrayList;
43+
import java.util.Arrays;
4644
import java.util.Collections;
4745
import java.util.HashMap;
4846
import java.util.HashSet;
@@ -234,26 +232,17 @@ private void handleClusterDiscovered() {
234232
return;
235233
}
236234

237-
// The LB policy config is provided in service_config.proto/JSON format. It is unwrapped
238-
// to determine the name of the policy in the load balancer registry.
239-
LbConfig unwrappedLbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(
240-
root.result.lbPolicyConfig());
241-
LoadBalancerProvider lbProvider = lbRegistry.getProvider(unwrappedLbConfig.getPolicyName());
242-
if (lbProvider == null) {
243-
throw NameResolver.ConfigOrError.fromError(Status.UNAVAILABLE.withDescription(
244-
"No provider available for LB: " + unwrappedLbConfig.getPolicyName())).getError()
245-
.asRuntimeException();
246-
}
247-
NameResolver.ConfigOrError configOrError = lbProvider.parseLoadBalancingPolicyConfig(
248-
unwrappedLbConfig.getRawConfigValue());
235+
// The LB policy config is provided in service_config.proto/JSON format.
236+
NameResolver.ConfigOrError configOrError =
237+
GracefulSwitchLoadBalancer.parseLoadBalancingPolicyConfig(
238+
Arrays.asList(root.result.lbPolicyConfig()), lbRegistry);
249239
if (configOrError.getError() != null) {
250240
throw configOrError.getError().augmentDescription("Unable to parse the LB config")
251241
.asRuntimeException();
252242
}
253243

254244
ClusterResolverConfig config = new ClusterResolverConfig(
255-
Collections.unmodifiableList(instances),
256-
new PolicySelection(lbProvider, configOrError.getConfig()));
245+
Collections.unmodifiableList(instances), configOrError.getConfig());
257246
if (childLb == null) {
258247
childLb = lbRegistry.getProvider(CLUSTER_RESOLVER_POLICY_NAME).newLoadBalancer(helper);
259248
}

‎xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,10 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
145145
childLbHelper.updateSslContextProviderSupplier(config.tlsContext);
146146
childLbHelper.updateFilterMetadata(config.filterMetadata);
147147

148-
childSwitchLb.switchTo(config.childPolicy.getProvider());
149148
childSwitchLb.handleResolvedAddresses(
150149
resolvedAddresses.toBuilder()
151150
.setAttributes(attributes)
152-
.setLoadBalancingPolicyConfig(config.childPolicy.getConfig())
151+
.setLoadBalancingPolicyConfig(config.childConfig)
153152
.build());
154153
return Status.OK;
155154
}

‎xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancerProvider.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import io.grpc.LoadBalancerRegistry;
3030
import io.grpc.NameResolver.ConfigOrError;
3131
import io.grpc.Status;
32-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
3332
import io.grpc.xds.Endpoints.DropOverload;
3433
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
3534
import io.grpc.xds.client.Bootstrapper.ServerInfo;
@@ -97,12 +96,12 @@ static final class ClusterImplConfig {
9796
// Drop configurations.
9897
final List<DropOverload> dropCategories;
9998
// Provides the direct child policy and its config.
100-
final PolicySelection childPolicy;
99+
final Object childConfig;
101100
final Map<String, Struct> filterMetadata;
102101

103102
ClusterImplConfig(String cluster, @Nullable String edsServiceName,
104103
@Nullable ServerInfo lrsServerInfo, @Nullable Long maxConcurrentRequests,
105-
List<DropOverload> dropCategories, PolicySelection childPolicy,
104+
List<DropOverload> dropCategories, Object childConfig,
106105
@Nullable UpstreamTlsContext tlsContext, Map<String, Struct> filterMetadata) {
107106
this.cluster = checkNotNull(cluster, "cluster");
108107
this.edsServiceName = edsServiceName;
@@ -112,7 +111,7 @@ static final class ClusterImplConfig {
112111
this.filterMetadata = ImmutableMap.copyOf(filterMetadata);
113112
this.dropCategories = Collections.unmodifiableList(
114113
new ArrayList<>(checkNotNull(dropCategories, "dropCategories")));
115-
this.childPolicy = checkNotNull(childPolicy, "childPolicy");
114+
this.childConfig = checkNotNull(childConfig, "childConfig");
116115
}
117116

118117
@Override
@@ -124,7 +123,7 @@ public String toString() {
124123
.add("maxConcurrentRequests", maxConcurrentRequests)
125124
// Exclude tlsContext as its string representation is cumbersome.
126125
.add("dropCategories", dropCategories)
127-
.add("childPolicy", childPolicy)
126+
.add("childConfig", childConfig)
128127
.toString();
129128
}
130129
}

‎xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,11 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
127127
(ClusterResolverConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
128128
if (!Objects.equals(this.config, config)) {
129129
logger.log(XdsLogLevel.DEBUG, "Config: {0}", config);
130-
delegate.switchTo(new ClusterResolverLbStateFactory());
131130
this.config = config;
132-
delegate.handleResolvedAddresses(resolvedAddresses);
131+
Object gracefulConfig = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
132+
new ClusterResolverLbStateFactory(), config);
133+
delegate.handleResolvedAddresses(
134+
resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(gracefulConfig).build());
133135
}
134136
return Status.OK;
135137
}
@@ -165,7 +167,7 @@ private final class ClusterResolverLbState extends LoadBalancer {
165167
private final Helper helper;
166168
private final List<String> clusters = new ArrayList<>();
167169
private final Map<String, ClusterState> clusterStates = new HashMap<>();
168-
private PolicySelection endpointLbPolicy;
170+
private Object endpointLbConfig;
169171
private ResolvedAddresses resolvedAddresses;
170172
private LoadBalancer childLb;
171173

@@ -179,7 +181,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
179181
this.resolvedAddresses = resolvedAddresses;
180182
ClusterResolverConfig config =
181183
(ClusterResolverConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
182-
endpointLbPolicy = config.lbPolicy;
184+
endpointLbConfig = config.lbConfig;
183185
for (DiscoveryMechanism instance : config.discoveryMechanisms) {
184186
clusters.add(instance.cluster);
185187
ClusterState state;
@@ -454,7 +456,7 @@ public void run() {
454456
Map<String, PriorityChildConfig> priorityChildConfigs =
455457
generateEdsBasedPriorityChildConfigs(
456458
name, edsServiceName, lrsServerInfo, maxConcurrentRequests, tlsContext,
457-
filterMetadata, outlierDetection, endpointLbPolicy, lbRegistry,
459+
filterMetadata, outlierDetection, endpointLbConfig, lbRegistry,
458460
prioritizedLocalityWeights, dropOverloads);
459461
status = Status.OK;
460462
resolved = true;
@@ -717,11 +719,11 @@ private static PriorityChildConfig generateDnsBasedPriorityChildConfig(
717719
@Nullable UpstreamTlsContext tlsContext, Map<String, Struct> filterMetadata,
718720
LoadBalancerRegistry lbRegistry, List<DropOverload> dropOverloads) {
719721
// Override endpoint-level LB policy with pick_first for logical DNS cluster.
720-
PolicySelection endpointLbPolicy =
721-
new PolicySelection(lbRegistry.getProvider("pick_first"), null);
722+
Object endpointLbConfig = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
723+
lbRegistry.getProvider("pick_first"), null);
722724
ClusterImplConfig clusterImplConfig =
723725
new ClusterImplConfig(cluster, null, lrsServerInfo, maxConcurrentRequests,
724-
dropOverloads, endpointLbPolicy, tlsContext, filterMetadata);
726+
dropOverloads, endpointLbConfig, tlsContext, filterMetadata);
725727
LoadBalancerProvider clusterImplLbProvider =
726728
lbRegistry.getProvider(XdsLbPolicies.CLUSTER_IMPL_POLICY_NAME);
727729
PolicySelection clusterImplPolicy =
@@ -739,14 +741,14 @@ private static Map<String, PriorityChildConfig> generateEdsBasedPriorityChildCon
739741
String cluster, @Nullable String edsServiceName, @Nullable ServerInfo lrsServerInfo,
740742
@Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext,
741743
Map<String, Struct> filterMetadata,
742-
@Nullable OutlierDetection outlierDetection, PolicySelection endpointLbPolicy,
744+
@Nullable OutlierDetection outlierDetection, Object endpointLbConfig,
743745
LoadBalancerRegistry lbRegistry, Map<String,
744746
Map<Locality, Integer>> prioritizedLocalityWeights, List<DropOverload> dropOverloads) {
745747
Map<String, PriorityChildConfig> configs = new HashMap<>();
746748
for (String priority : prioritizedLocalityWeights.keySet()) {
747749
ClusterImplConfig clusterImplConfig =
748750
new ClusterImplConfig(cluster, edsServiceName, lrsServerInfo, maxConcurrentRequests,
749-
dropOverloads, endpointLbPolicy, tlsContext, filterMetadata);
751+
dropOverloads, endpointLbConfig, tlsContext, filterMetadata);
750752
LoadBalancerProvider clusterImplLbProvider =
751753
lbRegistry.getProvider(XdsLbPolicies.CLUSTER_IMPL_POLICY_NAME);
752754
PolicySelection priorityChildPolicy =

‎xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import io.grpc.LoadBalancerProvider;
2828
import io.grpc.NameResolver.ConfigOrError;
2929
import io.grpc.Status;
30-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
3130
import io.grpc.xds.EnvoyServerProtoData.OutlierDetection;
3231
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
3332
import io.grpc.xds.client.Bootstrapper.ServerInfo;
@@ -73,18 +72,17 @@ public LoadBalancer newLoadBalancer(Helper helper) {
7372
static final class ClusterResolverConfig {
7473
// Ordered list of clusters to be resolved.
7574
final List<DiscoveryMechanism> discoveryMechanisms;
76-
// Endpoint-level load balancing policy with config
77-
// (round_robin, least_request_experimental or ring_hash_experimental).
78-
final PolicySelection lbPolicy;
75+
// GracefulSwitch configuration
76+
final Object lbConfig;
7977

80-
ClusterResolverConfig(List<DiscoveryMechanism> discoveryMechanisms, PolicySelection lbPolicy) {
78+
ClusterResolverConfig(List<DiscoveryMechanism> discoveryMechanisms, Object lbConfig) {
8179
this.discoveryMechanisms = checkNotNull(discoveryMechanisms, "discoveryMechanisms");
82-
this.lbPolicy = checkNotNull(lbPolicy, "lbPolicy");
80+
this.lbConfig = checkNotNull(lbConfig, "lbConfig");
8381
}
8482

8583
@Override
8684
public int hashCode() {
87-
return Objects.hash(discoveryMechanisms, lbPolicy);
85+
return Objects.hash(discoveryMechanisms, lbConfig);
8886
}
8987

9088
@Override
@@ -97,14 +95,14 @@ public boolean equals(Object o) {
9795
}
9896
ClusterResolverConfig that = (ClusterResolverConfig) o;
9997
return discoveryMechanisms.equals(that.discoveryMechanisms)
100-
&& lbPolicy.equals(that.lbPolicy);
98+
&& lbConfig.equals(that.lbConfig);
10199
}
102100

103101
@Override
104102
public String toString() {
105103
return MoreObjects.toStringHelper(this)
106104
.add("discoveryMechanisms", discoveryMechanisms)
107-
.add("lbPolicy", lbPolicy)
105+
.add("lbConfig", lbConfig)
108106
.toString();
109107
}
110108

‎xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import io.grpc.Status.Code;
4949
import io.grpc.SynchronizationContext;
5050
import io.grpc.internal.ObjectPool;
51+
import io.grpc.util.GracefulSwitchLoadBalancerAccessor;
5152
import io.grpc.xds.CdsLoadBalancerProvider.CdsConfig;
5253
import io.grpc.xds.ClusterResolverLoadBalancerProvider.ClusterResolverConfig;
5354
import io.grpc.xds.ClusterResolverLoadBalancerProvider.ClusterResolverConfig.DiscoveryMechanism;
@@ -177,7 +178,9 @@ public void discoverTopLevelEdsCluster() {
177178
DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms);
178179
assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME,
179180
null, LRS_SERVER_INFO, 100L, upstreamTlsContext, outlierDetection);
180-
assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName()).isEqualTo("round_robin");
181+
assertThat(
182+
GracefulSwitchLoadBalancerAccessor.getChildProvider(childLbConfig.lbConfig).getPolicyName())
183+
.isEqualTo("round_robin");
181184
}
182185

183186
@Test
@@ -194,9 +197,12 @@ public void discoverTopLevelLogicalDnsCluster() {
194197
DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms);
195198
assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.LOGICAL_DNS, null,
196199
DNS_HOST_NAME, LRS_SERVER_INFO, 100L, upstreamTlsContext, null);
197-
assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName())
200+
assertThat(
201+
GracefulSwitchLoadBalancerAccessor.getChildProvider(childLbConfig.lbConfig).getPolicyName())
198202
.isEqualTo("least_request_experimental");
199-
assertThat(((LeastRequestConfig) childLbConfig.lbPolicy.getConfig()).choiceCount).isEqualTo(3);
203+
LeastRequestConfig lrConfig = (LeastRequestConfig)
204+
GracefulSwitchLoadBalancerAccessor.getChildConfig(childLbConfig.lbConfig);
205+
assertThat(lrConfig.choiceCount).isEqualTo(3);
200206
}
201207

202208
@Test
@@ -303,10 +309,13 @@ public void discoverAggregateCluster() {
303309
upstreamTlsContext, outlierDetection);
304310
assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(2), cluster4,
305311
DiscoveryMechanism.Type.EDS, null, null, LRS_SERVER_INFO, 300L, null, outlierDetection);
306-
assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName())
312+
assertThat(
313+
GracefulSwitchLoadBalancerAccessor.getChildProvider(childLbConfig.lbConfig).getPolicyName())
307314
.isEqualTo("ring_hash_experimental"); // dominated by top-level cluster's config
308-
assertThat(((RingHashConfig) childLbConfig.lbPolicy.getConfig()).minRingSize).isEqualTo(100L);
309-
assertThat(((RingHashConfig) childLbConfig.lbPolicy.getConfig()).maxRingSize).isEqualTo(1000L);
315+
RingHashConfig ringHashConfig = (RingHashConfig)
316+
GracefulSwitchLoadBalancerAccessor.getChildConfig(childLbConfig.lbConfig);
317+
assertThat(ringHashConfig.minRingSize).isEqualTo(100L);
318+
assertThat(ringHashConfig.maxRingSize).isEqualTo(1000L);
310319
}
311320

312321
@Test
@@ -665,9 +674,9 @@ public void unknownLbProvider() {
665674
xdsClient.deliverCdsUpdate(CLUSTER,
666675
CdsUpdate.forEds(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_INFO, 100L, upstreamTlsContext,
667676
outlierDetection)
668-
.lbPolicyConfig(ImmutableMap.of("unknown", ImmutableMap.of("foo", "bar"))).build());
677+
.lbPolicyConfig(ImmutableMap.of("unknownLb", ImmutableMap.of("foo", "bar"))).build());
669678
} catch (Exception e) {
670-
assertThat(e).hasMessageThat().contains("No provider available");
679+
assertThat(e).hasMessageThat().contains("unknownLb");
671680
return;
672681
}
673682
fail("Expected the unknown LB to cause an exception");

0 commit comments

Comments
 (0)