From d3331d953d48071778455dd182c1fff8d6d8845c Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 25 Aug 2022 10:48:28 -0700 Subject: [PATCH] core: outlier detection max ejection logic update (#9492) Stop further ejection if the ejection percentage is lesser than or equal to the maximum ejection percentage. Previously this was only done when the current ejection percentage was lesser than the maximum. --- .../util/OutlierDetectionLoadBalancer.java | 18 ++++++++++-------- .../util/OutlierDetectionLoadBalancerTest.java | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index d6ada1dbd1..87f9bdce2b 100644 --- a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -733,10 +733,11 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer { mean - stdev * (config.successRateEjection.stdevFactor / 1000f); for (AddressTracker tracker : trackersWithVolume) { - // If we are above the max ejection percentage, don't eject any more. This will allow the - // total ejections to go one above the max, but at the same time it assures at least one - // ejection, which the spec calls for. This behavior matches what Envoy proxy does. - if (trackerMap.ejectionPercentage() > config.maxEjectionPercent) { + // If we are above or equal to the max ejection percentage, don't eject any more. This will + // allow the total ejections to go one above the max, but at the same time it assures at + // least one ejection, which the spec calls for. This behavior matches what Envoy proxy + // does. + if (trackerMap.ejectionPercentage() >= config.maxEjectionPercent) { return; } @@ -797,10 +798,11 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer { // If this address does not have enough volume to be considered, skip to the next one. for (AddressTracker tracker : trackersWithVolume) { - // If we are above the max ejection percentage, don't eject any more. This will allow the - // total ejections to go one above the max, but at the same time it assures at least one - // ejection, which the spec calls for. This behavior matches what Envoy proxy does. - if (trackerMap.ejectionPercentage() > config.maxEjectionPercent) { + // If we are above or equal to the max ejection percentage, don't eject any more. This will + // allow the total ejections to go one above the max, but at the same time it assures at + // least one ejection, which the spec calls for. This behavior matches what Envoy proxy + // does. + if (trackerMap.ejectionPercentage() >= config.maxEjectionPercent) { return; } diff --git a/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java b/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java index b60c685fa5..c11e6328f3 100644 --- a/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java @@ -585,7 +585,7 @@ public class OutlierDetectionLoadBalancerTest { @Test public void successRateThreeOutliers_maxEjectionPercentage() { OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder() - .setMaxEjectionPercent(20) + .setMaxEjectionPercent(30) .setSuccessRateEjection( new SuccessRateEjection.Builder() .setMinimumHosts(3)