From 197bd09580946d4d072136f2ef269d88379fe81f Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 25 Aug 2022 09:09:19 -0700 Subject: [PATCH] core: outlier detection max ejection logic update 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 d6ada1dbd1d..87f9bdce2b3 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 void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeNanos) 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 void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeNanos) // 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 b60c685fa5d..c11e6328f34 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 void successRateTwoOutliers() { @Test public void successRateThreeOutliers_maxEjectionPercentage() { OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder() - .setMaxEjectionPercent(20) + .setMaxEjectionPercent(30) .setSuccessRateEjection( new SuccessRateEjection.Builder() .setMinimumHosts(3)