Make outliers fail gradually and restore test coverage

Author: incubos

util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java

@@ -834,8 +834,8 @@ public void ejectOutliers(EndpointTrackerMap trackerMap, long ejectionTimeNanos)
           return;
         }
 
-        // If success rate is zero or below the threshold, eject the address.
-        if (tracker.inactiveSuccessCount() == 0L || tracker.successRate() < requiredSuccessRate) {
+        // If success rate is below the threshold or is zero, eject the address.
+        if (tracker.successRate() < requiredSuccessRate || tracker.inactiveSuccessCount() == 0L) {
           logger.log(ChannelLogLevel.DEBUG,
                   "SuccessRate algorithm detected outlier: {0}. "
                           + "Parameters: successRate={1}, mean={2}, stdev={3}, "

util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java

@@ -725,10 +725,10 @@ public void successRateTwoOutliers() {
   }
 
   /**
-   * Two outliers get ejected with default stdevFactor.
+   * One then two outliers get ejected with default stdevFactor.
    */
   @Test
-  public void successRateTwoOutliersWithDefaultStdevFactor() {
+  public void successRateOneThenTwoOutliersWithDefaultStdevFactor() {
     OutlierDetectionLoadBalancerConfig config = new OutlierDetectionLoadBalancerConfig.Builder()
             .setMaxEjectionPercent(50)
             .setSuccessRateEjection(
@@ -741,14 +741,24 @@ public void successRateTwoOutliersWithDefaultStdevFactor() {
 
     loadBalancer.acceptResolvedAddresses(buildResolvedAddress(config, servers));
 
+    // One endpoint is failing
+    generateLoad(ImmutableMap.of(subchannel1, Status.DEADLINE_EXCEEDED), 7);
+
+    // Move forward in time to a point where the detection timer has fired.
+    forwardTime(config);
+
+    // The one subchannel that was returning errors should be ejected.
+    assertEjectedSubchannels(ImmutableSet.of(ImmutableSet.of(servers.get(0).getAddresses().get(0))));
+
+    // Two endpoints are failing
     generateLoad(ImmutableMap.of(
             subchannel1, Status.DEADLINE_EXCEEDED,
-            subchannel2, Status.DEADLINE_EXCEEDED), 7);
+            subchannel2, Status.DEADLINE_EXCEEDED), 12);
 
     // Move forward in time to a point where the detection timer has fired.
     forwardTime(config);
 
-    // The one subchannel that was returning errors should be ejected.
+    // Both subchannels that were returning errors should be ejected.
     assertEjectedSubchannels(ImmutableSet.of(ImmutableSet.of(servers.get(0).getAddresses().get(0)),
             ImmutableSet.of(servers.get(1).getAddresses().get(0))));
   }