Skip to content

Commit

Permalink
core: outlier_detection LB to use acceptResolvedAddresses() (grpc#9557)
Browse files Browse the repository at this point in the history 
Switch OutlierDetectionLoadBalancer to implement
acceptResolvedAddresses() to allow for the eventual deprecation of
handleResolvedAddresses().
  • Loading branch information
@temawi @larry-safran
temawi authored and larry-safran committed Oct 6, 2022
1 parent 2316836 commit 760cfb2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 36 deletions.
3 changes: 2 additions & 1 deletion core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -142,6 +142,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
switchLb.handleResolvedAddresses(
resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(config.childPolicy.getConfig())
.build());
return true;
}

@Override
Expand Down
71 changes: 36 additions & 35 deletions core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(
Expand All @@ -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);

Expand All @@ -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.
Expand All @@ -284,22 +284,22 @@ 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()
.setIntervalNanos(config.intervalNanos * 2)
.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.
Expand All @@ -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);
}
Expand All @@ -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();
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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),
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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()
Expand All @@ -1136,6 +1136,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
subchannel.start(mock(SubchannelStateListener.class));
deliverSubchannelState(READY);
}
return true;
}

@Override
Expand Down

0 comments on commit 760cfb2

Please sign in to comment.