Skip to content

Commit 80cc988

Browse files
authored
xds: Use acceptResolvedAddresses() for WeightedTarget children (#12053)
Convert the tests to use acceptResolvedAddresses() as well.
1 parent 3f5fdf1 commit 80cc988

File tree

2 files changed

+26
-12
lines changed

2 files changed

+26
-12
lines changed

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,19 @@ public Status acceptResolvedAddressesInternal(ResolvedAddresses resolvedAddresse
8787
}
8888
}
8989
targets = newTargets;
90+
Status status = Status.OK;
9091
for (String targetName : targets.keySet()) {
91-
childBalancers.get(targetName).handleResolvedAddresses(
92+
Status newStatus = childBalancers.get(targetName).acceptResolvedAddresses(
9293
resolvedAddresses.toBuilder()
9394
.setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), targetName))
9495
.setLoadBalancingPolicyConfig(targets.get(targetName).childConfig)
9596
.setAttributes(resolvedAddresses.getAttributes().toBuilder()
9697
.set(CHILD_NAME, targetName)
9798
.build())
9899
.build());
100+
if (!newStatus.isOk()) {
101+
status = newStatus;
102+
}
99103
}
100104

101105
// Cleanup removed targets.
@@ -108,7 +112,7 @@ public Status acceptResolvedAddressesInternal(ResolvedAddresses resolvedAddresse
108112
childBalancers.keySet().retainAll(targets.keySet());
109113
childHelpers.keySet().retainAll(targets.keySet());
110114
updateOverallBalancingState();
111-
return Status.OK;
115+
return status;
112116
}
113117

114118
@Override

xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java

+20-10
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public String getPolicyName() {
113113
public LoadBalancer newLoadBalancer(Helper helper) {
114114
childHelpers.add(helper);
115115
LoadBalancer childBalancer = mock(LoadBalancer.class);
116+
when(childBalancer.acceptResolvedAddresses(any())).thenReturn(Status.OK);
116117
childBalancers.add(childBalancer);
117118
fooLbCreated++;
118119
return childBalancer;
@@ -139,6 +140,7 @@ public String getPolicyName() {
139140
public LoadBalancer newLoadBalancer(Helper helper) {
140141
childHelpers.add(helper);
141142
LoadBalancer childBalancer = mock(LoadBalancer.class);
143+
when(childBalancer.acceptResolvedAddresses(any())).thenReturn(Status.OK);
142144
childBalancers.add(childBalancer);
143145
barLbCreated++;
144146
return childBalancer;
@@ -180,7 +182,7 @@ public void tearDown() {
180182
}
181183

182184
@Test
183-
public void handleResolvedAddresses() {
185+
public void acceptResolvedAddresses() {
184186
ArgumentCaptor<ResolvedAddresses> resolvedAddressesCaptor =
185187
ArgumentCaptor.forClass(ResolvedAddresses.class);
186188
Attributes.Key<Object> fakeKey = Attributes.Key.create("fake_key");
@@ -203,20 +205,21 @@ public void handleResolvedAddresses() {
203205
eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2"));
204206
EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]);
205207
eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3"));
206-
weightedTargetLb.handleResolvedAddresses(
208+
Status status = weightedTargetLb.acceptResolvedAddresses(
207209
ResolvedAddresses.newBuilder()
208210
.setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3))
209211
.setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build())
210212
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
211213
.build());
214+
assertThat(status.isOk()).isTrue();
212215
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
213216
assertThat(childBalancers).hasSize(4);
214217
assertThat(childHelpers).hasSize(4);
215218
assertThat(fooLbCreated).isEqualTo(2);
216219
assertThat(barLbCreated).isEqualTo(2);
217220

218221
for (int i = 0; i < childBalancers.size(); i++) {
219-
verify(childBalancers.get(i)).handleResolvedAddresses(resolvedAddressesCaptor.capture());
222+
verify(childBalancers.get(i)).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
220223
ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue();
221224
assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]);
222225
assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue);
@@ -226,6 +229,11 @@ public void handleResolvedAddresses() {
226229
.containsExactly(socketAddresses[i]);
227230
}
228231

232+
// Even when a child return an error from the update, the other children should still receive
233+
// their updates.
234+
Status acceptReturnStatus = Status.UNAVAILABLE.withDescription("Didn't like something");
235+
when(childBalancers.get(2).acceptResolvedAddresses(any())).thenReturn(acceptReturnStatus);
236+
229237
// Update new weighted target config for a typical workflow.
230238
// target0 removed. target1, target2, target3 changed weight and config. target4 added.
231239
int[] newWeights = new int[]{11, 22, 33, 44};
@@ -243,11 +251,12 @@ public void handleResolvedAddresses() {
243251
"target4",
244252
new WeightedPolicySelection(
245253
newWeights[3], newChildConfig(fooLbProvider, newConfigs[3])));
246-
weightedTargetLb.handleResolvedAddresses(
254+
status = weightedTargetLb.acceptResolvedAddresses(
247255
ResolvedAddresses.newBuilder()
248256
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
249257
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets))
250258
.build());
259+
assertThat(status.getCode()).isEqualTo(acceptReturnStatus.getCode());
251260
verify(helper, atLeast(2))
252261
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
253262
assertThat(childBalancers).hasSize(5);
@@ -258,7 +267,7 @@ public void handleResolvedAddresses() {
258267
verify(childBalancers.get(0)).shutdown();
259268
for (int i = 1; i < childBalancers.size(); i++) {
260269
verify(childBalancers.get(i), atLeastOnce())
261-
.handleResolvedAddresses(resolvedAddressesCaptor.capture());
270+
.acceptResolvedAddresses(resolvedAddressesCaptor.capture());
262271
assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig())
263272
.isEqualTo(newConfigs[i - 1]);
264273
}
@@ -286,7 +295,7 @@ public void handleNameResolutionError() {
286295
"target2", weightedLbConfig2,
287296
// {foo, 40, config3}
288297
"target3", weightedLbConfig3);
289-
weightedTargetLb.handleResolvedAddresses(
298+
weightedTargetLb.acceptResolvedAddresses(
290299
ResolvedAddresses.newBuilder()
291300
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
292301
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@@ -313,7 +322,7 @@ public void balancingStateUpdatedFromChildBalancers() {
313322
"target2", weightedLbConfig2,
314323
// {foo, 40, config3}
315324
"target3", weightedLbConfig3);
316-
weightedTargetLb.handleResolvedAddresses(
325+
weightedTargetLb.acceptResolvedAddresses(
317326
ResolvedAddresses.newBuilder()
318327
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
319328
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@@ -395,7 +404,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() {
395404
Map<String, WeightedPolicySelection> targets = ImmutableMap.of(
396405
"target0", weightedLbConfig0,
397406
"target1", weightedLbConfig1);
398-
weightedTargetLb.handleResolvedAddresses(
407+
weightedTargetLb.acceptResolvedAddresses(
399408
ResolvedAddresses.newBuilder()
400409
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
401410
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@@ -421,7 +430,7 @@ public void noDuplicateOverallBalancingStateUpdate() {
421430
weights[0], newChildConfig(fakeLbProvider, configs[0])),
422431
"target3", new WeightedPolicySelection(
423432
weights[3], newChildConfig(fakeLbProvider, configs[3])));
424-
weightedTargetLb.handleResolvedAddresses(
433+
weightedTargetLb.acceptResolvedAddresses(
425434
ResolvedAddresses.newBuilder()
426435
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
427436
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@@ -470,9 +479,10 @@ static class FakeLoadBalancer extends LoadBalancer {
470479
}
471480

472481
@Override
473-
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
482+
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
474483
helper.updateBalancingState(
475484
TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL)));
485+
return Status.OK;
476486
}
477487

478488
@Override

0 commit comments

Comments
 (0)