Skip to content

Commit beb3232

Browse files
authored
xds: immediately update picker when circuit breakers/drop policies change (#7600)
Previously the EDS LB policies does not propagate an updated picker that uses the new circuit breaker threshold and drop policies when those values change. The result is new circuit breaker/drop policies are not dynamically applied to new RPCs unless subchannel state has changed. This change fixes this problem. Whenever the EDS LB policy receives an config update, the immediately updates the picker with corresponding circuit breakers and drop policies to the channel so that the channel is alway picking up the latest configuration.
1 parent a589f52 commit beb3232

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
2121
import static io.grpc.xds.XdsLbPolicies.LRS_POLICY_NAME;
2222
import static io.grpc.xds.XdsLbPolicies.PRIORITY_POLICY_NAME;
23+
import static io.grpc.xds.XdsSubchannelPickers.BUFFER_PICKER;
2324

2425
import com.google.common.annotations.VisibleForTesting;
2526
import io.grpc.Attributes;
@@ -370,6 +371,8 @@ private void propagateResourceError(Status error) {
370371

371372
private final class RequestLimitingLbHelper extends ForwardingLoadBalancerHelper {
372373
private final Helper helper;
374+
private ConnectivityState currentState = ConnectivityState.CONNECTING;
375+
private SubchannelPicker currentPicker = BUFFER_PICKER;
373376
private List<DropOverload> dropPolicies = Collections.emptyList();
374377
private long maxConcurrentRequests = DEFAULT_PER_CLUSTER_MAX_CONCURRENT_REQUESTS;
375378

@@ -380,6 +383,8 @@ private RequestLimitingLbHelper(Helper helper) {
380383
@Override
381384
public void updateBalancingState(
382385
ConnectivityState newState, final SubchannelPicker newPicker) {
386+
currentState = newState;
387+
currentPicker = newPicker;
383388
SubchannelPicker picker = new RequestLimitingSubchannelPicker(
384389
newPicker, dropPolicies, maxConcurrentRequests);
385390
delegate().updateBalancingState(newState, picker);
@@ -392,10 +397,12 @@ protected Helper delegate() {
392397

393398
private void updateDropPolicies(List<DropOverload> dropOverloads) {
394399
dropPolicies = dropOverloads;
400+
updateBalancingState(currentState, currentPicker);
395401
}
396402

397403
private void updateMaxConcurrentRequests(long maxConcurrentRequests) {
398404
this.maxConcurrentRequests = maxConcurrentRequests;
405+
updateBalancingState(currentState, currentPicker);
399406
}
400407

401408
private final class RequestLimitingSubchannelPicker extends SubchannelPicker {

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ public void handleDrops() {
441441
new EdsConfig(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_NAME, null,
442442
weightedTargetSelection, fakeRoundRobinSelection))
443443
.build());
444-
when(mockRandom.nextInt(anyInt())).thenReturn(499_999, 1_000_000);
444+
when(mockRandom.nextInt(anyInt())).thenReturn(499_999, 999_999, 1_000_000);
445445
EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1");
446446
LocalityLbEndpoints localityLbEndpoints1 =
447447
buildLocalityLbEndpoints(1, 10, Collections.singletonMap(endpoint1, true));
@@ -465,6 +465,18 @@ public void handleDrops() {
465465
assertThat(xdsClient.clusterStats.get(EDS_SERVICE_NAME).categorizedDrops.get("throttle"))
466466
.isEqualTo(1);
467467

468+
// Dynamically update drop policies.
469+
xdsClient.deliverClusterLoadAssignment(
470+
EDS_SERVICE_NAME,
471+
Collections.singletonList(new DropOverload("lb", 1_000_000)),
472+
Collections.singletonMap(locality1, localityLbEndpoints1));
473+
result = currentPicker.pickSubchannel(mock(PickSubchannelArgs.class));
474+
assertThat(result.getStatus().isOk()).isFalse();
475+
assertThat(result.getStatus().getCode()).isEqualTo(Code.UNAVAILABLE);
476+
assertThat(result.getStatus().getDescription()).isEqualTo("Dropped: lb");
477+
assertThat(xdsClient.clusterStats.get(EDS_SERVICE_NAME).categorizedDrops.get("lb"))
478+
.isEqualTo(1);
479+
468480
result = currentPicker.pickSubchannel(mock(PickSubchannelArgs.class));
469481
assertThat(result.getStatus().isOk()).isTrue();
470482
assertThat(result.getSubchannel()).isSameInstanceAs(subchannel);
@@ -516,6 +528,23 @@ public void maxConcurrentRequests_appliedByLbConfig() {
516528
assertThat(result.getStatus().getDescription())
517529
.isEqualTo("Cluster max concurrent requests limit exceeded");
518530
assertThat(xdsClient.clusterStats.get(EDS_SERVICE_NAME).totalDrops).isEqualTo(1L);
531+
532+
// Dynamically increment circuit breakers max_concurrent_requests threshold.
533+
maxConcurrentRequests = 101L;
534+
loadBalancer.handleResolvedAddresses(
535+
ResolvedAddresses.newBuilder()
536+
.setAddresses(Collections.<EquivalentAddressGroup>emptyList())
537+
.setAttributes(
538+
Attributes.newBuilder().set(XdsAttributes.XDS_CLIENT_POOL, xdsClientPool).build())
539+
.setLoadBalancingPolicyConfig(
540+
new EdsConfig(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_NAME, maxConcurrentRequests,
541+
weightedTargetSelection, fakeRoundRobinSelection))
542+
.build());
543+
544+
result = currentPicker.pickSubchannel(mock(PickSubchannelArgs.class));
545+
assertThat(result.getStatus().isOk()).isTrue();
546+
assertThat(result.getSubchannel()).isSameInstanceAs(subchannel);
547+
assertThat(xdsClient.clusterStats.get(EDS_SERVICE_NAME).totalDrops).isEqualTo(1L);
519548
}
520549

521550
@Test

0 commit comments

Comments
 (0)