From c3930428d88f221d60ca16063c04501042063fef Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Thu, 7 Mar 2024 10:01:19 -0700 Subject: [PATCH] Cleanup the builder interface --- .../DefaultLoadBalancerBuilder.java | 51 ++----- .../loadbalancer/DefaultRequestTracker.java | 3 + .../loadbalancer/LoadBalancerBuilder.java | 61 ++------- .../loadbalancer/OutlierDetectorConfig.java | 125 +++++++++++++++++- 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancerBuilder.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancerBuilder.java index 6f99ab5312..d65580d6fd 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancerBuilder.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancerBuilder.java @@ -51,11 +51,7 @@ final class DefaultLoadBalancerBuilder loadBalancerObserver; - @Nullable - private OutlierDetectorFactory outlierDetectorFactory; - private Duration healthCheckInterval = DEFAULT_HEALTH_CHECK_INTERVAL; - private Duration healthCheckJitter = DEFAULT_HEALTH_CHECK_JITTER; - private int healthCheckFailedConnectionsThreshold = DEFAULT_HEALTH_CHECK_FAILED_CONNECTIONS_THRESHOLD; + private OutlierDetectorConfig outlierDetectorConfig = OutlierDetectorConfig.DEFAULT_CONFIG; private Duration healthCheckResubscribeInterval = DEFAULT_HEALTH_CHECK_RESUBSCRIBE_INTERVAL; private Duration healthCheckResubscribeJitter = DEFAULT_HEALTH_CHECK_JITTER; @@ -85,9 +81,10 @@ public LoadBalancerBuilder loadBalancerObserver( } @Override - public LoadBalancerBuilder outlierDetectorFactory( - OutlierDetectorFactory outlierDetectorFactory) { - this.outlierDetectorFactory = outlierDetectorFactory; + public LoadBalancerBuilder outlierDetectorConfig( + @Nullable OutlierDetectorConfig outlierDetectorConfig) { + this.outlierDetectorConfig = outlierDetectorConfig == null ? + OutlierDetectorConfig.DEFAULT_CONFIG : outlierDetectorConfig; return this; } @@ -97,14 +94,6 @@ public LoadBalancerBuilder backgroundExecutor(Executor backg return this; } - @Override - public LoadBalancerBuilder healthCheckInterval(Duration interval, Duration jitter) { - validateHealthCheckIntervals(interval, jitter); - this.healthCheckInterval = interval; - this.healthCheckJitter = jitter; - return this; - } - @Override public LoadBalancerBuilder healthCheckResubscribeInterval( Duration interval, Duration jitter) { @@ -114,37 +103,23 @@ public LoadBalancerBuilder healthCheckResubscribeInterval( return this; } - @Override - public LoadBalancerBuilder healthCheckFailedConnectionsThreshold( - int threshold) { - if (threshold == 0) { - throw new IllegalArgumentException("Invalid health-check failed connections (expected != 0)"); - } - this.healthCheckFailedConnectionsThreshold = threshold; - return this; - } - @Override public LoadBalancerFactory build() { final HealthCheckConfig healthCheckConfig; - if (this.healthCheckFailedConnectionsThreshold < 0) { + final Executor executor = getExecutor(); + if (outlierDetectorConfig.failedConnectionsThreshold() < 0) { healthCheckConfig = null; } else { - healthCheckConfig = new HealthCheckConfig(getExecutor(), - healthCheckInterval, healthCheckJitter, healthCheckFailedConnectionsThreshold, + healthCheckConfig = new HealthCheckConfig(executor, + outlierDetectorConfig.failedConnectionsProbeInterval(), + outlierDetectorConfig.failedConnectionsProbeJitter(), + outlierDetectorConfig.failedConnectionsThreshold(), healthCheckResubscribeInterval, healthCheckResubscribeJitter); } final LoadBalancerObserver loadBalancerObserver = this.loadBalancerObserver != null ? this.loadBalancerObserver : NoopLoadBalancerObserver.instance(); - Function> healthCheckerSupplier; - if (outlierDetectorFactory == null) { - healthCheckerSupplier = null; - } else { - final Executor executor = getExecutor(); - healthCheckerSupplier = (String lbDescrption) -> - outlierDetectorFactory.newHealthChecker(executor, lbDescrption); - } - + Function> healthCheckerSupplier = + (String lbDescription) -> new XdsOutlierDetector<>(executor, outlierDetectorConfig, lbDescription); return new DefaultLoadBalancerFactory<>(id, loadBalancingPolicy, linearSearchSpace, healthCheckConfig, loadBalancerObserver, healthCheckerSupplier); } diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java index 9929bdbd7c..620297fe03 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java @@ -17,6 +17,7 @@ import io.servicetalk.client.api.ScoreSupplier; +import java.time.Duration; import java.util.concurrent.locks.StampedLock; import static io.servicetalk.utils.internal.NumberUtils.ensurePositive; @@ -39,6 +40,8 @@ */ abstract class DefaultRequestTracker implements RequestTracker, ScoreSupplier { private static final long MAX_MS_TO_NS = NANOSECONDS.convert(MAX_VALUE, MILLISECONDS); + + static final Duration DEFAULT_EWMA_HALF_LIFE = Duration.ofSeconds(10); static final long DEFAULT_CANCEL_PENALTY = 5L; static final long DEFAULT_ERROR_PENALTY = 10L; diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerBuilder.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerBuilder.java index 1c461cf242..5adff2de40 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerBuilder.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerBuilder.java @@ -84,28 +84,20 @@ LoadBalancerBuilder loadBalancerObserver( @Nullable LoadBalancerObserver loadBalancerObserver); /** - * Set the {@link OutlierDetectorFactory} to use with this load balancer. - * @param outlierDetectorFactory the {@link OutlierDetectorFactory} to use, or {@code null} to not use a - * {@link OutlierDetector}. + * Set the {@link OutlierDetectorConfig} to use with this load balancer. + * @param outlierDetectorConfig the {@link OutlierDetectorConfig} to use, or {@code null} to use the default + * outlier detection. * @return {code this} */ - LoadBalancerBuilder outlierDetectorFactory( - OutlierDetectorFactory outlierDetectorFactory); + LoadBalancerBuilder outlierDetectorConfig( + @Nullable OutlierDetectorConfig outlierDetectorConfig); /** - * This {@link LoadBalancer} may monitor hosts to which connection establishment has failed - * using health checks that run in the background. The health check tries to establish a new connection - * and if it succeeds, the host is returned to the load balancing pool. As long as the connection - * establishment fails, the host is not considered for opening new connections for processed requests. - * If an {@link Executor} is not provided using this method, a default shared instance is used - * for all {@link LoadBalancer LoadBalancers} created by this factory. - *

- * {@link #healthCheckFailedConnectionsThreshold(int)} can be used to disable this mechanism and always - * consider all hosts for establishing new connections. + * Set the background {@link Executor} to use for determining time and scheduling background tasks such + * as those associated with outlier detection. * - * @param backgroundExecutor {@link Executor} on which to schedule health checking. + * @param backgroundExecutor {@link Executor} to use as a time source and for scheduling background tasks. * @return {@code this}. - * @see #healthCheckFailedConnectionsThreshold(int) */ LoadBalancerBuilder backgroundExecutor(Executor backgroundExecutor); @@ -125,22 +117,6 @@ LoadBalancerBuilder outlierDetectorFactory( */ LoadBalancerBuilder linearSearchSpace(int linearSearchSpace); - // TODO: these healthCheck* methods should be moved into their own OutlierDetection configuration instance - // and much like the LoadBalancingPolicy, we should be able to add `OutlierDetectionPolicy`s - /** - * Configure an interval for health checking a host that failed to open connections. If no interval is provided - * using this method, a default value will be used. - *

- * {@link #healthCheckFailedConnectionsThreshold(int)} can be used to disable the health checking mechanism - * and always consider all hosts for establishing new connections. - * - * @param interval interval at which a background health check will be scheduled. - * @param jitter the amount of jitter to apply to each retry {@code interval}. - * @return {@code this}. - * @see #healthCheckFailedConnectionsThreshold(int) - */ - LoadBalancerBuilder healthCheckInterval(Duration interval, Duration jitter); - /** * Configure an interval for re-subscribing to the original events stream in case all existing hosts become * unhealthy. @@ -149,32 +125,15 @@ LoadBalancerBuilder outlierDetectorFactory( * known hosts become unhealthy, which could happen due to intermediate caching layers, re-subscribe to the * events stream can help to exit from a dead state. *

- * {@link #healthCheckFailedConnectionsThreshold(int)} can be used to disable the health checking mechanism - * and always consider all hosts for establishing new connections. + * Note: setting the interval to {@code Duration.ofNanos(Long.MAX_VALUE)} will effectively disable health check + * resubscribes. * * @param interval interval at which re-subscribes will be scheduled. * @param jitter the amount of jitter to apply to each re-subscribe {@code interval}. * @return {@code this}. - * @see #healthCheckFailedConnectionsThreshold(int) */ LoadBalancerBuilder healthCheckResubscribeInterval(Duration interval, Duration jitter); - /** - * Configure a threshold for consecutive connection failures to a host. When the {@link LoadBalancer} - * consecutively fails to open connections in the amount greater or equal to the specified value, - * the host will be marked as unhealthy and connection establishment will take place in the background - * repeatedly until a connection is established. During that time, the host will not take part in - * load balancing selection. - *

- * Use a negative value of the argument to disable health checking. - * - * @param threshold number of consecutive connection failures to consider a host unhealthy and eligible for - * background health checking. Use negative value to disable the health checking mechanism. - * @return {@code this}. - * @see #backgroundExecutor(Executor) - */ - LoadBalancerBuilder healthCheckFailedConnectionsThreshold(int threshold); - /** * Builds the {@link LoadBalancerFactory} configured by this builder. * diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetectorConfig.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetectorConfig.java index f314469ed0..86515e4430 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetectorConfig.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetectorConfig.java @@ -15,9 +15,17 @@ */ package io.servicetalk.loadbalancer; +import io.servicetalk.client.api.LoadBalancer; + import java.time.Duration; +import java.util.Objects; import java.util.concurrent.ThreadLocalRandom; +import static io.servicetalk.loadbalancer.DefaultRequestTracker.DEFAULT_EWMA_HALF_LIFE; +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.validateHealthCheckIntervals; import static io.servicetalk.utils.internal.NumberUtils.ensureNonNegative; import static io.servicetalk.utils.internal.NumberUtils.ensurePositive; import static java.util.Objects.requireNonNull; @@ -30,7 +38,14 @@ */ public final class OutlierDetectorConfig { + static final OutlierDetectorConfig DEFAULT_CONFIG = new Builder().build(); + private final Duration ewmaHalfLife; + private final int failedConnectionsThreshold; + + private final Duration failedConnectionsProbeInterval; + private final Duration failedConnectionsProbeJitter; + private final int consecutive5xx; private final Duration interval; private final Duration baseEjectionTime; @@ -47,7 +62,9 @@ public final class OutlierDetectorConfig { private final Duration maxEjectionTime; private final Duration maxEjectionTimeJitter; - OutlierDetectorConfig(final Duration ewmaHalfLife, + OutlierDetectorConfig(final Duration ewmaHalfLife, final int failedConnectionsThreshold, + final Duration failedConnectionsProbeInterval, final Duration failedConnectionsProbeJitter, + // true xDS settings final int consecutive5xx, final Duration interval, final Duration baseEjectionTime, final int maxEjectionPercentage, final int enforcingConsecutive5xx, final int enforcingSuccessRate, final int successRateMinimumHosts, @@ -56,6 +73,9 @@ public final class OutlierDetectorConfig { final int failurePercentageMinimumHosts, final int failurePercentageRequestVolume, final Duration maxEjectionTime, final Duration maxEjectionTimeJitter) { this.ewmaHalfLife = requireNonNull(ewmaHalfLife, "ewmaHalfLife"); + this.failedConnectionsThreshold = failedConnectionsThreshold; + this.failedConnectionsProbeInterval = requireNonNull(failedConnectionsProbeInterval, "failedConnectionsProbeInterval"); + this.failedConnectionsProbeJitter = requireNonNull(failedConnectionsProbeJitter, "failedConnectionsProbeJitter"); this.consecutive5xx = consecutive5xx; this.interval = requireNonNull(interval, "interval"); this.baseEjectionTime = requireNonNull(baseEjectionTime, "baseEjectionTime"); @@ -83,6 +103,21 @@ public Duration ewmaHalfLife() { return ewmaHalfLife; } + // TODO: needs javadocs. + public int failedConnectionsThreshold() { + return failedConnectionsThreshold; + } + + // TODO: needs javadocs. + public Duration failedConnectionsProbeInterval() { + return failedConnectionsProbeInterval; + } + + // TODO: needs javadocs. + public Duration failedConnectionsProbeJitter() { + return failedConnectionsProbeJitter; + } + /** * The number of consecutive failures before the attempt to suspect the host. * @return the number of consecutive failures before the attempt to suspect the host. @@ -213,11 +248,52 @@ public Duration maxEjectionTimeJitter() { return maxEjectionTimeJitter; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + OutlierDetectorConfig that = (OutlierDetectorConfig) o; + return failedConnectionsThreshold == that.failedConnectionsThreshold && + consecutive5xx == that.consecutive5xx && maxEjectionPercentage == that.maxEjectionPercentage && + enforcingConsecutive5xx == that.enforcingConsecutive5xx && + enforcingSuccessRate == that.enforcingSuccessRate && + successRateMinimumHosts == that.successRateMinimumHosts && + successRateRequestVolume == that.successRateRequestVolume && + successRateStdevFactor == that.successRateStdevFactor && + failurePercentageThreshold == that.failurePercentageThreshold && + enforcingFailurePercentage == that.enforcingFailurePercentage && + failurePercentageMinimumHosts == that.failurePercentageMinimumHosts && + failurePercentageRequestVolume == that.failurePercentageRequestVolume && + ewmaHalfLife.equals(that.ewmaHalfLife) + && failedConnectionsProbeInterval.equals(that.failedConnectionsProbeInterval) && + failedConnectionsProbeJitter.equals(that.failedConnectionsProbeJitter) && + interval.equals(that.interval) && baseEjectionTime.equals(that.baseEjectionTime) && + maxEjectionTime.equals(that.maxEjectionTime) && + maxEjectionTimeJitter.equals(that.maxEjectionTimeJitter); + } + + @Override + public int hashCode() { + return Objects.hash(ewmaHalfLife, failedConnectionsThreshold, failedConnectionsProbeInterval, + failedConnectionsProbeJitter, consecutive5xx, interval, baseEjectionTime, maxEjectionPercentage, + enforcingConsecutive5xx, enforcingSuccessRate, successRateMinimumHosts, successRateRequestVolume, + successRateStdevFactor, failurePercentageThreshold, enforcingFailurePercentage, + failurePercentageMinimumHosts, failurePercentageRequestVolume, maxEjectionTime, maxEjectionTimeJitter); + } + /** * A builder for {@link OutlierDetectorConfig} instances. */ public static final class Builder { - private Duration ewmaHalfLife = Duration.ofSeconds(10); + + // Non-xDS settings + private Duration ewmaHalfLife = DEFAULT_EWMA_HALF_LIFE; + + private int failedConnectionsThreshold = DEFAULT_HEALTH_CHECK_FAILED_CONNECTIONS_THRESHOLD; + private Duration failedConnectionsProbeInterval = DEFAULT_HEALTH_CHECK_INTERVAL; + private Duration failedConnectionsProbeJitter = DEFAULT_HEALTH_CHECK_JITTER; + + // True xDS settings private int consecutive5xx = 5; private Duration interval = Duration.ofSeconds(10); @@ -254,8 +330,10 @@ public static final class Builder { * @return the OutlierDetectorConfig. */ public OutlierDetectorConfig build() { - return new OutlierDetectorConfig(ewmaHalfLife, consecutive5xx, - interval, baseEjectionTime, + return new OutlierDetectorConfig(ewmaHalfLife, failedConnectionsThreshold, + failedConnectionsProbeInterval, failedConnectionsProbeJitter, + // xDS settings + consecutive5xx, interval, baseEjectionTime, maxEjectionPercentage, enforcingConsecutive5xx, enforcingSuccessRate, successRateMinimumHosts, successRateRequestVolume, successRateStdevFactor, @@ -279,6 +357,45 @@ public Builder ewmaHalfLife(final Duration ewmaHalfLife) { return this; } + /** + * Configure a threshold for consecutive connection failures to a host. When the {@link LoadBalancer} + * consecutively fails to open connections in the amount greater or equal to the specified value, + * the host will be marked as unhealthy and connection establishment will take place in the background + * repeatedly until a connection is established. During that time, the host will not take part in + * load balancing selection. + *

+ * Use a negative value of the argument to disable health checking. + * + * @param failedConnectionsThreshold number of consecutive connection failures to consider a host unhealthy and eligible for + * background health checking. Use negative value to disable the health checking mechanism. + * @return {@code this}. + */ + Builder failedConnectionsThreshold(int failedConnectionsThreshold) { + this.failedConnectionsThreshold = failedConnectionsThreshold; + if (failedConnectionsThreshold == 0) { + throw new IllegalArgumentException("Not valid value: 0"); + } + return this; + } + + /** + * Configure an interval for health checking a host that failed to open connections. If no interval is provided + * using this method, a default value will be used. + *

+ * {@link #healthCheckFailedConnectionsThreshold(int)} can be used to disable the health checking mechanism + * and always consider all hosts for establishing new connections. + * + * @param interval interval at which a background health check will be scheduled. + * @param jitter the amount of jitter to apply to each retry {@code interval}. + * @return {@code this}. + */ + Builder failedConnectionsProbeInterval(Duration interval, Duration jitter) { + validateHealthCheckIntervals(interval, jitter); + this.failedConnectionsProbeInterval = requireNonNull(interval, "interval"); + this.failedConnectionsProbeJitter = requireNonNull(jitter, "jitter"); + return this; + } + /** * Set the threshold for consecutive failures before a host is ejected. * Defaults to 5.