Skip to content

Commit

Permalink
loadbalancer-experimental: remove unused fields from OutlierDetectorC…
Browse files Browse the repository at this point in the history 
…onfig (#2841)

Motivation:

The outlier detector configuration currently mirrors that specified by
xDS but we don't have an intention to split local and remote origin
failures any time soon. That makes the config just confusing.

Modifications:

Remove the unused config options.

Result:

Less confusion.
  • Loading branch information
@bryce-anderson
bryce-anderson authored Feb 19, 2024
1 parent fb39471 commit b533ef2
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 244 deletions.
239 changes: 4 additions & 235 deletions ...alancer-experimental/src/main/java/io/servicetalk/loadbalancer/OutlierDetectorConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,33 +40,21 @@ public final class OutlierDetectorConfig {
private final int successRateMinimumHosts;
private final int successRateRequestVolume;
private final int successRateStdevFactor;
private final int consecutiveGatewayFailure;
private final int enforcingConsecutiveGatewayFailure;
private final boolean splitExternalLocalOriginErrors;
private final int consecutiveLocalOriginFailure;
private final int enforcingConsecutiveLocalOriginFailure;
private final int enforcingLocalOriginSuccessRate;
private final int failurePercentageThreshold;
private final int enforcingFailurePercentage;
private final int enforcingFailurePercentageLocalOrigin;
private final int failurePercentageMinimumHosts;
private final int failurePercentageRequestVolume;
private final Duration maxEjectionTime;
private final Duration maxEjectionTimeJitter;
private final boolean successfulActiveHealthCheckUnejectHost;

OutlierDetectorConfig(final Duration ewmaHalfLife,
final int consecutive5xx, final Duration interval, final Duration baseEjectionTime,
final int maxEjectionPercentage, final int enforcingConsecutive5xx,
final int enforcingSuccessRate, final int successRateMinimumHosts,
final int successRateRequestVolume, final int successRateStdevFactor,
final int consecutiveGatewayFailure, final int enforcingConsecutiveGatewayFailure,
final boolean splitExternalLocalOriginErrors, final int consecutiveLocalOriginFailure,
final int enforcingConsecutiveLocalOriginFailure, final int enforcingLocalOriginSuccessRate,
final int failurePercentageThreshold, final int enforcingFailurePercentage,
final int enforcingFailurePercentageLocalOrigin, final int failurePercentageMinimumHosts,
final int failurePercentageRequestVolume, final Duration maxEjectionTime,
final Duration maxEjectionTimeJitter, final boolean successfulActiveHealthCheckUnejectHost) {
final int failurePercentageMinimumHosts, final int failurePercentageRequestVolume,
final Duration maxEjectionTime, final Duration maxEjectionTimeJitter) {
this.ewmaHalfLife = requireNonNull(ewmaHalfLife, "ewmaHalfLife");
this.consecutive5xx = consecutive5xx;
this.interval = requireNonNull(interval, "interval");
Expand All @@ -77,20 +65,12 @@ public final class OutlierDetectorConfig {
this.successRateMinimumHosts = successRateMinimumHosts;
this.successRateRequestVolume = successRateRequestVolume;
this.successRateStdevFactor = successRateStdevFactor;
this.consecutiveGatewayFailure = consecutiveGatewayFailure;
this.enforcingConsecutiveGatewayFailure = enforcingConsecutiveGatewayFailure;
this.splitExternalLocalOriginErrors = splitExternalLocalOriginErrors;
this.consecutiveLocalOriginFailure = consecutiveLocalOriginFailure;
this.enforcingConsecutiveLocalOriginFailure = enforcingConsecutiveLocalOriginFailure;
this.enforcingLocalOriginSuccessRate = enforcingLocalOriginSuccessRate;
this.failurePercentageThreshold = failurePercentageThreshold;
this.enforcingFailurePercentage = enforcingFailurePercentage;
this.enforcingFailurePercentageLocalOrigin = enforcingFailurePercentageLocalOrigin;
this.failurePercentageMinimumHosts = failurePercentageMinimumHosts;
this.failurePercentageRequestVolume = failurePercentageRequestVolume;
this.maxEjectionTime = requireNonNull(maxEjectionTime, "maxEjectionTime");
this.maxEjectionTimeJitter = requireNonNull(maxEjectionTimeJitter, "maxEjectionTimeJitter");
this.successfulActiveHealthCheckUnejectHost = successfulActiveHealthCheckUnejectHost;
}

/**
Expand Down Expand Up @@ -181,61 +161,6 @@ public int successRateStdevFactor() {
return successRateStdevFactor;
}

/**
* The threshold for consecutive gateway failures before local the host is ejected.
* @return the threshold for consecutive gateway failures before local the host is ejected.
*/
public int consecutiveGatewayFailure() {
return consecutiveGatewayFailure;
}

/**
* The probability in percentage that a host will be marked as unhealthy when a host exceeds the consecutive gateway
* failure threshold.
* @return the probability with which the host should be marked as unhealthy.
*/
public int enforcingConsecutiveGatewayFailure() {
return enforcingConsecutiveGatewayFailure;
}

/**
* Whether to split local origin and remote origin failures into separate failure detectors.
* Note: this being true predicates the validity of many of the configuration parameters of this class.
* @return true if local and remote origin failures should be split, false otherwise.
*/
public boolean splitExternalLocalOriginErrors() {
return splitExternalLocalOriginErrors;
}

/**
* The threshold for locally originated consecutive failures before ejection occurs.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @return the threshold of consecutive locally originated failures for ejection.
*/
public int consecutiveLocalOriginFailure() {
return consecutiveLocalOriginFailure;
}

/**
* The probability in percentage that a host will be marked as unhealthy when a host exceeds the consecutive local
* origin failure threshold.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @return the probability with which the host should be marked as unhealthy.
*/
public int enforcingConsecutiveLocalOriginFailure() {
return enforcingConsecutiveLocalOriginFailure;
}

/**
* The probability in percentage that a host will be marked as unhealthy when a host exceeds the success rate
* outlier detectors threshold for local origin failures.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @return the probability with which the host should be marked as unhealthy.
*/
public int enforcingLocalOriginSuccessRate() {
return enforcingLocalOriginSuccessRate;
}

/**
* The failure threshold in percentage for ejecting a host.
* @return the failure threshold in percentage for ejecting a host.
Expand All @@ -253,16 +178,6 @@ public int enforcingFailurePercentage() {
return enforcingFailurePercentage;
}

/**
* The probability in percentage that a host will be marked as unhealthy when a host exceeds the failure percentage
* outlier detectors threshold for locally originated failures.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @return the probability with which the host should be marked as unhealthy.
*/
public int enforcingFailurePercentageLocalOrigin() {
return enforcingFailurePercentageLocalOrigin;
}

/**
* The minimum number of hosts required to perform the failure percentage outlier detector analysis.
* @return the minimum number of hosts required to perform the failure percentage outlier detector analysis.
Expand Down Expand Up @@ -298,15 +213,6 @@ public Duration maxEjectionTimeJitter() {
return maxEjectionTimeJitter;
}

/**
* Whether to un-eject a host that has had a successful active health check event to be revived regardless of the
* remaining ejection time.
* @return whether to un-eject a host regardless of remaining ejection time.
*/
public boolean successfulActiveHealthCheckUnejectHost() {
return successfulActiveHealthCheckUnejectHost;
}

/**
* A builder for {@link OutlierDetectorConfig} instances.
*/
Expand All @@ -330,23 +236,10 @@ public static final class Builder {

private int successRateStdevFactor = 1900;

private int consecutiveGatewayFailure = 5;

private int enforcingConsecutiveGatewayFailure;

private boolean splitExternalLocalOriginErrors;

private int consecutiveLocalOriginFailure = 5;

private int enforcingConsecutiveLocalOriginFailure = 100;

private int enforcingLocalOriginSuccessRate = 100;

private int failurePercentageThreshold = 85;

private int enforcingFailurePercentage;

private int enforcingFailurePercentageLocalOrigin;

private int failurePercentageMinimumHosts = 5;

Expand All @@ -356,8 +249,6 @@ public static final class Builder {

private Duration maxEjectionTimeJitter = Duration.ZERO;

private boolean successfulActiveHealthCheckUnejectHost = true;

/**
* Build the OutlierDetectorConfig.
* @return the OutlierDetectorConfig.
Expand All @@ -368,14 +259,9 @@ public OutlierDetectorConfig build() {
maxEjectionPercentage, enforcingConsecutive5xx,
enforcingSuccessRate, successRateMinimumHosts,
successRateRequestVolume, successRateStdevFactor,
consecutiveGatewayFailure, enforcingConsecutiveGatewayFailure,
splitExternalLocalOriginErrors, consecutiveLocalOriginFailure,
enforcingConsecutiveLocalOriginFailure, enforcingLocalOriginSuccessRate,
failurePercentageThreshold, enforcingFailurePercentage,
enforcingFailurePercentageLocalOrigin, failurePercentageMinimumHosts,
failurePercentageRequestVolume, maxEjectionTime,
maxEjectionTimeJitter,
successfulActiveHealthCheckUnejectHost);
failurePercentageMinimumHosts, failurePercentageRequestVolume,
maxEjectionTime, maxEjectionTimeJitter);
}

/**
Expand Down Expand Up @@ -509,93 +395,6 @@ public Builder successRateStdevFactor(final int successRateStdevFactor) {
return this;
}

/**
* Set the threshold for consecutive gateway failures before local the host is ejected.
* Defaults to 5.
* @param consecutiveGatewayFailure the threshold for consecutive gateway failures before local the host is
* ejected.
* @return {@code this}.
*/
public Builder consecutiveGatewayFailure(final int consecutiveGatewayFailure) {
ensurePositive(consecutiveGatewayFailure, "consecutiveGatewayFailure");
this.consecutiveGatewayFailure = consecutiveGatewayFailure;
return this;
}

/**
* Set the probability in percentage that a host will be marked as unhealthy when a host exceeds the consecutive
* gateway failure threshold.
* Defaults to 0.
* @param enforcingConsecutiveGatewayFailure the probability in percentage that a host will be marked as
* unhealthy when a host exceeds the consecutive gateway failure
* threshold.
* @return {@code this}.
*/
public Builder enforcingConsecutiveGatewayFailure(final int enforcingConsecutiveGatewayFailure) {
ensureNonNegative(enforcingConsecutiveGatewayFailure, "enforcingConsecutiveGatewayFailure");
this.enforcingConsecutiveGatewayFailure = enforcingConsecutiveGatewayFailure;
return this;
}

/**
* Set whether to split local origin and remote origin failures into separate failure detectors.
* Defaults to false.
* Note: this being true predicates the validity of many of the configuration parameters of this class.
* @param splitExternalLocalOriginErrors whether to split local origin and remote origin failures into separate
* failure detectors.
* @return {@code this}.
*/
public Builder splitExternalLocalOriginErrors(final boolean splitExternalLocalOriginErrors) {
this.splitExternalLocalOriginErrors = splitExternalLocalOriginErrors;
return this;
}

/**
* Set the threshold for locally originated consecutive failures before ejection occurs.
* Defaults to 5.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @param consecutiveLocalOriginFailure the threshold for locally originated consecutive failures before
* ejection occurs.
* @return {@code this}.
*/
public Builder consecutiveLocalOriginFailure(final int consecutiveLocalOriginFailure) {
ensurePositive(consecutiveLocalOriginFailure, "consecutiveLocalOriginFailure");
this.consecutiveLocalOriginFailure = consecutiveLocalOriginFailure;
return this;
}

/**
* Set the probability in percentage that a host will be marked as unhealthy when a host exceeds the consecutive
* local origin failure threshold.
* Defaults to 100%.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @param enforcingConsecutiveLocalOriginFailure the probability in percentage that a host will be marked as
* unhealthy when a host exceeds the consecutive local origin
* failure threshold.
* @return {@code this}.
*/
public Builder enforcingConsecutiveLocalOriginFailure(final int enforcingConsecutiveLocalOriginFailure) {
ensureNonNegative(enforcingConsecutiveLocalOriginFailure, "enforcingConsecutiveLocalOriginFailure");
this.enforcingConsecutiveLocalOriginFailure = enforcingConsecutiveLocalOriginFailure;
return this;
}

/**
* Set the probability in percentage that a host will be marked as unhealthy when a host exceeds the success
* rate outlier detectors threshold for local origin failures.
* Defaults to 100%.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @param enforcingLocalOriginSuccessRate the probability in percentage that a host will be marked as unhealthy
* when a host exceeds the success rate outlier detectors threshold for
* local origin failures.
* @return {@code this}.
*/
public Builder enforcingLocalOriginSuccessRate(final int enforcingLocalOriginSuccessRate) {
ensureNonNegative(enforcingLocalOriginSuccessRate, "enforcingLocalOriginSuccessRate");
this.enforcingLocalOriginSuccessRate = enforcingLocalOriginSuccessRate;
return this;
}

/**
* Set the failure threshold in percentage for ejecting a host.
* Defaults to 85%.
Expand All @@ -622,22 +421,6 @@ public Builder enforcingFailurePercentage(final int enforcingFailurePercentage)
return this;
}

/**
* Set the probability in percentage that a host will be marked as unhealthy when a host exceeds the failure
* percentage outlier detectors threshold for locally originated failures.
* Defaults to 0%.
* Note: this value is only considered if {@code splitExternalLocalOriginErrors()} is true.
* @param enforcingFailurePercentageLocalOrigin the probability in percentage that a host will be marked as
* unhealthy when a host exceeds the failure percentage outlier
* detectors threshold for locally originated failures.
* @return {@code this}.
*/
public Builder enforcingFailurePercentageLocalOrigin(final int enforcingFailurePercentageLocalOrigin) {
ensureNonNegative(enforcingFailurePercentageLocalOrigin, "enforcingFailurePercentageLocalOrigin");
this.enforcingFailurePercentageLocalOrigin = enforcingFailurePercentageLocalOrigin;
return this;
}

/**
* Set the minimum number of hosts required to perform the failure percentage outlier detector analysis.
* Defaults to 5.
Expand Down Expand Up @@ -691,20 +474,6 @@ public Builder maxEjectionTimeJitter(final Duration maxEjectionTimeJitter) {
ensureNonNegative(maxEjectionTimeJitter.toNanos(), "maxEjectionTimeJitter");
return this;
}

/**
* Set whether to un-eject a host that has had a successful active health check event to be revived regardless
* of the remaining ejection time.
* Defaults to true.
* @param successfulActiveHealthCheckUnejectHost whether to un-eject a host that has had a successful active
* health check event to be revived regardless of the remaining
* ejection time.
* @return {@code this}.
*/
public Builder successfulActiveHealthCheckUnejectHost(final boolean successfulActiveHealthCheckUnejectHost) {
this.successfulActiveHealthCheckUnejectHost = successfulActiveHealthCheckUnejectHost;
return this;
}
}

static boolean enforcing(int enforcingPercentage) {
Expand Down
6 changes: 1 addition & 5 deletions ...balancer-experimental/src/test/java/io/servicetalk/loadbalancer/XdsHealthCheckerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ private OutlierDetectorConfig.Builder withAllEnforcing() {
// set enforcing rates to 100% so that we don't have to deal with statics
.enforcingConsecutive5xx(100)
.enforcingFailurePercentage(100)
.enforcingSuccessRate(100)
.enforcingConsecutiveGatewayFailure(100)
.enforcingConsecutiveLocalOriginFailure(100)
.enforcingFailurePercentageLocalOrigin(100)
.enforcingLocalOriginSuccessRate(100);
.enforcingSuccessRate(100);
}

@BeforeEach
Expand Down
11 changes: 7 additions & 4 deletions ...lancer-experimental/src/test/java/io/servicetalk/loadbalancer/XdsHealthIndicatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,15 @@ class XdsHealthIndicatorTest {
@BeforeEach
void initialize() {
testExecutor = executor.executor();
config = new OutlierDetectorConfig.Builder()
config = baseBuilder().build();
initIndicator();
}

private OutlierDetectorConfig.Builder baseBuilder() {
return new OutlierDetectorConfig.Builder()
.maxEjectionTimeJitter(Duration.ZERO)
.maxEjectionTime(Duration.ofSeconds(MAX_EJECTION_SECONDS))
.baseEjectionTime(Duration.ofSeconds(1))
.build();
initIndicator();
.baseEjectionTime(Duration.ofSeconds(1));
}

private void initIndicator() {
Expand Down

0 comments on commit b533ef2

Please sign in to comment.