From 4cc9c8debe1a98f7a5b1d0884538a71ac7de620b Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Tue, 16 Aug 2022 15:52:28 -0700 Subject: [PATCH] Various review comment fixes. --- .../util/OutlierDetectionLoadBalancer.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index e343d0e366fe..02b31390142b 100644 --- a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -654,6 +654,9 @@ void maybeUnejectOutliers(Long detectionTimerStartNanos) { * with the next ejection. */ double nextEjectionPercentage() { + if (trackerMap.isEmpty()) { + return 0; + } int totalAddresses = 0; int ejectedAddresses = 0; for (AddressTracker tracker : trackerMap.values()) { @@ -672,12 +675,10 @@ void maybeUnejectOutliers(Long detectionTimerStartNanos) { */ interface OutlierEjectionAlgorithm { - /** - * Is the given {@link EquivalentAddressGroup} an outlier based on the past call results stored - * in {@link AddressTracker}. - */ + /** Eject any outlier addresses. */ void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeMillis); + /** Builds a list of algorithms that are enabled in the given config. */ @Nullable static List forConfig(OutlierDetectionLoadBalancerConfig config) { ImmutableList.Builder algoListBuilder = ImmutableList.builder(); @@ -724,15 +725,16 @@ public void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeNanos) double mean = mean(successRates); double stdev = standardDeviation(successRates, mean); + double requiredSuccessRate = + mean - stdev * (config.successRateEjection.stdevFactor / 1000f); + for (AddressTracker tracker : trackersWithVolume) { - // If an ejection now would take us past the max configured ejection percentagem stop here. + // If an ejection now would take us past the max configured ejection percentage, stop here. if (trackerMap.nextEjectionPercentage() > config.maxEjectionPercent) { return; } // If success rate is below the threshold, eject the address. - double requiredSuccessRate = - mean - stdev * (config.successRateEjection.stdevFactor / 1000f); if (tracker.successRate() < requiredSuccessRate) { // Only eject some addresses based on the enforcement percentage. if (new Random().nextInt(100) < config.successRateEjection.enforcementPercentage) { @@ -896,7 +898,7 @@ public Builder setMaxEjectionPercent(Integer maxEjectionPercent) { return this; } - /** Set to enable success rate eejction. */ + /** Set to enable success rate ejection. */ public Builder setSuccessRateEjection( SuccessRateEjection successRateEjection) { this.successRateEjection = successRateEjection; @@ -962,6 +964,7 @@ public Builder setStdevFactor(Integer stdevFactor) { /** Only eject this percentage of outliers. */ public Builder setEnforcementPercentage(Integer enforcementPercentage) { checkArgument(enforcementPercentage != null); + checkArgument(enforcementPercentage >= 0 && enforcementPercentage <= 100); this.enforcementPercentage = enforcementPercentage; return this; } @@ -969,6 +972,7 @@ public Builder setEnforcementPercentage(Integer enforcementPercentage) { /** The minimum amount of hosts needed for success rate ejection. */ public Builder setMinimumHosts(Integer minimumHosts) { checkArgument(minimumHosts != null); + checkArgument(minimumHosts >= 0); this.minimumHosts = minimumHosts; return this; } @@ -976,6 +980,7 @@ public Builder setMinimumHosts(Integer minimumHosts) { /** The minimum address request volume to be considered for success rate ejection. */ public Builder setRequestVolume(Integer requestVolume) { checkArgument(requestVolume != null); + checkArgument(requestVolume >= 0); this.requestVolume = requestVolume; return this; } @@ -1013,6 +1018,7 @@ public static class Builder { /** The failure percentage that will result in an address being considered an outlier. */ public Builder setThreshold(Integer threshold) { checkArgument(threshold != null); + checkArgument(threshold >= 0 && threshold <= 100); this.threshold = threshold; return this; } @@ -1020,6 +1026,7 @@ public Builder setThreshold(Integer threshold) { /** Only eject this percentage of outliers. */ public Builder setEnforcementPercentage(Integer enforcementPercentage) { checkArgument(enforcementPercentage != null); + checkArgument(enforcementPercentage >= 0 && enforcementPercentage <= 100); this.enforcementPercentage = enforcementPercentage; return this; } @@ -1027,6 +1034,7 @@ public Builder setEnforcementPercentage(Integer enforcementPercentage) { /** The minimum amount of host for failure percentage ejection to be enabled. */ public Builder setMinimumHosts(Integer minimumHosts) { checkArgument(minimumHosts != null); + checkArgument(minimumHosts >= 0); this.minimumHosts = minimumHosts; return this; } @@ -1037,6 +1045,7 @@ public Builder setMinimumHosts(Integer minimumHosts) { */ public Builder setRequestVolume(Integer requestVolume) { checkArgument(requestVolume != null); + checkArgument(requestVolume >= 0); this.requestVolume = requestVolume; return this; } @@ -1049,11 +1058,8 @@ public FailurePercentageEjection build() { } } - /** - * Determine if outlier detection is at all enabled in this config. - */ + /** Determine if any outlier detection algorithms are enabled in the config. */ boolean outlierDetectionEnabled() { - // One of the two supported algorithms needs to be configured. return successRateEjection != null || failurePercentageEjection != null; } }