Skip to content

Commit

Permalink
More Idel feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bryce-anderson committed Mar 26, 2024
1 parent fa916db commit 6cdfa93
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private static LoadBalancerFactory<InetSocketAddress, FilterableStreamingHttpLoa
// 429 TOO MANY REQUESTS. In the future the classification of responses will be configurable.
new OutlierDetectorConfig.Builder()
// set the interval to 30 seconds (default: to 10 seconds)
.scanInterval(ofSeconds(30))
.failureDetectorInterval(ofSeconds(30))
// set a more aggressive consecutive failure policy (default: 5)
.consecutive5xx(3)
// enabled failure percentage detection (default: 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public LoadBalancerFactory<ResolvedAddress, C> build() {
} else {
healthCheckConfig = new HealthCheckConfig(
executor,
outlierDetectorConfig.scanInterval(),
outlierDetectorConfig.scanIntervalJitter(),
outlierDetectorConfig.failureDetectorInterval(),
outlierDetectorConfig.failureDetectorIntervalJitter(),
outlierDetectorConfig.failedConnectionsThreshold(),
outlierDetectorConfig.serviceDiscoveryResubscribeInterval(),
outlierDetectorConfig.serviceDiscoveryResubscribeJitter());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ public final class OutlierDetectorConfig {
private final long ewmaCancellationPenalty;
private final long ewmaErrorPenalty;
private final int failedConnectionsThreshold;
private final Duration scanIntervalJitter;
private final Duration failureDetectorIntervalJitter;

private final Duration serviceDiscoveryResubscribeInterval;
private final Duration serviceDiscoveryResubscribeJitter;

// xDS defined settings
private final int consecutive5xx;
private final Duration scanInterval;
private final Duration failureDetectorInterval;
private final Duration baseEjectionTime;
private final int maxEjectionPercentage;
private final int enforcingConsecutive5xx;
Expand All @@ -70,10 +70,10 @@ public final class OutlierDetectorConfig {
private final Duration maxEjectionTimeJitter;

OutlierDetectorConfig(final Duration ewmaHalfLife, final long ewmaCancellationPenalty, final long ewmaErrorPenalty,
int failedConnectionsThreshold, final Duration scanIntervalJitter,
int failedConnectionsThreshold, final Duration failureDetectorIntervalJitter,
final Duration serviceDiscoveryResubscribeInterval, final Duration serviceDiscoveryResubscribeJitter,
// true xDS settings
final int consecutive5xx, final Duration scanInterval, final Duration baseEjectionTime,
final int consecutive5xx, final Duration failureDetectorInterval, final Duration baseEjectionTime,
final int maxEjectionPercentage, final int enforcingConsecutive5xx,
final int enforcingSuccessRate, final int successRateMinimumHosts,
final int successRateRequestVolume, final int successRateStdevFactor,
Expand All @@ -84,14 +84,15 @@ public final class OutlierDetectorConfig {
this.ewmaCancellationPenalty = ensureNonNegative(ewmaCancellationPenalty, "ewmaCancellationPenalty");
this.ewmaErrorPenalty = ensureNonNegative(ewmaErrorPenalty, "ewmaErrorPenalty");
this.failedConnectionsThreshold = failedConnectionsThreshold;
this.scanIntervalJitter = requireNonNull(scanIntervalJitter, "scanIntervalJitter");
this.failureDetectorIntervalJitter = requireNonNull(
failureDetectorIntervalJitter, "failureDetectorIntervalJitter");
this.serviceDiscoveryResubscribeInterval = requireNonNull(
serviceDiscoveryResubscribeInterval, "serviceDiscoveryResubscribeInterval");
this.serviceDiscoveryResubscribeJitter = requireNonNull(
serviceDiscoveryResubscribeJitter, "serviceDiscoveryResubscribeJitter");
// xDS settings.
this.consecutive5xx = consecutive5xx;
this.scanInterval = requireNonNull(scanInterval, "scanInterval");
this.failureDetectorInterval = requireNonNull(failureDetectorInterval, "failureDetectorInterval");
this.baseEjectionTime = requireNonNull(baseEjectionTime, "baseEjectionTime");
this.maxEjectionPercentage = maxEjectionPercentage;
this.enforcingConsecutive5xx = enforcingConsecutive5xx;
Expand Down Expand Up @@ -147,11 +148,11 @@ public int failedConnectionsThreshold() {
/**
* The jitter used along with the configured interval to determine duration between outlier detector checks.
* @return the jitter used along with the configured interval to determine duration between outlier detector checks.
* @see #scanInterval()
* @see Builder#scanInterval(Duration, Duration)
* @see #failureDetectorInterval()
* @see Builder#failureDetectorInterval(Duration, Duration)
*/
public Duration scanIntervalJitter() {
return scanIntervalJitter;
public Duration failureDetectorIntervalJitter() {
return failureDetectorIntervalJitter;
}

/**
Expand Down Expand Up @@ -189,8 +190,8 @@ public int consecutive5xx() {
* health check to see if a host can be considered revived.
* @return the interval on which to run failure percentage and success rate failure detectors.
*/
public Duration scanInterval() {
return scanInterval;
public Duration failureDetectorInterval() {
return failureDetectorInterval;
}

/**
Expand Down Expand Up @@ -318,7 +319,7 @@ public static final class Builder {

// Default xDS outlier detector settings.
private static final int DEFAULT_CONSECUTIVE_5XX = 5;
private static final Duration DEFAULT_SCAN_INTERVAL = ofSeconds(10);
private static final Duration DEFAULT_FAILURE_DETECTOR_INTERVAL = ofSeconds(10);
private static final Duration DEFAULT_BASE_EJECTION_TIME = ofSeconds(30);
private static final int DEFAULT_MAX_EJECTION_PERCENTAGE = 10;
private static final int DEFAULT_ENFORCING_CONSECUTIVE_5XX = 100;
Expand Down Expand Up @@ -348,7 +349,7 @@ public static final class Builder {
// default values (5s for L4 and 10s for xDS). We've decided to use the xDS default since it is viable for
// both whereas choosing 5s would necessitate changing a lot of related xDS settings such as min request
// volume, etc.
private Duration scanInterval = DEFAULT_SCAN_INTERVAL;
private Duration failureDetectorInterval = DEFAULT_FAILURE_DETECTOR_INTERVAL;

private Duration baseEjectionTime = DEFAULT_BASE_EJECTION_TIME;
private int maxEjectionPercentage = DEFAULT_MAX_EJECTION_PERCENTAGE;
Expand All @@ -373,11 +374,11 @@ public static final class Builder {
Builder(final OutlierDetectorConfig outlierDetectorConfig) {
this.ewmaHalfLife = outlierDetectorConfig.ewmaHalfLife;
this.failedConnectionsThreshold = outlierDetectorConfig.failedConnectionsThreshold;
this.intervalJitter = outlierDetectorConfig.scanIntervalJitter;
this.intervalJitter = outlierDetectorConfig.failureDetectorIntervalJitter;
this.serviceDiscoveryResubscribeInterval = outlierDetectorConfig.serviceDiscoveryResubscribeInterval;
this.serviceDiscoveryResubscribeJitter = outlierDetectorConfig.serviceDiscoveryResubscribeJitter;
this.consecutive5xx = outlierDetectorConfig.consecutive5xx;
this.scanInterval = outlierDetectorConfig.scanInterval;
this.failureDetectorInterval = outlierDetectorConfig.failureDetectorInterval;
this.baseEjectionTime = outlierDetectorConfig.baseEjectionTime;
this.maxEjectionPercentage = outlierDetectorConfig.maxEjectionPercentage;
this.enforcingConsecutive5xx = outlierDetectorConfig.enforcingConsecutive5xx;
Expand Down Expand Up @@ -408,7 +409,7 @@ public OutlierDetectorConfig build() {
return new OutlierDetectorConfig(ewmaHalfLife, ewmaCancellationPenalty, ewmaErrorPenalty,
failedConnectionsThreshold, intervalJitter, serviceDiscoveryResubscribeInterval, serviceDiscoveryResubscribeJitter,
// xDS settings
consecutive5xx, scanInterval, baseEjectionTime,
consecutive5xx, failureDetectorInterval, baseEjectionTime,
maxEjectionPercentage, enforcingConsecutive5xx,
enforcingSuccessRate, successRateMinimumHosts,
successRateRequestVolume, successRateStdevFactor,
Expand Down Expand Up @@ -485,7 +486,7 @@ public Builder serviceDiscoveryResubscribeInterval(Duration interval, Duration j
* 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 on the {@link #scanInterval()} (with jitter {@link #scanIntervalJitter()}) until a connection is
* repeatedly on the {@link #failureDetectorInterval()} (with jitter {@link #failureDetectorIntervalJitter()}) until a connection is
* established. During that time, the host will not take part in load balancing selection.
* <p>
* Use a negative value of the argument to disable health checking.
Expand Down Expand Up @@ -516,16 +517,16 @@ public Builder consecutive5xx(final int consecutive5xx) {
}

/**
* Set the scan interval on which the outlier detector will perform periodic tasks.
* Set the failure detector interval on which the outlier detector will perform periodic tasks.
* These tasks can include detection of outlier or the active revival checks.
* This method will also use either the default jitter or the provided interval, whichever is smaller.
* Defaults to 10 second interval with 3 second jitter.
* @param interval the interval on which to run failure percentage and success rate failure detectors.
* @return {@code this}
*/
public Builder scanInterval(final Duration interval) {
public Builder failureDetectorInterval(final Duration interval) {
requireNonNull(interval, "interval");
return scanInterval(interval, interval.compareTo(DEFAULT_HEALTH_CHECK_INTERVAL) < 0 ?
return failureDetectorInterval(interval, interval.compareTo(DEFAULT_HEALTH_CHECK_INTERVAL) < 0 ?
interval.dividedBy(2) : DEFAULT_HEALTH_CHECK_JITTER);
}

Expand All @@ -538,9 +539,9 @@ public Builder scanInterval(final Duration interval) {
* [interval - jitter, interval + jitter].
* @return {@code this}
*/
public Builder scanInterval(final Duration interval, final Duration jitter) {
public Builder failureDetectorInterval(final Duration interval, final Duration jitter) {
validateHealthCheckIntervals(interval, jitter);
this.scanInterval = requireNonNull(interval, "interval");
this.failureDetectorInterval = requireNonNull(interval, "interval");
this.intervalJitter = jitter;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ public void cancel() {

private Cancellable scheduleNextOutliersCheck(OutlierDetectorConfig currentConfig) {
Runnable checkOutliers = () -> sequentialExecutor.execute(this::sequentialCheckOutliers);
final long minIntervalNanos = currentConfig.scanInterval().toNanos() -
currentConfig.scanIntervalJitter().toNanos();
final long maxIntervalNanos = addWithOverflowProtection(currentConfig.scanInterval().toNanos(),
currentConfig.scanIntervalJitter().toNanos());
final long minIntervalNanos = currentConfig.failureDetectorInterval().toNanos() -
currentConfig.failureDetectorIntervalJitter().toNanos();
final long maxIntervalNanos = addWithOverflowProtection(currentConfig.failureDetectorInterval().toNanos(),
currentConfig.failureDetectorIntervalJitter().toNanos());
return executor.schedule(checkOutliers, ThreadLocalRandom.current().nextLong(
// + 1 to make the bound inclusive
minIntervalNanos, maxIntervalNanos + 1), TimeUnit.NANOSECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ abstract class LoadBalancerTest extends LoadBalancerTestScaffold {
static final String[] EMPTY_ARRAY = {};

static final OutlierDetectorConfig BASE_CONFIG = new OutlierDetectorConfig.Builder()
.scanInterval(ofSeconds(10), ZERO)
.failureDetectorInterval(ofSeconds(10), ZERO)
.enforcingFailurePercentage(0)
.enforcingConsecutive5xx(0)
.enforcingSuccessRate(0)
Expand Down Expand Up @@ -547,7 +547,7 @@ void healthCheckRecoversFromUnexpectedError() throws Exception {
lb = (TestableLoadBalancer<String, TestLoadBalancedConnection>)
baseLoadBalancerBuilder()
.outlierDetectorConfig(new OutlierDetectorConfig.Builder(BASE_CONFIG)
.scanInterval(ofMillis(50), ofMillis(10))
.failureDetectorInterval(ofMillis(50), ofMillis(10))
.failedConnectionsThreshold(1)
.build())
.backgroundExecutor(new DelegatingExecutor(testExecutor) {
Expand Down Expand Up @@ -723,7 +723,7 @@ void resubscribeToEventsNotTriggeredWhenDisabled() throws Exception {
lb = (TestableLoadBalancer<String, TestLoadBalancedConnection>)
baseLoadBalancerBuilder()
.outlierDetectorConfig(new OutlierDetectorConfig.Builder(BASE_CONFIG)
.scanInterval(ofMillis(50), ofMillis(10))
.failureDetectorInterval(ofMillis(50), ofMillis(10))
// Set resubscribe interval to very large number
.serviceDiscoveryResubscribeInterval(ofNanos(MAX_VALUE), ZERO)
.build())
Expand Down Expand Up @@ -757,7 +757,7 @@ void handleAllDiscoveryEvents() throws Exception {
lb = (TestableLoadBalancer<String, TestLoadBalancedConnection>)
baseLoadBalancerBuilder()
.outlierDetectorConfig(new OutlierDetectorConfig.Builder(BASE_CONFIG)
.scanInterval(ofMillis(50), ofMillis(10))
.failureDetectorInterval(ofMillis(50), ofMillis(10))
.build())
.backgroundExecutor(testExecutor)
.build()
Expand Down Expand Up @@ -834,7 +834,7 @@ protected final TestableLoadBalancer<String, TestLoadBalancedConnection> newTest
return (TestableLoadBalancer<String, TestLoadBalancedConnection>)
baseLoadBalancerBuilder()
.outlierDetectorConfig(new OutlierDetectorConfig.Builder(BASE_CONFIG)
.scanInterval(ofMillis(50), ofMillis(10))
.failureDetectorInterval(ofMillis(50), ofMillis(10))
.build())
.backgroundExecutor(testExecutor)
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public LoadBalancerBuilder<String, TestLoadBalancedConnection> linearSearchSpace
public LoadBalancerBuilder<String, TestLoadBalancedConnection> outlierDetectorConfig(
OutlierDetectorConfig config) {
underlying = underlying
.healthCheckInterval(config.scanInterval(), config.scanIntervalJitter())
.healthCheckInterval(config.failureDetectorInterval(), config.failureDetectorIntervalJitter())
.healthCheckFailedConnectionsThreshold(config.failedConnectionsThreshold())
.healthCheckResubscribeInterval(
config.serviceDiscoveryResubscribeInterval(), config.serviceDiscoveryResubscribeJitter());
Expand Down

0 comments on commit 6cdfa93

Please sign in to comment.