Support running multiple ejection algorithms.

grpc · Aug 15, 2022 · 0191a3b · 0191a3b
1 parent 4278053
commit 0191a3b
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 9 deletions.
diff --git a/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/core/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java
@@ -24,6 +24,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ForwardingMap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import io.grpc.Attributes;
@@ -164,8 +165,9 @@ public void run() {
 
       trackerMap.swapCounters();
 
-      OutlierEjectionAlgorithm.forConfig(config)
-          .ejectOutliers(trackerMap, detectionTimerStartNanos);
+      for (OutlierEjectionAlgorithm algo : OutlierEjectionAlgorithm.forConfig(config)) {
+        algo.ejectOutliers(trackerMap, detectionTimerStartNanos);
+      }
 
       trackerMap.maybeUnejectOutliers(detectionTimerStartNanos);
     }
@@ -649,14 +651,15 @@ interface OutlierEjectionAlgorithm {
     void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeMillis);
 
     @Nullable
-    static OutlierEjectionAlgorithm forConfig(OutlierDetectionLoadBalancerConfig config) {
+    static List<OutlierEjectionAlgorithm> forConfig(OutlierDetectionLoadBalancerConfig config) {
+      ImmutableList.Builder<OutlierEjectionAlgorithm> algoListBuilder = ImmutableList.builder();
       if (config.successRateEjection != null) {
-        return new SuccessRateOutlierEjectionAlgorithm(config);
-      } else if (config.failurePercentageEjection != null) {
-        return new FailurePercentageOutlierEjectionAlgorithm(config);
-      } else {
-        return null;
+        algoListBuilder.add(new SuccessRateOutlierEjectionAlgorithm(config));
       }
+      if (config.failurePercentageEjection != null) {
+        algoListBuilder.add(new FailurePercentageOutlierEjectionAlgorithm(config));
+      }
+      return algoListBuilder.build();
     }
   }
 
@@ -752,7 +755,7 @@ public void ejectOutliers(AddressTrackerMap trackerMap, long ejectionTimeMillis)
 
       // If this address does not have enough volume to be considered, skip to the next one.
       for (AddressTracker tracker : trackerMap.values()) {
-        // 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;
         }

diff --git a/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java b/core/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java
@@ -810,6 +810,86 @@ public void subchannelUpdateAddress_multipleReplacedWithSingle() {
     assertThat(subchannel.isEjected()).isFalse();
   }
 
+  /** Both algorithms configured, but no outliers. */
+  @Test
+  public void successRateAndFailurePercentage_noOutliers() {
+    OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder()
+        .setMaxEjectionPercent(50)
+        .setSuccessRateEjection(
+            new SuccessRateEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10).build())
+        .setFailurePercentageEjection(
+            new FailurePercentageEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10).build())
+        .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build();
+
+    loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers));
+
+    generateLoad(ImmutableMap.of());
+
+    // Move forward in time to a point where the detection timer has fired.
+    fakeClock.forwardTime(config.intervalSecs + 1, TimeUnit.SECONDS);
+
+    // No outliers, no ejections.
+    assertEjectedSubchannels(ImmutableSet.of());
+  }
+
+  /** Both algorithms configured, success rate detects an outlier. */
+  @Test
+  public void successRateAndFailurePercentage_successRateOutlier() {
+    OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder()
+        .setMaxEjectionPercent(50)
+        .setSuccessRateEjection(
+            new SuccessRateEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10).build())
+        .setFailurePercentageEjection(
+            new FailurePercentageEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10)
+                .setEnforcementPercentage(0).build()) // Configured, but not enforcing.
+        .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build();
+
+    loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers));
+
+    generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED));
+
+    // Move forward in time to a point where the detection timer has fired.
+    fakeClock.forwardTime(config.intervalSecs + 1, TimeUnit.SECONDS);
+
+    // The one subchannel that was returning errors should be ejected.
+    assertEjectedSubchannels(ImmutableSet.of(servers.get(0)));
+  }
+
+  /** Both algorithms configured, error percentage detects an outlier. */
+  @Test
+  public void successRateAndFailurePercentage_errorPercentageOutlier() {
+    OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder()
+        .setMaxEjectionPercent(50)
+        .setSuccessRateEjection(
+            new SuccessRateEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10)
+                .setEnforcementPercentage(0).build())
+        .setFailurePercentageEjection(
+            new FailurePercentageEjection.Builder()
+                .setMinimumHosts(3)
+                .setRequestVolume(10).build()) // Configured, but not enforcing.
+        .setChildPolicy(new PolicySelection(roundRobinLbProvider, null)).build();
+
+    loadBalancer.handleResolvedAddresses(buildResolvedAddress(config, servers));
+
+    generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED));
+
+    // Move forward in time to a point where the detection timer has fired.
+    fakeClock.forwardTime(config.intervalSecs + 1, TimeUnit.SECONDS);
+
+    // The one subchannel that was returning errors should be ejected.
+    assertEjectedSubchannels(ImmutableSet.of(servers.get(0)));
+  }
+
   @Test
   public void mathChecksOut() {
     ImmutableList<Double> values = ImmutableList.of(600d, 470d, 170d, 430d, 300d);