Skip to content

Commit fd6e0df

Browse files
shivaspeaksshivaspeaks
authored
xds: Disable Priority LB child policy retention cache (#12806)
Implements gRFC A115: https://github.com/grpc/proposal/blob/master/A115-remove-priority-lb-child-policy-cache.md
1 parent c62cdef commit fd6e0df

2 files changed

Lines changed: 150 additions & 39 deletions

File tree

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.grpc.Status;
2929
import io.grpc.SynchronizationContext;
3030
import io.grpc.SynchronizationContext.ScheduledHandle;
31+
import io.grpc.internal.GrpcUtil;
3132
import io.grpc.util.ForwardingLoadBalancerHelper;
3233
import io.grpc.util.GracefulSwitchLoadBalancer;
3334
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig;
@@ -73,6 +74,8 @@ final class PriorityLoadBalancer extends LoadBalancer {
7374
private SubchannelPicker currentPicker;
7475
// Set to true if currently in the process of handling resolved addresses.
7576
private boolean handlingResolvedAddresses;
77+
static boolean enablePriorityLbChildPolicyCache =
78+
GrpcUtil.getFlag("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", false);
7679

7780
PriorityLoadBalancer(Helper helper) {
7881
this.helper = checkNotNull(helper, "helper");
@@ -98,7 +101,12 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
98101
if (!prioritySet.contains(priority)) {
99102
ChildLbState childLbState = children.get(priority);
100103
if (childLbState != null) {
101-
childLbState.deactivate();
104+
if (enablePriorityLbChildPolicyCache) {
105+
childLbState.deactivate();
106+
} else {
107+
childLbState.tearDown();
108+
children.remove(priority);
109+
}
102110
}
103111
}
104112
}

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

Lines changed: 141 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,106 @@ public void tearDown() {
149149

150150
@Test
151151
public void acceptResolvedAddresses() {
152+
boolean originalFlagVal = PriorityLoadBalancer.enablePriorityLbChildPolicyCache;
153+
PriorityLoadBalancer.enablePriorityLbChildPolicyCache = true;
154+
try {
155+
SocketAddress socketAddress = new InetSocketAddress(8080);
156+
EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress);
157+
eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1"));
158+
List<EquivalentAddressGroup> addresses = ImmutableList.of(eag);
159+
Attributes attributes =
160+
Attributes.newBuilder().set(Attributes.Key.create("fakeKey"), "fakeValue").build();
161+
Object fooConfig0 = new Object();
162+
PriorityChildConfig priorityChildConfig0 =
163+
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true);
164+
Object barConfig0 = new Object();
165+
PriorityChildConfig priorityChildConfig1 =
166+
new PriorityChildConfig(newChildConfig(barLbProvider, barConfig0), true);
167+
Object fooConfig1 = new Object();
168+
PriorityChildConfig priorityChildConfig2 =
169+
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true);
170+
PriorityLbConfig priorityLbConfig =
171+
new PriorityLbConfig(
172+
ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1,
173+
"p2", priorityChildConfig2),
174+
ImmutableList.of("p0", "p1", "p2"));
175+
Status status = priorityLb.acceptResolvedAddresses(
176+
ResolvedAddresses.newBuilder()
177+
.setAddresses(addresses)
178+
.setAttributes(attributes)
179+
.setLoadBalancingPolicyConfig(priorityLbConfig)
180+
.build());
181+
assertThat(status.getCode()).isEqualTo(Status.Code.OK);
182+
assertThat(fooBalancers).hasSize(1);
183+
assertThat(barBalancers).isEmpty();
184+
LoadBalancer fooBalancer0 = Iterables.getOnlyElement(fooBalancers);
185+
verify(fooBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
186+
ResolvedAddresses addressesReceived = resolvedAddressesCaptor.getValue();
187+
assertThat(addressesReceived.getAddresses()).isEmpty();
188+
assertThat(addressesReceived.getAttributes()).isEqualTo(attributes);
189+
assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(fooConfig0);
190+
191+
// Fail over to p1.
192+
fakeClock.forwardTime(10, TimeUnit.SECONDS);
193+
assertThat(fooBalancers).hasSize(1);
194+
assertThat(barBalancers).hasSize(1);
195+
LoadBalancer barBalancer0 = Iterables.getOnlyElement(barBalancers);
196+
verify(barBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
197+
addressesReceived = resolvedAddressesCaptor.getValue();
198+
assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses())
199+
.containsExactly(socketAddress);
200+
assertThat(addressesReceived.getAttributes()).isEqualTo(attributes);
201+
assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(barConfig0);
202+
203+
// Fail over to p2.
204+
fakeClock.forwardTime(10, TimeUnit.SECONDS);
205+
assertThat(fooBalancers).hasSize(2);
206+
assertThat(barBalancers).hasSize(1);
207+
LoadBalancer fooBalancer1 = Iterables.getLast(fooBalancers);
208+
verify(fooBalancer1).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
209+
addressesReceived = resolvedAddressesCaptor.getValue();
210+
assertThat(addressesReceived.getAddresses()).isEmpty();
211+
assertThat(addressesReceived.getAttributes()).isEqualTo(attributes);
212+
assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(fooConfig1);
213+
214+
// New update: p0 and p2 deleted; p1 config changed.
215+
SocketAddress newSocketAddress = new InetSocketAddress(8081);
216+
EquivalentAddressGroup newEag = new EquivalentAddressGroup(newSocketAddress);
217+
newEag = AddressFilter.setPathFilter(newEag, ImmutableList.of("p1"));
218+
List<EquivalentAddressGroup> newAddresses = ImmutableList.of(newEag);
219+
Object newBarConfig = new Object();
220+
PriorityLbConfig newPriorityLbConfig =
221+
new PriorityLbConfig(
222+
ImmutableMap.of("p1",
223+
new PriorityChildConfig(newChildConfig(barLbProvider, newBarConfig), true)),
224+
ImmutableList.of("p1"));
225+
status = priorityLb.acceptResolvedAddresses(
226+
ResolvedAddresses.newBuilder()
227+
.setAddresses(newAddresses)
228+
.setLoadBalancingPolicyConfig(newPriorityLbConfig)
229+
.build());
230+
assertThat(status.getCode()).isEqualTo(Status.Code.OK);
231+
assertThat(fooBalancers).hasSize(2);
232+
assertThat(barBalancers).hasSize(1);
233+
verify(barBalancer0, times(2)).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
234+
addressesReceived = resolvedAddressesCaptor.getValue();
235+
assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses())
236+
.containsExactly(newSocketAddress);
237+
assertThat(addressesReceived.getAttributes()).isEqualTo(Attributes.EMPTY);
238+
assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(newBarConfig);
239+
verify(fooBalancer0, never()).shutdown();
240+
verify(fooBalancer1, never()).shutdown();
241+
fakeClock.forwardTime(15, TimeUnit.MINUTES);
242+
verify(fooBalancer0).shutdown();
243+
verify(fooBalancer1).shutdown();
244+
verify(barBalancer0, never()).shutdown();
245+
} finally {
246+
PriorityLoadBalancer.enablePriorityLbChildPolicyCache = originalFlagVal;
247+
}
248+
}
249+
250+
@Test
251+
public void acceptResolvedAddresses_cacheDisabled() {
152252
SocketAddress socketAddress = new InetSocketAddress(8080);
153253
EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress);
154254
eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1"));
@@ -233,9 +333,6 @@ public void acceptResolvedAddresses() {
233333
.containsExactly(newSocketAddress);
234334
assertThat(addressesReceived.getAttributes()).isEqualTo(Attributes.EMPTY);
235335
assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(newBarConfig);
236-
verify(fooBalancer0, never()).shutdown();
237-
verify(fooBalancer1, never()).shutdown();
238-
fakeClock.forwardTime(15, TimeUnit.MINUTES);
239336
verify(fooBalancer0).shutdown();
240337
verify(fooBalancer1).shutdown();
241338
verify(barBalancer0, never()).shutdown();
@@ -297,41 +394,47 @@ public void acceptResolvedAddresses_propagatesChildFailures() {
297394

298395
@Test
299396
public void handleNameResolutionError() {
300-
Object fooConfig0 = new Object();
301-
PriorityChildConfig priorityChildConfig0 =
302-
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true);
303-
Object fooConfig1 = new Object();
304-
PriorityChildConfig priorityChildConfig1 =
305-
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true);
306-
307-
PriorityLbConfig priorityLbConfig =
308-
new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0"));
309-
priorityLb.acceptResolvedAddresses(
310-
ResolvedAddresses.newBuilder()
311-
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
312-
.setLoadBalancingPolicyConfig(priorityLbConfig)
313-
.build());
314-
LoadBalancer fooLb0 = Iterables.getOnlyElement(fooBalancers);
315-
Status status = Status.DATA_LOSS.withDescription("fake error");
316-
priorityLb.handleNameResolutionError(status);
317-
verify(fooLb0).handleNameResolutionError(status);
318-
319-
priorityLbConfig =
320-
new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1"));
321-
priorityLb.acceptResolvedAddresses(
322-
ResolvedAddresses.newBuilder()
323-
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
324-
.setLoadBalancingPolicyConfig(priorityLbConfig)
325-
.build());
326-
assertThat(fooBalancers).hasSize(2);
327-
LoadBalancer fooLb1 = Iterables.getLast(fooBalancers);
328-
status = Status.UNAVAILABLE.withDescription("fake error");
329-
priorityLb.handleNameResolutionError(status);
330-
// fooLb0 is deactivated but not yet deleted. However, because it is delisted by the latest
331-
// address update, name resolution error will not be propagated to it.
332-
verify(fooLb0, never()).shutdown();
333-
verify(fooLb0, never()).handleNameResolutionError(status);
334-
verify(fooLb1).handleNameResolutionError(status);
397+
boolean originalFlagVal = PriorityLoadBalancer.enablePriorityLbChildPolicyCache;
398+
PriorityLoadBalancer.enablePriorityLbChildPolicyCache = true;
399+
try {
400+
Object fooConfig0 = new Object();
401+
PriorityChildConfig priorityChildConfig0 =
402+
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true);
403+
Object fooConfig1 = new Object();
404+
PriorityChildConfig priorityChildConfig1 =
405+
new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true);
406+
407+
PriorityLbConfig priorityLbConfig =
408+
new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0"));
409+
priorityLb.acceptResolvedAddresses(
410+
ResolvedAddresses.newBuilder()
411+
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
412+
.setLoadBalancingPolicyConfig(priorityLbConfig)
413+
.build());
414+
LoadBalancer fooLb0 = Iterables.getOnlyElement(fooBalancers);
415+
Status status = Status.DATA_LOSS.withDescription("fake error");
416+
priorityLb.handleNameResolutionError(status);
417+
verify(fooLb0).handleNameResolutionError(status);
418+
419+
priorityLbConfig =
420+
new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1"));
421+
priorityLb.acceptResolvedAddresses(
422+
ResolvedAddresses.newBuilder()
423+
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
424+
.setLoadBalancingPolicyConfig(priorityLbConfig)
425+
.build());
426+
assertThat(fooBalancers).hasSize(2);
427+
LoadBalancer fooLb1 = Iterables.getLast(fooBalancers);
428+
status = Status.UNAVAILABLE.withDescription("fake error");
429+
priorityLb.handleNameResolutionError(status);
430+
// fooLb0 is deactivated but not yet deleted. However, because it is delisted by the latest
431+
// address update, name resolution error will not be propagated to it.
432+
verify(fooLb0, never()).shutdown();
433+
verify(fooLb0, never()).handleNameResolutionError(status);
434+
verify(fooLb1).handleNameResolutionError(status);
435+
} finally {
436+
PriorityLoadBalancer.enablePriorityLbChildPolicyCache = originalFlagVal;
437+
}
335438
}
336439

337440
@Test

0 commit comments

Comments
 (0)