From 2289956ec7159687b5ec5863ec494194df864efa Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Mon, 19 Sep 2022 20:20:08 -0700 Subject: [PATCH] core: outlier_detection LB to use acceptResolvedAddresses() (#9557) Switch OutlierDetectionLoadBalancer to implement acceptResolvedAddresses() to allow for the eventual deprecation of handleResolvedAddresses(). --- .../util/OutlierDetectionLoadBalancer.java | 3 +- .../OutlierDetectionLoadBalancerTest.java | 71 ++++++++++--------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 87f9bdce2b3..e7d93d9d52f 100644 --- a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -89,7 +89,7 @@ public OutlierDetectionLoadBalancer(Helper helper, TimeProvider timeProvider) { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { OutlierDetectionLoadBalancerConfig config = (OutlierDetectionLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -142,6 +142,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { switchLb.handleResolvedAddresses( resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(config.childPolicy.getConfig()) .build()); + return true; } @Override diff --git a/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java b/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java index 4e94b0089fd..ccf3d40cdb6 100644 --- a/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java @@ -202,7 +202,7 @@ public void handleNameResolutionError_noChildLb() { @Test public void handleNameResolutionError_withChildLb() { - loadBalancer.handleResolvedAddresses(buildResolvedAddress( + loadBalancer.acceptResolvedAddresses(buildResolvedAddress( new OutlierDetectionLoadBalancerConfig.Builder() .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(), @@ -217,7 +217,7 @@ public void handleNameResolutionError_withChildLb() { */ @Test public void shutdown() { - loadBalancer.handleResolvedAddresses(buildResolvedAddress( + loadBalancer.acceptResolvedAddresses(buildResolvedAddress( new OutlierDetectionLoadBalancerConfig.Builder() .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(), @@ -230,14 +230,14 @@ public void shutdown() { * Base case for accepting new resolved addresses. */ @Test - public void handleResolvedAddresses() { + public void acceptResolvedAddresses() { OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder() .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(); ResolvedAddresses resolvedAddresses = buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress)); - loadBalancer.handleResolvedAddresses(resolvedAddresses); + loadBalancer.acceptResolvedAddresses(resolvedAddresses); // Handling of resolved addresses is delegated verify(mockChildLb).handleResolvedAddresses( @@ -256,14 +256,14 @@ public void handleResolvedAddresses() { * Outlier detection first enabled, then removed. */ @Test - public void handleResolvedAddresses_outlierDetectionDisabled() { + public void acceptResolvedAddresses_outlierDetectionDisabled() { OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder() .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(); ResolvedAddresses resolvedAddresses = buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress)); - loadBalancer.handleResolvedAddresses(resolvedAddresses); + loadBalancer.acceptResolvedAddresses(resolvedAddresses); fakeClock.forwardTime(15, TimeUnit.SECONDS); @@ -272,7 +272,7 @@ public void handleResolvedAddresses_outlierDetectionDisabled() { config = new OutlierDetectionLoadBalancerConfig.Builder().setChildPolicy( new PolicySelection(mockChildLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress))); // Pending task should be gone since OD is disabled. @@ -284,14 +284,14 @@ public void handleResolvedAddresses_outlierDetectionDisabled() { * Tests different scenarios when the timer interval in the config changes. */ @Test - public void handleResolvedAddresses_intervalUpdate() { + public void acceptResolvedAddresses_intervalUpdate() { OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder() .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(); ResolvedAddresses resolvedAddresses = buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress)); - loadBalancer.handleResolvedAddresses(resolvedAddresses); + loadBalancer.acceptResolvedAddresses(resolvedAddresses); // Config update has doubled the interval config = new OutlierDetectionLoadBalancerConfig.Builder() @@ -299,7 +299,7 @@ public void handleResolvedAddresses_intervalUpdate() { .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(mockChildLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress))); // If the timer has not run yet the task is just rescheduled to run after the new delay. @@ -320,7 +320,7 @@ public void handleResolvedAddresses_intervalUpdate() { // not changing in the config, the next task due time should remain unchanged. fakeClock.forwardTime(4, TimeUnit.SECONDS); task = fakeClock.getPendingTasks().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( buildResolvedAddress(config, new EquivalentAddressGroup(mockSocketAddress))); assertThat(task.dueTimeNanos).isEqualTo(config.intervalNanos + config.intervalNanos + 1); } @@ -334,7 +334,7 @@ public void delegatePick() throws Exception { .setSuccessRateEjection(new SuccessRateEjection.Builder().build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers.get(0))); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers.get(0))); // Make one of the subchannels READY. final Subchannel readySubchannel = subchannels.values().iterator().next(); @@ -361,7 +361,7 @@ public void successRateNoOutliers() { new SuccessRateEjection.Builder().setMinimumHosts(3).setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(), 7); @@ -385,7 +385,7 @@ public void successRateOneOutlier() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -410,7 +410,7 @@ public void successRateOneOutlier_configChange() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -430,7 +430,7 @@ public void successRateOneOutlier_configChange() { .setEnforcementPercentage(0).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel2, Status.DEADLINE_EXCEEDED), 8); @@ -455,7 +455,7 @@ public void successRateOneOutlier_unejected() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -488,7 +488,7 @@ public void successRateOneOutlier_notEnoughVolume() { .setRequestVolume(20).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); // We produce an outlier, but don't give it enough calls to reach the minimum volume. generateLoad( @@ -516,7 +516,7 @@ public void successRateOneOutlier_notEnoughAddressesWithVolume() { .setRequestVolume(20).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad( ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), @@ -546,7 +546,7 @@ public void successRateOneOutlier_enforcementPercentage() { .build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -571,7 +571,7 @@ public void successRateTwoOutliers() { .setStdevFactor(1).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of( subchannel1, Status.DEADLINE_EXCEEDED, @@ -600,7 +600,7 @@ public void successRateThreeOutliers_maxEjectionPercentage() { .setStdevFactor(1).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of( subchannel1, Status.DEADLINE_EXCEEDED, @@ -634,7 +634,7 @@ public void failurePercentageNoOutliers() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); // By default all calls will return OK. generateLoad(ImmutableMap.of(), 7); @@ -659,7 +659,7 @@ public void failurePercentageOneOutlier() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -683,7 +683,7 @@ public void failurePercentageOneOutlier_notEnoughVolume() { .setRequestVolume(100).build()) // We won't produce this much volume... .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -708,7 +708,7 @@ public void failurePercentageOneOutlier_notEnoughAddressesWithVolume() { .setRequestVolume(20).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad( ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), @@ -738,7 +738,7 @@ public void failurePercentageOneOutlier_enforcementPercentage() { .build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -767,7 +767,7 @@ public void successRateAndFailurePercentageThreeOutliers() { .build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); // Three subchannels with problems, but one only has a single call that failed. // This is not enough for success rate to catch, but failure percentage is @@ -804,7 +804,7 @@ public void subchannelUpdateAddress_singleReplaced() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -851,7 +851,7 @@ public void subchannelUpdateAddress_singleReplacedWithMultiple() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(), 7); @@ -895,7 +895,7 @@ public void subchannelUpdateAddress_multipleReplacedWithSingle() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(fakeLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 6); @@ -957,7 +957,7 @@ public void successRateAndFailurePercentage_noOutliers() { .setRequestVolume(10).build()) .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(), 7); @@ -984,7 +984,7 @@ public void successRateAndFailurePercentage_successRateOutlier() { .setEnforcementPercentage(0).build()) // Configured, but not enforcing. .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -1011,7 +1011,7 @@ public void successRateAndFailurePercentage_errorPercentageOutlier() { .setRequestVolume(10).build()) // Configured, but not enforcing. .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build(); - loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers)); + loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers)); generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7); @@ -1127,7 +1127,7 @@ private static final class FakeLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { subchannelList = new ArrayList<>(); for (EquivalentAddressGroup eag: resolvedAddresses.getAddresses()) { Subchannel subchannel = helper.createSubchannel(CreateSubchannelArgs.newBuilder() @@ -1136,6 +1136,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { subchannel.start(mock(SubchannelStateListener.class)); deliverSubchannelState(READY); } + return true; } @Override