loadbalancer: remove RoundRobinLoadBalancerBuilderProvider and inline its behavior

servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactory.java

@@ -55,6 +55,9 @@
 public final class RoundRobinLoadBalancerFactory<ResolvedAddress, C extends LoadBalancedConnection>
         implements LoadBalancerFactory<ResolvedAddress, C> {
 
+    static final String ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER =
+            "io.servicetalk.loadbalancer.roundRobinUsesDefaultLoadBalancer";
+
     private final String id;
     private final int linearSearchSpace;
     @Nullable
@@ -74,17 +77,21 @@ public <T extends C> LoadBalancer<T> newLoadBalancer(
             final String targetResource,
             final Publisher<? extends Collection<? extends ServiceDiscovererEvent<ResolvedAddress>>> eventPublisher,
             final ConnectionFactory<ResolvedAddress, T> connectionFactory) {
-        return new RoundRobinLoadBalancer<>(id, targetResource, eventPublisher, connectionFactory,
-                linearSearchSpace, healthCheckConfig);
+        // We have to indirect here instead of at the `Builder.build()` call because as it turns out
+        // `Builder.build()` has a return type of RoundRobinLoadBalancerFactory and is public API.
+        return useDefaultLoadBalancer() ?
+                buildDefaultLoadBalancerFactory(targetResource, eventPublisher, connectionFactory) :
+                new RoundRobinLoadBalancer<>(
+                        id, targetResource, eventPublisher, connectionFactory, linearSearchSpace, healthCheckConfig);
     }
 
     @Override
     public LoadBalancer<C> newLoadBalancer(
             final Publisher<? extends Collection<? extends ServiceDiscovererEvent<ResolvedAddress>>> eventPublisher,
             final ConnectionFactory<ResolvedAddress, C> connectionFactory,
             final String targetResource) {
-        return new RoundRobinLoadBalancer<>(id, targetResource, eventPublisher, connectionFactory,
-                linearSearchSpace, healthCheckConfig);
+        // For now, we forward to the deprecated method since it is more generic.
+        return newLoadBalancer(targetResource, eventPublisher, connectionFactory);
     }
 
     @Override
@@ -102,6 +109,59 @@ public String toString() {
                 '}';
     }
 
+    private <T extends C> LoadBalancer<T> buildDefaultLoadBalancerFactory(
+            final String targetResource,
+            final Publisher<? extends Collection<? extends ServiceDiscovererEvent<ResolvedAddress>>> eventPublisher,
+            final ConnectionFactory<ResolvedAddress, T> connectionFactory) {
+        final int healthCheckFailedConnectionsThreshold;
+        final Duration healthCheckInterval;
+        final Duration healthCheckJitter;
+        final Duration healthCheckResubscribeInterval;
+        final Duration healthCheckResubscribeJitter;
+        final Executor backgroundExecutor;
+        if (healthCheckConfig == null) {
+            healthCheckFailedConnectionsThreshold = -1; // disabled, the rest are fillers.
+            healthCheckInterval = DEFAULT_HEALTH_CHECK_INTERVAL;
+            healthCheckJitter = DEFAULT_HEALTH_CHECK_JITTER;
+            healthCheckResubscribeInterval = DEFAULT_HEALTH_CHECK_RESUBSCRIBE_INTERVAL;
+            healthCheckResubscribeJitter = DEFAULT_HEALTH_CHECK_JITTER;
+            backgroundExecutor = null;
+        } else {
+            healthCheckFailedConnectionsThreshold = healthCheckConfig.failedThreshold;
+            healthCheckInterval = healthCheckConfig.healthCheckInterval;
+            healthCheckJitter = healthCheckConfig.jitter;
+            healthCheckResubscribeInterval = healthCheckConfig.resubscribeInterval;
+            healthCheckResubscribeJitter = healthCheckConfig.healthCheckResubscribeJitter;
+            backgroundExecutor = healthCheckConfig.executor;
+        }
+
+        OutlierDetectorConfig outlierDetectorConfig = new OutlierDetectorConfig.Builder()
+                .ewmaHalfLife(Duration.ZERO)
+                // disable the xDS outlier detectors
+                .enforcingFailurePercentage(0)
+                .enforcingSuccessRate(0)
+                .enforcingConsecutive5xx(0)
+                // set the ServiceTalk L4 connection outlier detector settings
+                .failedConnectionsThreshold(healthCheckFailedConnectionsThreshold)
+                .failureDetectorInterval(healthCheckInterval, healthCheckJitter)
+                .serviceDiscoveryResubscribeInterval(healthCheckResubscribeInterval, healthCheckResubscribeJitter)
+                .build();
+        LoadBalancingPolicy<ResolvedAddress, T> loadBalancingPolicy =
+                LoadBalancingPolicies.roundRobin()
+                        .failOpen(false)
+                        .ignoreWeights(true)
+                        .build();
+        LoadBalancerBuilder<ResolvedAddress, T> builder = LoadBalancers.builder(id);
+        if (backgroundExecutor != null) {
+            builder = builder.backgroundExecutor(backgroundExecutor);
+        }
+        return builder.outlierDetectorConfig(outlierDetectorConfig)
+                .loadBalancingPolicy(loadBalancingPolicy)
+                .connectionSelectorPolicy(ConnectionSelectorPolicies.linearSearch(linearSearchSpace))
+                .build()
+                .newLoadBalancer(eventPublisher, connectionFactory, targetResource);
+    }
+
     /**
      * Builder for {@link RoundRobinLoadBalancerFactory}.
      *
@@ -227,4 +287,10 @@ static Executor getInstance() {
             return INSTANCE;
         }
     }
+
+    private static boolean useDefaultLoadBalancer() {
+        // Enabled by default.
+        String propValue = System.getProperty(ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER);
+        return propValue == null || Boolean.parseBoolean(propValue);
+    }
 }

servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerBuilderProviderTest.java

@@ -26,7 +26,6 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
-import static io.servicetalk.loadbalancer.RoundRobinToDefaultLBMigrationProvider.PROPERTY_NAME;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
@@ -35,13 +34,11 @@ class RoundRobinLoadBalancerBuilderProviderTest {
     @BeforeEach
     void reset() {
         TestRoundRobinLoadBalancerBuilderProvider.reset();
-        System.setProperty(PROPERTY_NAME, Boolean.FALSE.toString());
     }
 
     @AfterEach
     void deactivate() {
         TestRoundRobinLoadBalancerBuilderProvider.activated.set(false);
-        System.setProperty(PROPERTY_NAME, Boolean.TRUE.toString());
     }
 
     @Test

servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactoryTest.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright © 2024 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.servicetalk.loadbalancer;
+
+import io.servicetalk.client.api.ConnectionFactory;
+import io.servicetalk.client.api.LoadBalancer;
+import io.servicetalk.concurrent.api.Publisher;
+import io.servicetalk.concurrent.api.Single;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+
+import static io.servicetalk.loadbalancer.RoundRobinLoadBalancerFactory.ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+
+@Execution(ExecutionMode.SAME_THREAD)
+final class RoundRobinLoadBalancerFactoryTest {
+
+    @AfterEach
+    void cleanup() {
+        System.clearProperty(ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER);
+    }
+
+    @Test
+    void generateDefaultLoadBalancerIfEnabled() {
+        System.setProperty(ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER, "true");
+        assertThat(getLoadBalancer(), instanceOf(DefaultLoadBalancer.class));
+    }
+
+    @Test
+    void doesNotGenerateDefaultLoadBalancerIfDisabled() {
+        System.setProperty(ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER, "false");
+        assertThat(getLoadBalancer(), instanceOf(RoundRobinLoadBalancer.class));
+    }
+
+    @Test
+    void defaultResultIsDefaultLoadBalancer() {
+        assertThat(getLoadBalancer(), instanceOf(DefaultLoadBalancer.class));
+    }
+
+    static LoadBalancer<TestLoadBalancedConnection> getLoadBalancer() {
+        ConnectionFactory<String, TestLoadBalancedConnection> connectionFactory =
+                new TestConnectionFactory(ignored -> Single.never());
+        return RoundRobinLoadBalancers.<String, TestLoadBalancedConnection>builder("balancer").build()
+                .newLoadBalancer(Publisher.never(), connectionFactory, "targetResource");
+    }
+}