Skip to content

Commit 2c49cc4

Browse files
ejona86ejona86
committed
xds: Avoid switchTo in WrrLocalityLb and WeightedTargetLb
1 parent ebed047 commit 2c49cc4

10 files changed

+94
-97
lines changed

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,19 @@ public Status acceptResolvedAddressesInternal(ResolvedAddresses resolvedAddresse
7979
WeightedTargetConfig weightedTargetConfig = (WeightedTargetConfig) lbConfig;
8080
Map<String, WeightedPolicySelection> newTargets = weightedTargetConfig.targets;
8181
for (String targetName : newTargets.keySet()) {
82-
WeightedPolicySelection weightedChildLbConfig = newTargets.get(targetName);
8382
if (!targets.containsKey(targetName)) {
8483
ChildHelper childHelper = new ChildHelper(targetName);
8584
GracefulSwitchLoadBalancer childBalancer = new GracefulSwitchLoadBalancer(childHelper);
86-
childBalancer.switchTo(weightedChildLbConfig.policySelection.getProvider());
8785
childHelpers.put(targetName, childHelper);
8886
childBalancers.put(targetName, childBalancer);
89-
} else if (!weightedChildLbConfig.policySelection.getProvider().equals(
90-
targets.get(targetName).policySelection.getProvider())) {
91-
childBalancers.get(targetName)
92-
.switchTo(weightedChildLbConfig.policySelection.getProvider());
9387
}
9488
}
9589
targets = newTargets;
9690
for (String targetName : targets.keySet()) {
9791
childBalancers.get(targetName).handleResolvedAddresses(
9892
resolvedAddresses.toBuilder()
9993
.setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), targetName))
100-
.setLoadBalancingPolicyConfig(targets.get(targetName).policySelection.getConfig())
94+
.setLoadBalancingPolicyConfig(targets.get(targetName).childConfig)
10195
.setAttributes(resolvedAddresses.getAttributes().toBuilder()
10296
.set(CHILD_NAME, targetName)
10397
.build())

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

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,8 @@
2626
import io.grpc.NameResolver.ConfigOrError;
2727
import io.grpc.Status;
2828
import io.grpc.internal.JsonUtil;
29-
import io.grpc.internal.ServiceConfigUtil;
30-
import io.grpc.internal.ServiceConfigUtil.LbConfig;
31-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
29+
import io.grpc.util.GracefulSwitchLoadBalancer;
3230
import java.util.LinkedHashMap;
33-
import java.util.List;
3431
import java.util.Map;
3532
import java.util.Objects;
3633
import javax.annotation.Nullable;
@@ -97,22 +94,16 @@ public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
9794
return ConfigOrError.fromError(Status.INTERNAL.withDescription(
9895
"Wrong weight for target " + name + " in weighted_target LB policy:\n " + rawConfig));
9996
}
100-
List<LbConfig> childConfigCandidates = ServiceConfigUtil.unwrapLoadBalancingConfigList(
101-
JsonUtil.getListOfObjects(rawWeightedTarget, "childPolicy"));
102-
if (childConfigCandidates == null || childConfigCandidates.isEmpty()) {
103-
return ConfigOrError.fromError(Status.INTERNAL.withDescription(
104-
"No child policy for target " + name + " in weighted_target LB policy:\n "
105-
+ rawConfig));
106-
}
10797
LoadBalancerRegistry lbRegistry =
10898
this.lbRegistry == null ? LoadBalancerRegistry.getDefaultRegistry() : this.lbRegistry;
109-
ConfigOrError selectedConfig =
110-
ServiceConfigUtil.selectLbPolicyFromList(childConfigCandidates, lbRegistry);
111-
if (selectedConfig.getError() != null) {
112-
return selectedConfig;
99+
ConfigOrError childConfig = GracefulSwitchLoadBalancer.parseLoadBalancingPolicyConfig(
100+
JsonUtil.getListOfObjects(rawWeightedTarget, "childPolicy"), lbRegistry);
101+
if (childConfig.getError() != null) {
102+
return ConfigOrError.fromError(Status.INTERNAL
103+
.withDescription("Could not parse weighted_target's child policy:" + name)
104+
.withCause(childConfig.getError().asRuntimeException()));
113105
}
114-
PolicySelection policySelection = (PolicySelection) selectedConfig.getConfig();
115-
parsedChildConfigs.put(name, new WeightedPolicySelection(weight, policySelection));
106+
parsedChildConfigs.put(name, new WeightedPolicySelection(weight, childConfig.getConfig()));
116107
}
117108
return ConfigOrError.fromConfig(new WeightedTargetConfig(parsedChildConfigs));
118109
} catch (RuntimeException e) {
@@ -125,11 +116,11 @@ public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
125116
static final class WeightedPolicySelection {
126117

127118
final int weight;
128-
final PolicySelection policySelection;
119+
final Object childConfig;
129120

130-
WeightedPolicySelection(int weight, PolicySelection policySelection) {
121+
WeightedPolicySelection(int weight, Object childConfig) {
131122
this.weight = weight;
132-
this.policySelection = policySelection;
123+
this.childConfig = childConfig;
133124
}
134125

135126
@Override
@@ -141,19 +132,19 @@ public boolean equals(Object o) {
141132
return false;
142133
}
143134
WeightedPolicySelection that = (WeightedPolicySelection) o;
144-
return weight == that.weight && Objects.equals(policySelection, that.policySelection);
135+
return weight == that.weight && Objects.equals(childConfig, that.childConfig);
145136
}
146137

147138
@Override
148139
public int hashCode() {
149-
return Objects.hash(weight, policySelection);
140+
return Objects.hash(weight, childConfig);
150141
}
151142

152143
@Override
153144
public String toString() {
154145
return MoreObjects.toStringHelper(this)
155146
.add("weight", weight)
156-
.add("policySelection", policySelection)
147+
.add("childConfig", childConfig)
157148
.toString();
158149
}
159150
}

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import io.grpc.LoadBalancer;
2828
import io.grpc.LoadBalancerRegistry;
2929
import io.grpc.Status;
30-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
3130
import io.grpc.util.GracefulSwitchLoadBalancer;
3231
import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedPolicySelection;
3332
import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedTargetConfig;
@@ -108,13 +107,15 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
108107
for (String locality : localityWeights.keySet()) {
109108
weightedPolicySelections.put(locality,
110109
new WeightedPolicySelection(localityWeights.get(locality),
111-
wrrLocalityConfig.childPolicy));
110+
wrrLocalityConfig.childConfig));
112111
}
113112

114-
switchLb.switchTo(lbRegistry.getProvider(WEIGHTED_TARGET_POLICY_NAME));
113+
Object switchConfig = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
114+
lbRegistry.getProvider(WEIGHTED_TARGET_POLICY_NAME),
115+
new WeightedTargetConfig(weightedPolicySelections));
115116
switchLb.handleResolvedAddresses(
116117
resolvedAddresses.toBuilder()
117-
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(weightedPolicySelections))
118+
.setLoadBalancingPolicyConfig(switchConfig)
118119
.build());
119120

120121
return Status.OK;
@@ -136,10 +137,10 @@ public void shutdown() {
136137
*/
137138
static final class WrrLocalityConfig {
138139

139-
final PolicySelection childPolicy;
140+
final Object childConfig;
140141

141-
WrrLocalityConfig(PolicySelection childPolicy) {
142-
this.childPolicy = childPolicy;
142+
WrrLocalityConfig(Object childConfig) {
143+
this.childConfig = childConfig;
143144
}
144145

145146
@Override
@@ -151,17 +152,17 @@ public boolean equals(Object o) {
151152
return false;
152153
}
153154
WrrLocalityConfig that = (WrrLocalityConfig) o;
154-
return Objects.equals(childPolicy, that.childPolicy);
155+
return Objects.equals(childConfig, that.childConfig);
155156
}
156157

157158
@Override
158159
public int hashCode() {
159-
return Objects.hashCode(childPolicy);
160+
return Objects.hashCode(childConfig);
160161
}
161162

162163
@Override
163164
public String toString() {
164-
return MoreObjects.toStringHelper(this).add("childPolicy", childPolicy).toString();
165+
return MoreObjects.toStringHelper(this).add("childConfig", childConfig).toString();
165166
}
166167
}
167168
}

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

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,8 @@
2424
import io.grpc.NameResolver.ConfigOrError;
2525
import io.grpc.Status;
2626
import io.grpc.internal.JsonUtil;
27-
import io.grpc.internal.ServiceConfigUtil;
28-
import io.grpc.internal.ServiceConfigUtil.LbConfig;
29-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
27+
import io.grpc.util.GracefulSwitchLoadBalancer;
3028
import io.grpc.xds.WrrLocalityLoadBalancer.WrrLocalityConfig;
31-
import java.util.List;
3229
import java.util.Map;
3330

3431
/**
@@ -62,21 +59,14 @@ public String getPolicyName() {
6259
@Override
6360
public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
6461
try {
65-
List<LbConfig> childConfigCandidates = ServiceConfigUtil.unwrapLoadBalancingConfigList(
62+
ConfigOrError childConfig = GracefulSwitchLoadBalancer.parseLoadBalancingPolicyConfig(
6663
JsonUtil.getListOfObjects(rawConfig, "childPolicy"));
67-
if (childConfigCandidates == null || childConfigCandidates.isEmpty()) {
68-
return ConfigOrError.fromError(Status.INTERNAL.withDescription(
69-
"No child policy in wrr_locality LB policy: "
70-
+ rawConfig));
64+
if (childConfig.getError() != null) {
65+
return ConfigOrError.fromError(Status.INTERNAL
66+
.withDescription("Failed to parse child policy in wrr_locality LB policy: " + rawConfig)
67+
.withCause(childConfig.getError().asRuntimeException()));
7168
}
72-
ConfigOrError selectedConfig =
73-
ServiceConfigUtil.selectLbPolicyFromList(childConfigCandidates,
74-
LoadBalancerRegistry.getDefaultRegistry());
75-
if (selectedConfig.getError() != null) {
76-
return selectedConfig;
77-
}
78-
PolicySelection policySelection = (PolicySelection) selectedConfig.getConfig();
79-
return ConfigOrError.fromConfig(new WrrLocalityConfig(policySelection));
69+
return ConfigOrError.fromConfig(new WrrLocalityConfig(childConfig.getConfig()));
8070
} catch (RuntimeException e) {
8171
return ConfigOrError.fromError(Status.INTERNAL.withCause(e)
8272
.withDescription("Failed to parse wrr_locality LB config: " + rawConfig));

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
5353
import io.grpc.protobuf.ProtoUtils;
5454
import io.grpc.testing.TestMethodDescriptors;
55+
import io.grpc.util.GracefulSwitchLoadBalancer;
5556
import io.grpc.xds.ClusterImplLoadBalancerProvider.ClusterImplConfig;
5657
import io.grpc.xds.Endpoints.DropOverload;
5758
import io.grpc.xds.EnvoyServerProtoData.DownstreamTlsContext;
@@ -119,8 +120,8 @@ public void uncaughtException(Thread t, Throwable e) {
119120
private final FakeClock fakeClock = new FakeClock();
120121
private final Locality locality =
121122
Locality.create("test-region", "test-zone", "test-subzone");
122-
private final PolicySelection roundRobin =
123-
new PolicySelection(new FakeLoadBalancerProvider("round_robin"), null);
123+
private final Object roundRobin = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
124+
new FakeLoadBalancerProvider("round_robin"), null);
124125
private final List<FakeLoadBalancer> downstreamBalancers = new ArrayList<>();
125126
private final FakeTlsContextManager tlsContextManager = new FakeTlsContextManager();
126127
private final LoadStatsManager2 loadStatsManager =

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

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
import io.grpc.internal.GrpcUtil;
6060
import io.grpc.internal.ObjectPool;
6161
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
62+
import io.grpc.util.GracefulSwitchLoadBalancer;
63+
import io.grpc.util.GracefulSwitchLoadBalancerAccessor;
6264
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
6365
import io.grpc.util.OutlierDetectionLoadBalancerProvider;
6466
import io.grpc.xds.ClusterImplLoadBalancerProvider.ClusterImplConfig;
@@ -159,12 +161,14 @@ public void uncaughtException(Thread t, Throwable e) {
159161
private final NameResolverRegistry nsRegistry = new NameResolverRegistry();
160162
private final PolicySelection roundRobin = new PolicySelection(
161163
new FakeLoadBalancerProvider("wrr_locality_experimental"), new WrrLocalityConfig(
162-
new PolicySelection(new FakeLoadBalancerProvider("round_robin"), null)));
164+
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
165+
new FakeLoadBalancerProvider("round_robin"), null)));
163166
private final PolicySelection ringHash = new PolicySelection(
164167
new FakeLoadBalancerProvider("ring_hash_experimental"), new RingHashConfig(10L, 100L));
165168
private final PolicySelection leastRequest = new PolicySelection(
166169
new FakeLoadBalancerProvider("wrr_locality_experimental"), new WrrLocalityConfig(
167-
new PolicySelection(new FakeLoadBalancerProvider("least_request_experimental"),
170+
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
171+
new FakeLoadBalancerProvider("least_request_experimental"),
168172
new LeastRequestConfig(3))));
169173
private final List<FakeLoadBalancer> childBalancers = new ArrayList<>();
170174
private final List<FakeNameResolver> resolvers = new ArrayList<>();
@@ -331,8 +335,9 @@ public void edsClustersWithLeastRequestEndpointLbPolicy() {
331335
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
332336
WrrLocalityConfig wrrLocalityConfig =
333337
(WrrLocalityConfig) clusterImplConfig.childPolicy.getConfig();
334-
assertThat(wrrLocalityConfig.childPolicy.getProvider().getPolicyName()).isEqualTo(
335-
"least_request_experimental");
338+
LoadBalancerProvider childProvider =
339+
GracefulSwitchLoadBalancerAccessor.getChildProvider(wrrLocalityConfig.childConfig);
340+
assertThat(childProvider.getPolicyName()).isEqualTo("least_request_experimental");
336341

337342
assertThat(
338343
childBalancer.addresses.get(0).getAttributes()
@@ -414,8 +419,9 @@ public void edsClustersWithOutlierDetection() {
414419
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
415420
WrrLocalityConfig wrrLocalityConfig =
416421
(WrrLocalityConfig) clusterImplConfig.childPolicy.getConfig();
417-
assertThat(wrrLocalityConfig.childPolicy.getProvider().getPolicyName()).isEqualTo(
418-
"least_request_experimental");
422+
LoadBalancerProvider childProvider =
423+
GracefulSwitchLoadBalancerAccessor.getChildProvider(wrrLocalityConfig.childConfig);
424+
assertThat(childProvider.getPolicyName()).isEqualTo("least_request_experimental");
419425

420426
assertThat(
421427
childBalancer.addresses.get(0).getAttributes()
@@ -484,8 +490,9 @@ public void onlyEdsClusters_receivedEndpoints() {
484490
assertThat(clusterImplConfig1.childPolicy.getConfig()).isInstanceOf(WrrLocalityConfig.class);
485491
WrrLocalityConfig wrrLocalityConfig1 =
486492
(WrrLocalityConfig) clusterImplConfig1.childPolicy.getConfig();
487-
assertThat(wrrLocalityConfig1.childPolicy.getProvider().getPolicyName()).isEqualTo(
488-
"round_robin");
493+
LoadBalancerProvider childProvider1 =
494+
GracefulSwitchLoadBalancerAccessor.getChildProvider(wrrLocalityConfig1.childConfig);
495+
assertThat(childProvider1.getPolicyName()).isEqualTo("round_robin");
489496

490497
PriorityChildConfig priorityChildConfig2 = priorityLbConfig.childConfigs.get(priority2);
491498
assertThat(priorityChildConfig2.ignoreReresolution).isTrue();
@@ -498,8 +505,9 @@ public void onlyEdsClusters_receivedEndpoints() {
498505
assertThat(clusterImplConfig2.childPolicy.getConfig()).isInstanceOf(WrrLocalityConfig.class);
499506
WrrLocalityConfig wrrLocalityConfig2 =
500507
(WrrLocalityConfig) clusterImplConfig1.childPolicy.getConfig();
501-
assertThat(wrrLocalityConfig2.childPolicy.getProvider().getPolicyName()).isEqualTo(
502-
"round_robin");
508+
LoadBalancerProvider childProvider2 =
509+
GracefulSwitchLoadBalancerAccessor.getChildProvider(wrrLocalityConfig2.childConfig);
510+
assertThat(childProvider2.getPolicyName()).isEqualTo("round_robin");
503511

504512
PriorityChildConfig priorityChildConfig3 = priorityLbConfig.childConfigs.get(priority3);
505513
assertThat(priorityChildConfig3.ignoreReresolution).isTrue();
@@ -512,8 +520,9 @@ public void onlyEdsClusters_receivedEndpoints() {
512520
assertThat(clusterImplConfig3.childPolicy.getConfig()).isInstanceOf(WrrLocalityConfig.class);
513521
WrrLocalityConfig wrrLocalityConfig3 =
514522
(WrrLocalityConfig) clusterImplConfig1.childPolicy.getConfig();
515-
assertThat(wrrLocalityConfig3.childPolicy.getProvider().getPolicyName()).isEqualTo(
516-
"round_robin");
523+
LoadBalancerProvider childProvider3 =
524+
GracefulSwitchLoadBalancerAccessor.getChildProvider(wrrLocalityConfig3.childConfig);
525+
assertThat(childProvider3.getPolicyName()).isEqualTo("round_robin");
517526

518527
for (EquivalentAddressGroup eag : childBalancer.addresses) {
519528
if (eag.getAttributes().get(InternalXdsAttributes.ATTR_LOCALITY) == locality1) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import io.grpc.LoadBalancerRegistry;
2727
import io.grpc.NameResolver.ConfigOrError;
2828
import io.grpc.internal.JsonParser;
29-
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
29+
import io.grpc.util.GracefulSwitchLoadBalancer;
3030
import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedPolicySelection;
3131
import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedTargetConfig;
3232
import java.util.Map;
@@ -128,11 +128,13 @@ public ConfigOrError parseLoadBalancingPolicyConfig(Map<String, ?> rawConfig) {
128128
"target_1",
129129
new WeightedPolicySelection(
130130
10,
131-
new PolicySelection(lbProviderFoo, fooConfig)),
131+
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
132+
lbProviderFoo, fooConfig)),
132133
"target_2",
133134
new WeightedPolicySelection(
134135
20,
135-
new PolicySelection(lbProviderBar, barConfig)))));
136+
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
137+
lbProviderBar, barConfig)))));
136138
assertThat(parsedConfig).isEqualTo(expectedConfig);
137139
}
138140
}

0 commit comments

Comments
 (0)