diff --git a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactory.java b/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactory.java index 1dd681a440..c43e9a60f8 100644 --- a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactory.java +++ b/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactory.java @@ -55,6 +55,9 @@ public final class RoundRobinLoadBalancerFactory implements LoadBalancerFactory { + static final String ROUND_ROBIN_USER_DEFAULT_LOAD_BALANCER = + "io.servicetalk.loadbalancer.roundRobinUsesDefaultLoadBalancer"; + private final String id; private final int linearSearchSpace; @Nullable @@ -74,8 +77,12 @@ public LoadBalancer newLoadBalancer( final String targetResource, final Publisher>> eventPublisher, final ConnectionFactory 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 @@ -83,8 +90,8 @@ public LoadBalancer newLoadBalancer( final Publisher>> eventPublisher, final ConnectionFactory 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 LoadBalancer buildDefaultLoadBalancerFactory( + final String targetResource, + final Publisher>> eventPublisher, + final ConnectionFactory 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 loadBalancingPolicy = + LoadBalancingPolicies.roundRobin() + .failOpen(false) + .ignoreWeights(true) + .build(); + LoadBalancerBuilder 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); + } } diff --git a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProvider.java b/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProvider.java deleted file mode 100644 index 75d6368a42..0000000000 --- a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProvider.java +++ /dev/null @@ -1,149 +0,0 @@ -/* - * 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.LoadBalancedConnection; -import io.servicetalk.client.api.LoadBalancerFactory; -import io.servicetalk.concurrent.api.Executor; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.time.Duration; -import javax.annotation.Nullable; - -import static io.servicetalk.loadbalancer.HealthCheckConfig.DEFAULT_HEALTH_CHECK_FAILED_CONNECTIONS_THRESHOLD; -import static io.servicetalk.loadbalancer.HealthCheckConfig.DEFAULT_HEALTH_CHECK_INTERVAL; -import static io.servicetalk.loadbalancer.HealthCheckConfig.DEFAULT_HEALTH_CHECK_JITTER; -import static io.servicetalk.loadbalancer.HealthCheckConfig.DEFAULT_HEALTH_CHECK_RESUBSCRIBE_INTERVAL; -import static io.servicetalk.loadbalancer.HealthCheckConfig.validateHealthCheckIntervals; -import static io.servicetalk.utils.internal.NumberUtils.ensureNonNegative; - -/** - * A {@link RoundRobinLoadBalancerBuilderProvider} to automatically migrate from {@link RoundRobinLoadBalancerBuilder} - * to {@link LoadBalancerBuilder} implementation with compatible runtime behavior. - */ -public final class RoundRobinToDefaultLBMigrationProvider implements RoundRobinLoadBalancerBuilderProvider { - - static final String PROPERTY_NAME = "io.servicetalk.loadbalancer.roundRobinUsesDefaultLoadBalancer"; - - private static final Logger LOGGER = LoggerFactory.getLogger(RoundRobinToDefaultLBMigrationProvider.class); - - @Override - public - RoundRobinLoadBalancerBuilder newBuilder( - String id, RoundRobinLoadBalancerBuilder builder) { - if (isEnabled()) { - LOGGER.info("Enabling DefaultLoadBalancer in place of RoundRobinLoadBalancer for load balancer id {}", id); - return new DefaultLoadBalancerRoundRobinBuilder<>(id); - } else { - LOGGER.debug( - "Not enabling DefaultLoadBalancer in place of RoundRobinLoadBalancer for load balancer id {}", id); - return builder; - } - } - - private static boolean isEnabled() { - // Enabled by default. - String propValue = System.getProperty(PROPERTY_NAME); - return propValue == null || Boolean.parseBoolean(propValue); - } - - private static final class DefaultLoadBalancerRoundRobinBuilder - implements RoundRobinLoadBalancerBuilder { - - private final String id; - private int linearSearchSpace = 16; - @Nullable - private Executor backgroundExecutor; - private Duration healthCheckInterval = DEFAULT_HEALTH_CHECK_INTERVAL; - private Duration healthCheckJitter = DEFAULT_HEALTH_CHECK_JITTER; - private int healthCheckFailedConnectionsThreshold = DEFAULT_HEALTH_CHECK_FAILED_CONNECTIONS_THRESHOLD; - private Duration healthCheckResubscribeInterval = DEFAULT_HEALTH_CHECK_RESUBSCRIBE_INTERVAL; - private Duration healthCheckResubscribeJitter = DEFAULT_HEALTH_CHECK_JITTER; - - DefaultLoadBalancerRoundRobinBuilder(final String id) { - this.id = id; - } - - @Override - public RoundRobinLoadBalancerBuilder linearSearchSpace(int linearSearchSpace) { - this.linearSearchSpace = ensureNonNegative(linearSearchSpace, "linearSearchSpace"); - return this; - } - - @Override - public RoundRobinLoadBalancerBuilder backgroundExecutor(Executor backgroundExecutor) { - this.backgroundExecutor = backgroundExecutor; - return this; - } - - @Override - public RoundRobinLoadBalancerBuilder healthCheckInterval( - Duration interval, Duration jitter) { - validateHealthCheckIntervals(interval, jitter); - this.healthCheckInterval = interval; - this.healthCheckJitter = jitter; - return this; - } - - @Override - public RoundRobinLoadBalancerBuilder healthCheckResubscribeInterval( - Duration interval, Duration jitter) { - validateHealthCheckIntervals(interval, jitter); - this.healthCheckResubscribeInterval = interval; - this.healthCheckResubscribeJitter = jitter; - return this; - } - - @Override - public RoundRobinLoadBalancerBuilder healthCheckFailedConnectionsThreshold(int threshold) { - if (threshold == 0) { - throw new IllegalArgumentException("Health check failed connections threshold should not be 0"); - } - this.healthCheckFailedConnectionsThreshold = threshold; - return this; - } - - @Override - public LoadBalancerFactory build() { - OutlierDetectorConfig outlierDetectorConfig = new OutlierDetectorConfig.Builder() - .ewmaHalfLife(Duration.ZERO) - .enforcingFailurePercentage(0) - .enforcingSuccessRate(0) - .enforcingConsecutive5xx(0) - .failedConnectionsThreshold(healthCheckFailedConnectionsThreshold) - .failureDetectorInterval(healthCheckInterval, healthCheckJitter) - .serviceDiscoveryResubscribeInterval(healthCheckResubscribeInterval, healthCheckResubscribeJitter) - .build(); - - LoadBalancingPolicy loadBalancingPolicy = - LoadBalancingPolicies.roundRobin() - .failOpen(false) - .ignoreWeights(true) - .build(); - - LoadBalancerBuilder builder = LoadBalancers.builder(id); - if (backgroundExecutor != null) { - builder = builder.backgroundExecutor(backgroundExecutor); - } - return builder.outlierDetectorConfig(outlierDetectorConfig) - .loadBalancingPolicy(loadBalancingPolicy) - .connectionSelectorPolicy(ConnectionSelectorPolicies.linearSearch(linearSearchSpace)) - .build(); - } - } -} diff --git a/servicetalk-loadbalancer/src/main/resources/META-INF/services/io.servicetalk.loadbalancer.RoundRobinLoadBalancerBuilderProvider b/servicetalk-loadbalancer/src/main/resources/META-INF/services/io.servicetalk.loadbalancer.RoundRobinLoadBalancerBuilderProvider deleted file mode 100644 index 5be5d82773..0000000000 --- a/servicetalk-loadbalancer/src/main/resources/META-INF/services/io.servicetalk.loadbalancer.RoundRobinLoadBalancerBuilderProvider +++ /dev/null @@ -1 +0,0 @@ -io.servicetalk.loadbalancer.RoundRobinToDefaultLBMigrationProvider diff --git a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerBuilderProviderTest.java b/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerBuilderProviderTest.java index 494e33e7dd..03c95fd85d 100644 --- a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerBuilderProviderTest.java +++ b/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 diff --git a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactoryTest.java b/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinLoadBalancerFactoryTest.java new file mode 100644 index 0000000000..2352180bc7 --- /dev/null +++ b/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 getLoadBalancer() { + ConnectionFactory connectionFactory = + new TestConnectionFactory(ignored -> Single.never()); + return RoundRobinLoadBalancers.builder("balancer").build() + .newLoadBalancer(Publisher.never(), connectionFactory, "targetResource"); + } +} diff --git a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProviderTest.java b/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProviderTest.java deleted file mode 100644 index 8e59c9b5a2..0000000000 --- a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/RoundRobinToDefaultLBMigrationProviderTest.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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 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.RoundRobinToDefaultLBMigrationProvider.PROPERTY_NAME; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.sameInstance; - -@Execution(ExecutionMode.SAME_THREAD) -final class RoundRobinToDefaultLBMigrationProviderTest { - - private final RoundRobinToDefaultLBMigrationProvider provider = new RoundRobinToDefaultLBMigrationProvider(); - - @AfterEach - void cleanup() { - System.clearProperty(PROPERTY_NAME); - } - - @Test - void enabled() { - System.setProperty(PROPERTY_NAME, "true"); - RoundRobinLoadBalancerBuilder builder = - new RoundRobinLoadBalancerFactory.Builder<>(); - RoundRobinLoadBalancerBuilder result = provider.newBuilder( - "builder", builder); - assertThat(result.build(), instanceOf(DefaultLoadBalancerBuilder.DefaultLoadBalancerFactory.class)); - } - - @Test - void disabled() { - System.setProperty(PROPERTY_NAME, "false"); - RoundRobinLoadBalancerBuilder builder = - new RoundRobinLoadBalancerFactory.Builder<>(); - RoundRobinLoadBalancerBuilder result = provider.newBuilder( - "builder", builder); - assertThat(result, sameInstance(builder)); - } - - @Test - void defaultValue() { - RoundRobinLoadBalancerBuilder builder = - new RoundRobinLoadBalancerFactory.Builder<>(); - RoundRobinLoadBalancerBuilder result = provider.newBuilder( - "builder", builder); - assertThat(result.build(), instanceOf(DefaultLoadBalancerBuilder.DefaultLoadBalancerFactory.class)); - } - - @Test - void isCorrectlyServiceLoaded() { - System.setProperty(PROPERTY_NAME, "true"); - assertThat(RoundRobinLoadBalancers.builder("builder").build(), - instanceOf(DefaultLoadBalancerBuilder.DefaultLoadBalancerFactory.class)); - } -}