loadbalancer-experimental: don't always pay for EWMA with noop detect…

…or (#3057)

Motivation:

There is a perf regression in the DefaultLB in compatibility mode compared to
the round-robin LB when under very high request rate and that is at least in part
attributable to lock contention for the health indicator EWMA.

Modifications:

Don't create a health indicator if the EWMA half life is 0.

servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java

@@ -423,7 +423,9 @@ private PrioritizedHostImpl<ResolvedAddress, C> createHost(ServiceDiscovererEven
                     new DefaultHost<>(lbDescription, addr, connectionPoolStrategy,
                     connectionFactory, hostObserver, hostHealthCheckConfig, indicator),
                     eventWeight(event), eventPriority(event));
-            indicator.setHost(host);
+            if (indicator != null) {
+                indicator.setHost(host);
+            }
             host.onClose().afterFinally(() ->
                     sequentialExecutor.execute(() -> {
                         final List<PrioritizedHostImpl<ResolvedAddress, C>> currentHosts = usedHosts;

servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java

@@ -19,7 +19,7 @@
 
 import java.util.concurrent.locks.StampedLock;
 
-import static io.servicetalk.utils.internal.NumberUtils.ensurePositive;
+import static io.servicetalk.utils.internal.NumberUtils.ensureNonNegative;
 import static java.lang.Math.ceil;
 import static java.lang.Math.exp;
 import static java.lang.Math.log;
@@ -58,8 +58,8 @@ abstract class DefaultRequestTracker implements RequestTracker, ScoreSupplier {
 
     DefaultRequestTracker(final long halfLifeNanos, final int cancelPenalty, final int errorPenalty,
                           final int concurrentRequestPenalty) {
-        ensurePositive(halfLifeNanos, "halfLifeNanos");
-        this.invTau = Math.pow((halfLifeNanos / log(2)), -1);
+        ensureNonNegative(halfLifeNanos, "halfLifeNanos");
+        this.invTau = halfLifeNanos == 0 ? Double.MAX_VALUE : Math.pow((halfLifeNanos / log(2)), -1);
         this.cancelPenalty = cancelPenalty;
         this.errorPenalty = errorPenalty;
         this.concurrentRequestPenalty = concurrentRequestPenalty;

servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/NoopOutlierDetector.java

@@ -20,6 +20,7 @@
 import io.servicetalk.concurrent.api.Publisher;
 
 import java.util.concurrent.TimeUnit;
+import javax.annotation.Nullable;
 
 import static java.util.Objects.requireNonNull;
 
@@ -41,21 +42,22 @@ public void cancel() {
         // noop.
     }
 
+    @Nullable
     @Override
     public HealthIndicator<ResolvedAddress, C> newHealthIndicator(
             ResolvedAddress resolvedAddress, LoadBalancerObserver.HostObserver hostObserver) {
-        return new BasicHealthIndicator();
+        return outlierDetectorConfig.ewmaHalfLife().isZero() ? null : new EwmaHealthIndicator();
     }
 
     @Override
     public Publisher<Void> healthStatusChanged() {
         return Publisher.never();
     }
 
-    private final class BasicHealthIndicator extends DefaultRequestTracker
+    private final class EwmaHealthIndicator extends DefaultRequestTracker
             implements HealthIndicator<ResolvedAddress, C> {
 
-        BasicHealthIndicator() {
+        EwmaHealthIndicator() {
             super(outlierDetectorConfig.ewmaHalfLife().toNanos(), outlierDetectorConfig.ewmaCancellationPenalty(),
                     outlierDetectorConfig.ewmaErrorPenalty(), outlierDetectorConfig.concurrentRequestPenalty());
         }

servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetector.java

@@ -20,6 +20,8 @@
 import io.servicetalk.concurrent.api.Publisher;
 import io.servicetalk.loadbalancer.LoadBalancerObserver.HostObserver;
 
+import javax.annotation.Nullable;
+
 /**
  * The representation of a health checking system for use with load balancing.
  */
@@ -33,6 +35,7 @@ interface OutlierDetector<ResolvedAddress, C extends LoadBalancedConnection> ext
      * @param address the resolved address of the destination.
      * @return new {@link HealthIndicator}.
      */
+    @Nullable
     HealthIndicator<ResolvedAddress, C> newHealthIndicator(ResolvedAddress address, HostObserver hostObserver);
 
     /**

servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProvider.java

@@ -115,6 +115,7 @@ public RoundRobinLoadBalancerBuilder<ResolvedAddress, C> healthCheckFailedConnec
         @Override
         public LoadBalancerFactory<ResolvedAddress, C> build() {
             OutlierDetectorConfig outlierDetectorConfig = new OutlierDetectorConfig.Builder()
+                    .ewmaHalfLife(Duration.ZERO)
                     .enforcingFailurePercentage(0)
                     .enforcingSuccessRate(0)
                     .enforcingConsecutive5xx(0)

servicetalk-loadbalancer-experimental/src/test/java/io/servicetalk/loadbalancer/DefaultRequestTrackerTest.java

@@ -16,6 +16,8 @@
 package io.servicetalk.loadbalancer;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.time.Duration;
 import java.util.function.LongUnaryOperator;
@@ -89,6 +91,21 @@ void outstandingLatencyIsTracked() {
         assertEquals(-2_000, requestTracker.score());
     }
 
+    @ParameterizedTest
+    @ValueSource(longs = {0L, 1L})
+    void zeroHalfLife(final long timeStep) {
+        final LongUnaryOperator nextValueProvider = mock(LongUnaryOperator.class);
+        when(nextValueProvider.applyAsLong(anyLong())).thenAnswer(__ -> timeStep);
+
+        final DefaultRequestTracker requestTracker = new TestRequestTracker(Duration.ofSeconds(0), nextValueProvider);
+        assertEquals(0, requestTracker.score());
+
+        // upon success score
+        requestTracker.onRequestSuccess(requestTracker.beforeRequestStart());
+        // super quick, so our score is the max it can be which is 0.
+        assertEquals(0, requestTracker.score());
+    }
+
     static final class TestRequestTracker extends DefaultRequestTracker {
         private final LongUnaryOperator nextValueProvider;
         private long lastValue;