From f6e1fa7497ba4dd6d29affc4b5607f5afd1e202a Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Mon, 22 Apr 2024 17:50:24 -0600 Subject: [PATCH 1/8] loadbalancer-experimental: add a provider for enabled DefaultLoadBalancer Motivation: We want to make it easy for people to enable the DefaultLoadBalancer. Modifications: - add a new package that includes a provider that can be used --- .../README.adoc | 45 +++ .../build.gradle | 44 +++ .../gradle/checkstyle/suppressions.xml | 26 ++ .../gradle/spotbugs/test-exclusions.xml | 35 +++ .../DefaultHttpLoadBalancerProvider.java | 87 ++++++ .../DefaultLoadBalancerObserver.java | 104 +++++++ .../DefaultLoadBalancerProviderConfig.java | 268 ++++++++++++++++++ .../experimental/package-info.java | 16 ++ .../loadbalancer/OutlierDetectorConfig.java | 2 +- settings.gradle | 1 + 10 files changed, 627 insertions(+), 1 deletion(-) create mode 100644 servicetalk-loadbalancer-experimental-provider/README.adoc create mode 100644 servicetalk-loadbalancer-experimental-provider/build.gradle create mode 100644 servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml create mode 100644 servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml create mode 100644 servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java create mode 100644 servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java create mode 100644 servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java create mode 100644 servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java diff --git a/servicetalk-loadbalancer-experimental-provider/README.adoc b/servicetalk-loadbalancer-experimental-provider/README.adoc new file mode 100644 index 0000000000..e4b7803d50 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/README.adoc @@ -0,0 +1,45 @@ += DefaultLoadBalancer Providers + +This package provides providers for enabling the DefaultLoadBalancer via system properties to allow for easy +experimentation that doesn't require a recompilation of the application. + +== Enabling DefaultLoadBalancer via System Properties + +=== Dynamically Loading the DefaultHttpLoadBalancerProvider + +This package uses the standard providers pattern. To enable the provider you need to both include this package as +part of your application bundle and also include a file in the resources as follows: +``` +resources/META-INF/services/io.servicetalk.http.api.HttpProviders$SingleAddressHttpClientBuilderProvider +``` + +The contents of this must should contain the line + +``` +io.servicetalk.loadbalancer.experimental.DefaultHttpLoadBalancerProvider +``` + +=== Targeting Clients for Which to Enable DefaultLoadBalancer + +The `DefaultHttpLoadBalancerProvider` supports enabling the load balancer either for all clients or only a set of +specific clients. Enabling the load balancer for all clients can be done by setting the following system property: + +``` +io.servicetalk.loadbalancer.experimental.clientsEnabledFor=all +``` + +The experimental load balancer can also be enabled for only a subset of clients. This can be done via setting the +system property to a comma separated list: + +``` +io.servicetalk.loadbalancer.experimental.clientsEnabledFor=service1,service2 +``` + +The specific names will depend on how the client is build. If the client is built using a `HostAndPort`, the names are +only the host component. If the client is built using some other unresolved address form then the string representation +of that is used. + +=== All Supported Properties + +All system properties contain the prefix "io.servicetalk.loadbalancer.experimental.". A comprehensive list of the +supported properties can be found in the `DefaultLoadBalancerProviderConfig` class for reference. \ No newline at end of file diff --git a/servicetalk-loadbalancer-experimental-provider/build.gradle b/servicetalk-loadbalancer-experimental-provider/build.gradle new file mode 100644 index 0000000000..38b4c04762 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/build.gradle @@ -0,0 +1,44 @@ +/* + * Copyright © 2018-2019 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. + */ + +apply plugin: "io.servicetalk.servicetalk-gradle-plugin-internal-library" + +dependencies { + implementation platform(project(":servicetalk-dependencies")) + testImplementation enforcedPlatform("org.junit:junit-bom:$junit5Version") + + api project(":servicetalk-client-api") + api project(":servicetalk-concurrent-api") + + implementation project(":servicetalk-annotations") + implementation project(":servicetalk-loadbalancer") + implementation project(":servicetalk-loadbalancer-experimental") + implementation project(":servicetalk-http-api") + implementation project(":servicetalk-http-netty") + implementation project(":servicetalk-utils-internal") + implementation "com.google.code.findbugs:jsr305" + implementation "org.slf4j:slf4j-api" + + testImplementation testFixtures(project(":servicetalk-concurrent-api")) + testImplementation testFixtures(project(":servicetalk-concurrent-internal")) + testImplementation project(":servicetalk-concurrent-test-internal") + testImplementation project(":servicetalk-test-resources") + testImplementation "org.junit.jupiter:junit-jupiter-api" + testImplementation "org.junit.jupiter:junit-jupiter-params" + testImplementation "org.apache.logging.log4j:log4j-core" + testImplementation "org.hamcrest:hamcrest:$hamcrestVersion" + testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" +} diff --git a/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml b/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml new file mode 100644 index 0000000000..78094076e5 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml @@ -0,0 +1,26 @@ + + + + + + + + diff --git a/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml b/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml new file mode 100644 index 0000000000..7e276c24b9 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java new file mode 100644 index 0000000000..ca70f8f097 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java @@ -0,0 +1,87 @@ +/* + * 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.experimental; + +import io.servicetalk.client.api.LoadBalancerFactory; +import io.servicetalk.http.api.FilterableStreamingHttpLoadBalancedConnection; +import io.servicetalk.http.api.HttpLoadBalancerFactory; +import io.servicetalk.http.api.HttpProviders; +import io.servicetalk.http.api.SingleAddressHttpClientBuilder; +import io.servicetalk.http.netty.DefaultHttpLoadBalancerFactory; +import io.servicetalk.loadbalancer.LoadBalancers; +import io.servicetalk.transport.api.HostAndPort; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static java.util.Objects.requireNonNull; + +public class DefaultHttpLoadBalancerProvider implements HttpProviders.SingleAddressHttpClientBuilderProvider { + + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultHttpLoadBalancerProvider.class); + + private final DefaultLoadBalancerProviderConfig config; + + public DefaultHttpLoadBalancerProvider() { + this(DefaultLoadBalancerProviderConfig.INSTANCE); + } + + // exposed for testing + DefaultHttpLoadBalancerProvider(final DefaultLoadBalancerProviderConfig config) { + this.config = requireNonNull(config, "config"); + } + + @Override + public SingleAddressHttpClientBuilder newBuilder(U address, + SingleAddressHttpClientBuilder builder) { + final String serviceName = clientNameFromAddress(address); + if (config.enabledForServiceName(serviceName)) { + try { + HttpLoadBalancerFactory loadBalancerFactory = DefaultHttpLoadBalancerFactory.Builder.from( + defaultLoadBalancer(serviceName)).build(); + return builder.loadBalancerFactory(loadBalancerFactory); + } catch (Throwable ex) { + LOGGER.warn("Failed to enabled DefaultLoadBalancer for client to address {}.", address, ex); + } + } + return builder; + } + + private LoadBalancerFactory defaultLoadBalancer( + String serviceName) { + return LoadBalancers. + builder("experimental-load-balancer") + .loadBalancerObserver(new DefaultLoadBalancerObserver(serviceName)) + // set up the new features. + .outlierDetectorConfig(config.outlierDetectorConfig()) + .loadBalancingPolicy(config.getLoadBalancingPolicy()) + .build(); + } + + private static String clientNameFromAddress(Object address) { + String serviceName; + if (address instanceof HostAndPort) { + serviceName = ((HostAndPort) address).hostName(); + } else if (address instanceof String) { + serviceName = (String) address; + } else { + LOGGER.warn("Unknown service address type={} was provided, " + + "default 'toString()' will be used as serviceName", address.getClass()); + serviceName = address.toString(); + } + return serviceName; + } +} diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java new file mode 100644 index 0000000000..d92accdbd5 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java @@ -0,0 +1,104 @@ +/* + * 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.experimental; + +import io.servicetalk.client.api.NoActiveHostException; +import io.servicetalk.client.api.ServiceDiscovererEvent; + +import io.servicetalk.loadbalancer.LoadBalancerObserver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Collection; +import javax.annotation.Nullable; + +import static java.util.Objects.requireNonNull; + +public class DefaultLoadBalancerObserver implements LoadBalancerObserver { + + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLoadBalancerObserver.class); + + private final String clientName; + + public DefaultLoadBalancerObserver(final String clientName) { + this.clientName = requireNonNull(clientName, "clientName"); + } + + @Override + public HostObserver hostObserver(Object resolvedAddress) { + return new HostObserverImpl(resolvedAddress); + } + + @Override + public void onNoHostsAvailable() { + LOGGER.debug("{}- No hosts available", clientName); + } + + @Override + public void onServiceDiscoveryEvent(Collection> events, int oldHostSetSize, + int newHostSetSize) { + LOGGER.debug("{}- service discovery events. Old host set size: {}, new host set size: {}, events: {}", + clientName, oldHostSetSize, newHostSetSize, events); + } + + @Override + public void onNoActiveHostsAvailable(int hostSetSize, NoActiveHostException exception) { + LOGGER.debug("{}- No active hosts available. Host set size: {}.", clientName, hostSetSize, exception); + } + + private final class HostObserverImpl implements HostObserver { + + private final Object resolvedAddress; + + HostObserverImpl(final Object resolvedAddress) { + this.resolvedAddress = resolvedAddress; + } + + @Override + public void onHostMarkedExpired(int connectionCount) { + LOGGER.debug("{}:{}- onHostMarkedExpired(connectionCount: {})", + clientName, resolvedAddress, connectionCount); + } + + @Override + public void onActiveHostRemoved(int connectionCount) { + LOGGER.debug("{}:{}- onActiveHostRemoved(connectionCount: {})", + clientName, resolvedAddress, connectionCount); + } + + @Override + public void onExpiredHostRevived(int connectionCount) { + LOGGER.debug("{}:{}- onExpiredHostRevived(connectionCount: {})", + clientName, resolvedAddress, connectionCount); + } + + @Override + public void onExpiredHostRemoved(int connectionCount) { + LOGGER.debug("{}:{}- onExpiredHostRemoved(connectionCount: {})", + clientName, resolvedAddress, connectionCount); + } + + @Override + public void onHostMarkedUnhealthy(@Nullable Throwable cause) { + LOGGER.debug("{}:{}- onExpiredHostRemoved(ex)", clientName, resolvedAddress, cause); + } + + @Override + public void onHostRevived() { + LOGGER.debug("{}:{}- onHostRevived()", clientName, resolvedAddress); + } + } +} diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java new file mode 100644 index 0000000000..2b9fd252d5 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java @@ -0,0 +1,268 @@ +/* + * 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.experimental; + +import io.servicetalk.client.api.LoadBalancedConnection; + +import io.servicetalk.loadbalancer.LoadBalancingPolicy; +import io.servicetalk.loadbalancer.OutlierDetectorConfig; +import io.servicetalk.loadbalancer.P2CLoadBalancingPolicy; +import io.servicetalk.loadbalancer.RoundRobinLoadBalancingPolicy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; +import java.time.Duration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Properties; +import java.util.Set; + +import static java.time.Duration.ZERO; +import static java.time.Duration.ofMillis; +import static java.time.Duration.ofSeconds; +import static java.util.Objects.requireNonNull; + +final class DefaultLoadBalancerProviderConfig { + + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultHttpLoadBalancerProvider.class); + + private enum LBPolicy { + P2C, + RoundRobin + } + + // This prefix should be applied to all individual properties. + private static final String PROPERTY_PREFIX = "io.servicetalk.loadbalancer.experimental."; + + private static final String PROP_CLIENTS_ENABLED_FOR = "clientsEnabledFor"; + + private static final String PROP_FAILED_CONNECTIONS_THRESHOLD = "healthCheckFailedConnectionsThreshold"; + private static final String PROP_LOAD_BALANCING_POLICY = "clientExperimentalLoadBalancerPolicy"; + private static final String PROP_EWMA_HALF_LIFE_MS = "experimentalLoadBalancerEwmaHalfLifeMs"; + private static final String PROP_CONSECUTIVE_5XX = "experimentalLoadBalancerConsecutive5xx"; + private static final String PROP_INTERVAL_MS = "experimentalLoadBalancerIntervalMs"; + private static final String PROP_BASE_EJECTION_TIME_MS = "experimentalLoadBalancerBaseEjectionTimeMs"; + private static final String PROP_MAX_EJECTION_PERCENT = "experimentalLoadBalancerMaxEjectionPercent"; + private static final String PROP_ENFORCING_CONSECUTIVE_5XX = "experimentalLoadBalancerEnforcingConsecutive5xx"; + private static final String PROP_ENFORCING_SUCCESS_RATE = "experimentalLoadBalancerEnforcingSuccessRate"; + private static final String PROP_SUCCESS_RATE_MIN_HOSTS = "experimentalLoadBalancerSuccessRateMinimumHosts"; + private static final String PROP_SUCCESS_RATE_REQUEST_VOL = "experimentalLoadBalancerSuccessRateRequestVolume"; + private static final String PROP_SUCCESS_RATE_STDEV_FACTOR = "experimentalLoadBalancerSuccessRateStdevFactor"; + private static final String PROP_FAILURE_PERCENTAGE_THRESHOLD = + "experimentalLoadBalancerFailurePercentageThreshold"; + private static final String PROP_ENFORCING_FAILURE_PERCENTAGE = + "experimentalLoadBalancerEnforcingFailurePercentage"; + private static final String PROP_FAILURE_PERCENTAGE_MIN_HOSTS = + "experimentalLoadBalancerFailurePercentageMinimumHosts"; + private static final String PROP_FAILURE_PERCENTAGE_REQUEST_VOL = + "experimentalLoadBalancerFailurePercentageRequestVolume"; + private static final String PROP_MAX_EJECTION_TIME_MS = "experimentalLoadBalancerMaxEjectionTimeMs"; + private static final String PROP_EJECTION_TIME_JITTER_MS = "experimentalLoadBalancerEjectionTimeJitterMs"; + + static final DefaultLoadBalancerProviderConfig INSTANCE = new DefaultLoadBalancerProviderConfig(); + + private final Properties properties; + + private final String rawClientsEnabledFor; + private final Set clientsEnabledFor; + private final int failedConnectionsThreshold; + private final LBPolicy lbPolicy; + private final Duration ewmaHalfLife; + private final int consecutive5xx; + private final Duration interval; + private final Duration baseEjectionTime; + private final int maxEjectionPercentage; + private final int enforcingConsecutive5xx; + private final int enforcingSuccessRate; + private final int successRateMinimumHosts; + private final int successRateRequestVolume; + private final int successRateStdevFactor; + private final int failurePercentageThreshold; + private final int enforcingFailurePercentage; + private final int failurePercentageMinimumHosts; + private final int failurePercentageRequestVolume; + private final Duration maxEjectionTime; + private final Duration ejectionTimeJitter; + + private final Map fields; + + private DefaultLoadBalancerProviderConfig() { + this(System.getProperties()); + } + + private DefaultLoadBalancerProviderConfig(Properties properties) { + this.properties = requireNonNull(properties, "properties"); + rawClientsEnabledFor = getString(PROP_CLIENTS_ENABLED_FOR, ""); + clientsEnabledFor = getClientsEnabledFor(rawClientsEnabledFor); + failedConnectionsThreshold = getInt(PROP_FAILED_CONNECTIONS_THRESHOLD, 5 /*ST default*/); + lbPolicy = getLBPolicy(); + ewmaHalfLife = ofMillis(getLong(PROP_EWMA_HALF_LIFE_MS, ofSeconds(10).toMillis())); + consecutive5xx = getInt(PROP_CONSECUTIVE_5XX, 5); + interval = ofMillis(getLong(PROP_INTERVAL_MS, ofSeconds(10).toMillis())); + baseEjectionTime = ofMillis(getLong(PROP_BASE_EJECTION_TIME_MS, ofSeconds(30).toMillis())); + maxEjectionPercentage = getInt(PROP_MAX_EJECTION_PERCENT, 20); + enforcingConsecutive5xx = getInt(PROP_ENFORCING_CONSECUTIVE_5XX, 100); + enforcingSuccessRate = getInt(PROP_ENFORCING_SUCCESS_RATE, 100); + successRateMinimumHosts = getInt(PROP_SUCCESS_RATE_MIN_HOSTS, 5); + successRateRequestVolume = getInt(PROP_SUCCESS_RATE_REQUEST_VOL, 100); + successRateStdevFactor = getInt(PROP_SUCCESS_RATE_STDEV_FACTOR, 1900); + failurePercentageThreshold = getInt(PROP_FAILURE_PERCENTAGE_THRESHOLD, 85); + enforcingFailurePercentage = getInt(PROP_ENFORCING_FAILURE_PERCENTAGE, 0); + failurePercentageMinimumHosts = getInt(PROP_FAILURE_PERCENTAGE_MIN_HOSTS, 5); + failurePercentageRequestVolume = getInt(PROP_FAILURE_PERCENTAGE_REQUEST_VOL, 50); + maxEjectionTime = ofMillis(getLong(PROP_MAX_EJECTION_TIME_MS, ofSeconds(90).toMillis())); + ejectionTimeJitter = ofMillis(getLong(PROP_EJECTION_TIME_JITTER_MS, ZERO.toMillis())); + + fields = populateEventFields(); + } + + private Map populateEventFields() { + Map fields = new HashMap<>(); + fields.put("clientsEnabledFor", String.valueOf(clientsEnabledFor)); + fields.put("failedConnectionsThreshold", String.valueOf(failedConnectionsThreshold)); + fields.put("loadBalancingPolicy", String.valueOf(lbPolicy)); + fields.put("ewmaHalfLifeMillis", String.valueOf(ewmaHalfLife.toMillis())); + fields.put("consecutive5xx", String.valueOf(consecutive5xx)); + fields.put("intervalMillis", String.valueOf(interval.toMillis())); + fields.put("baseEjectionTimeMillis", String.valueOf(baseEjectionTime.toMillis())); + fields.put("ejectionTimeJitterMillis", String.valueOf(ejectionTimeJitter.toMillis())); + fields.put("maxEjectionPercentage", String.valueOf(maxEjectionPercentage)); + fields.put("enforcingConsecutive5xx", String.valueOf(enforcingConsecutive5xx)); + fields.put("enforcingSuccessRate", String.valueOf(enforcingSuccessRate)); + fields.put("successRateMinimumHosts", String.valueOf(successRateMinimumHosts)); + fields.put("successRateRequestVolume", String.valueOf(successRateRequestVolume)); + fields.put("successRateStdevFactor", String.valueOf(successRateStdevFactor)); + fields.put("failurePercentageThreshold", String.valueOf(failurePercentageThreshold)); + fields.put("enforcingFailurePercentage", String.valueOf(enforcingFailurePercentage)); + fields.put("failurePercentageMinimumHosts", String.valueOf(failurePercentageMinimumHosts)); + fields.put("failurePercentageRequestVolume", String.valueOf(failurePercentageRequestVolume)); + fields.put("maxEjectionTimeMillis", String.valueOf(maxEjectionTime.toMillis())); + return fields; + } + + // TODO: what can we do with this? + Map getEventFields() { + return fields; + } + + private LBPolicy getLBPolicy() { + return getString(PROP_LOAD_BALANCING_POLICY, "p2c") + .equalsIgnoreCase(LBPolicy.P2C.name()) ? LBPolicy.P2C : LBPolicy.RoundRobin; + } + + @Nullable + private Set getClientsEnabledFor(String propertyValue) { + propertyValue = propertyValue.trim(); + final Set result = new HashSet<>(); + // if enabled for all there is no need to parse. + if (!"all".equals(propertyValue)) { + for (String serviceName : propertyValue.split(",")) { + serviceName = serviceName.trim(); + if (!serviceName.isEmpty()) { + result.add(serviceName); + } + } + } + return result; + } + + LoadBalancingPolicy getLoadBalancingPolicy() { + if (lbPolicy == lbPolicy.P2C) { + return new P2CLoadBalancingPolicy.Builder().build(); + } else { + return new RoundRobinLoadBalancingPolicy.Builder().build(); + } + } + + boolean enabledForServiceName(String serviceName) { + return clientsEnabledFor == null || clientsEnabledFor.contains(serviceName); + } + + OutlierDetectorConfig outlierDetectorConfig() { + return new OutlierDetectorConfig.Builder() + .failedConnectionsThreshold(failedConnectionsThreshold) + .ewmaHalfLife(ewmaHalfLife) + .consecutive5xx(consecutive5xx) + .failureDetectorInterval(interval) + .baseEjectionTime(baseEjectionTime) + .ejectionTimeJitter(ejectionTimeJitter) + .maxEjectionPercentage(maxEjectionPercentage) + .enforcingConsecutive5xx(enforcingConsecutive5xx) + .enforcingSuccessRate(enforcingSuccessRate) + .successRateMinimumHosts(successRateMinimumHosts) + .successRateRequestVolume(successRateRequestVolume) + .successRateStdevFactor(successRateStdevFactor) + .failurePercentageThreshold(failurePercentageThreshold) + .enforcingFailurePercentage(enforcingFailurePercentage) + .failurePercentageMinimumHosts(failurePercentageMinimumHosts) + .failurePercentageRequestVolume(failurePercentageRequestVolume) + .maxEjectionTime(maxEjectionTime) + .build(); + } + + @Override + public String toString() { + return "ExperimentalOutlierDetectorConfig{" + + "clientsEnabledFor=" + rawClientsEnabledFor + + ", failedConnectionsThreshold=" + failedConnectionsThreshold + + ", lbPolicy=" + lbPolicy + + ", ewmaHalfLife=" + ewmaHalfLife + + ", consecutive5xx=" + consecutive5xx + + ", interval=" + interval + + ", baseEjectionTime=" + baseEjectionTime + + ", ejectionTimeJitter=" + ejectionTimeJitter + + ", maxEjectionPercentage=" + maxEjectionPercentage + + ", enforcingConsecutive5xx=" + enforcingConsecutive5xx + + ", enforcingSuccessRate=" + enforcingSuccessRate + + ", successRateMinimumHosts=" + successRateMinimumHosts + + ", successRateRequestVolume=" + successRateRequestVolume + + ", successRateStdevFactor=" + successRateStdevFactor + + ", failurePercentageThreshold=" + failurePercentageThreshold + + ", enforcingFailurePercentage=" + enforcingFailurePercentage + + ", failurePercentageMinimumHosts=" + failurePercentageMinimumHosts + + ", failurePercentageRequestVolume=" + failurePercentageRequestVolume + + ", maxEjectionTime=" + maxEjectionTime + + '}'; + } + + private String getString(String name, String defaultValue) { + return properties.getProperty(PROPERTY_PREFIX + name, defaultValue); + } + + private long getLong(String name, long defaultValue) { + String propertyValue = properties.getProperty(PROPERTY_PREFIX + name); + if (propertyValue == null) { + return defaultValue; + } + try { + return Long.parseLong(propertyValue.trim()); + } catch (Exception ex) { + LOGGER.warn("Exception parsing property {} with value {} into integral value. Using default.", + name, propertyValue, ex); + return defaultValue; + } + } + + private int getInt(String name, int defaultValue) { + long value = getLong(name, defaultValue); + if (value < Integer.MIN_VALUE || value > Integer.MAX_VALUE) { + throw new IllegalArgumentException("Integer overflow for value " + name + ": " + value); + } + return (int) value; + } +} diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java new file mode 100644 index 0000000000..4d38468cf9 --- /dev/null +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java @@ -0,0 +1,16 @@ +/* + * 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.experimental; 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 ddf44e8edb..80f1fabe98 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 @@ -497,7 +497,7 @@ public Builder serviceDiscoveryResubscribeInterval(Duration interval, Duration j * health checking mechanism. * @return {@code this}. */ - Builder failedConnectionsThreshold(int failedConnectionsThreshold) { + public Builder failedConnectionsThreshold(int failedConnectionsThreshold) { this.failedConnectionsThreshold = failedConnectionsThreshold; if (failedConnectionsThreshold == 0) { throw new IllegalArgumentException("Not valid value: 0"); diff --git a/settings.gradle b/settings.gradle index 9a6d88cf04..91aeecc523 100755 --- a/settings.gradle +++ b/settings.gradle @@ -88,6 +88,7 @@ include "servicetalk-annotations", "servicetalk-http-utils", "servicetalk-loadbalancer", "servicetalk-loadbalancer-experimental", + "servicetalk-loadbalancer-experimental-provider", "servicetalk-log4j2-mdc", "servicetalk-log4j2-mdc-utils", "servicetalk-logging-api", From d33cc14346a11cd3adc21c0aca2f4fa49ae8ff0f Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Tue, 23 Apr 2024 09:45:09 -0600 Subject: [PATCH 2/8] Allow overriding the name extraction and cleanup --- .../README.adoc | 7 +++ .../DefaultHttpLoadBalancerProvider.java | 11 +++- .../DefaultLoadBalancerObserver.java | 4 +- .../DefaultLoadBalancerProviderConfig.java | 56 +++---------------- 4 files changed, 27 insertions(+), 51 deletions(-) diff --git a/servicetalk-loadbalancer-experimental-provider/README.adoc b/servicetalk-loadbalancer-experimental-provider/README.adoc index e4b7803d50..cefe188e42 100644 --- a/servicetalk-loadbalancer-experimental-provider/README.adoc +++ b/servicetalk-loadbalancer-experimental-provider/README.adoc @@ -39,6 +39,13 @@ The specific names will depend on how the client is build. If the client is buil only the host component. If the client is built using some other unresolved address form then the string representation of that is used. +=== Customizing Name Extraction + +The provider depends on the service name for selecting which client to use. If you're using a custom naming system +the default implementation may not be able to decode the unresolved address type to the appropriate name. Custom naming +schemes can be supported by extending the `DefaultHttpLoadBalancerProvider` and overriding the `clientNameFromAddress` +method. Then this custom provider can be added to the service load list as described in the section above. + === All Supported Properties All system properties contain the prefix "io.servicetalk.loadbalancer.experimental.". A comprehensive list of the diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java index ca70f8f097..c369728c51 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java @@ -45,7 +45,7 @@ public DefaultHttpLoadBalancerProvider() { } @Override - public SingleAddressHttpClientBuilder newBuilder(U address, + public final SingleAddressHttpClientBuilder newBuilder(U address, SingleAddressHttpClientBuilder builder) { final String serviceName = clientNameFromAddress(address); if (config.enabledForServiceName(serviceName)) { @@ -71,7 +71,14 @@ private LoadBalancerFactory the unresolved type of the address. + * @param address the address from which to extract the service name. + * @return the String representation of the provided address. + */ + protected String clientNameFromAddress(U address) { String serviceName; if (address instanceof HostAndPort) { serviceName = ((HostAndPort) address).hostName(); diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java index d92accdbd5..ac87160c52 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java @@ -17,8 +17,8 @@ import io.servicetalk.client.api.NoActiveHostException; import io.servicetalk.client.api.ServiceDiscovererEvent; - import io.servicetalk.loadbalancer.LoadBalancerObserver; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,7 +27,7 @@ import static java.util.Objects.requireNonNull; -public class DefaultLoadBalancerObserver implements LoadBalancerObserver { +final class DefaultLoadBalancerObserver implements LoadBalancerObserver { private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLoadBalancerObserver.class); diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java index 2b9fd252d5..c4dfd32e97 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java @@ -16,19 +16,16 @@ package io.servicetalk.loadbalancer.experimental; import io.servicetalk.client.api.LoadBalancedConnection; - import io.servicetalk.loadbalancer.LoadBalancingPolicy; import io.servicetalk.loadbalancer.OutlierDetectorConfig; import io.servicetalk.loadbalancer.P2CLoadBalancingPolicy; import io.servicetalk.loadbalancer.RoundRobinLoadBalancingPolicy; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nullable; import java.time.Duration; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.Properties; import java.util.Set; @@ -99,15 +96,13 @@ private enum LBPolicy { private final Duration maxEjectionTime; private final Duration ejectionTimeJitter; - private final Map fields; - private DefaultLoadBalancerProviderConfig() { this(System.getProperties()); } private DefaultLoadBalancerProviderConfig(Properties properties) { this.properties = requireNonNull(properties, "properties"); - rawClientsEnabledFor = getString(PROP_CLIENTS_ENABLED_FOR, ""); + rawClientsEnabledFor = getString(PROP_CLIENTS_ENABLED_FOR, "").trim(); clientsEnabledFor = getClientsEnabledFor(rawClientsEnabledFor); failedConnectionsThreshold = getInt(PROP_FAILED_CONNECTIONS_THRESHOLD, 5 /*ST default*/); lbPolicy = getLBPolicy(); @@ -127,37 +122,6 @@ private DefaultLoadBalancerProviderConfig(Properties properties) { failurePercentageRequestVolume = getInt(PROP_FAILURE_PERCENTAGE_REQUEST_VOL, 50); maxEjectionTime = ofMillis(getLong(PROP_MAX_EJECTION_TIME_MS, ofSeconds(90).toMillis())); ejectionTimeJitter = ofMillis(getLong(PROP_EJECTION_TIME_JITTER_MS, ZERO.toMillis())); - - fields = populateEventFields(); - } - - private Map populateEventFields() { - Map fields = new HashMap<>(); - fields.put("clientsEnabledFor", String.valueOf(clientsEnabledFor)); - fields.put("failedConnectionsThreshold", String.valueOf(failedConnectionsThreshold)); - fields.put("loadBalancingPolicy", String.valueOf(lbPolicy)); - fields.put("ewmaHalfLifeMillis", String.valueOf(ewmaHalfLife.toMillis())); - fields.put("consecutive5xx", String.valueOf(consecutive5xx)); - fields.put("intervalMillis", String.valueOf(interval.toMillis())); - fields.put("baseEjectionTimeMillis", String.valueOf(baseEjectionTime.toMillis())); - fields.put("ejectionTimeJitterMillis", String.valueOf(ejectionTimeJitter.toMillis())); - fields.put("maxEjectionPercentage", String.valueOf(maxEjectionPercentage)); - fields.put("enforcingConsecutive5xx", String.valueOf(enforcingConsecutive5xx)); - fields.put("enforcingSuccessRate", String.valueOf(enforcingSuccessRate)); - fields.put("successRateMinimumHosts", String.valueOf(successRateMinimumHosts)); - fields.put("successRateRequestVolume", String.valueOf(successRateRequestVolume)); - fields.put("successRateStdevFactor", String.valueOf(successRateStdevFactor)); - fields.put("failurePercentageThreshold", String.valueOf(failurePercentageThreshold)); - fields.put("enforcingFailurePercentage", String.valueOf(enforcingFailurePercentage)); - fields.put("failurePercentageMinimumHosts", String.valueOf(failurePercentageMinimumHosts)); - fields.put("failurePercentageRequestVolume", String.valueOf(failurePercentageRequestVolume)); - fields.put("maxEjectionTimeMillis", String.valueOf(maxEjectionTime.toMillis())); - return fields; - } - - // TODO: what can we do with this? - Map getEventFields() { - return fields; } private LBPolicy getLBPolicy() { @@ -165,16 +129,14 @@ private LBPolicy getLBPolicy() { .equalsIgnoreCase(LBPolicy.P2C.name()) ? LBPolicy.P2C : LBPolicy.RoundRobin; } - @Nullable private Set getClientsEnabledFor(String propertyValue) { - propertyValue = propertyValue.trim(); final Set result = new HashSet<>(); // if enabled for all there is no need to parse. if (!"all".equals(propertyValue)) { for (String serviceName : propertyValue.split(",")) { - serviceName = serviceName.trim(); - if (!serviceName.isEmpty()) { - result.add(serviceName); + String trimmed = serviceName.trim(); + if (!trimmed.isEmpty()) { + result.add(trimmed); } } } @@ -190,7 +152,7 @@ LoadBalancingPolicy getLoadBalancing } boolean enabledForServiceName(String serviceName) { - return clientsEnabledFor == null || clientsEnabledFor.contains(serviceName); + return "all".equals(rawClientsEnabledFor) || clientsEnabledFor.contains(serviceName); } OutlierDetectorConfig outlierDetectorConfig() { @@ -218,7 +180,7 @@ OutlierDetectorConfig outlierDetectorConfig() { @Override public String toString() { return "ExperimentalOutlierDetectorConfig{" + - "clientsEnabledFor=" + rawClientsEnabledFor + + "clientsEnabledFor=" + rawClientsEnabledFor + ", failedConnectionsThreshold=" + failedConnectionsThreshold + ", lbPolicy=" + lbPolicy + ", ewmaHalfLife=" + ewmaHalfLife + @@ -252,8 +214,8 @@ private long getLong(String name, long defaultValue) { try { return Long.parseLong(propertyValue.trim()); } catch (Exception ex) { - LOGGER.warn("Exception parsing property {} with value {} into integral value. Using default.", - name, propertyValue, ex); + LOGGER.warn("Exception parsing property {} with value {} to an integral value. Using the default of {}.", + name, propertyValue, defaultValue, ex); return defaultValue; } } From 0a23ebd7c2ea46717326d35d01773e7d41e811cf Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Tue, 23 Apr 2024 10:05:11 -0600 Subject: [PATCH 3/8] Ignore other LBs being set if we have picked DefaultLoadBalancer --- .../DefaultHttpLoadBalancerProvider.java | 23 ++++++++++++++++++- .../DefaultLoadBalancerObserver.java | 2 +- .../experimental/package-info.java | 3 +++ .../loadbalancer/package-info.java | 3 +++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java index c369728c51..78142b4439 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java @@ -16,6 +16,7 @@ package io.servicetalk.loadbalancer.experimental; import io.servicetalk.client.api.LoadBalancerFactory; +import io.servicetalk.http.api.DelegatingSingleAddressHttpClientBuilder; import io.servicetalk.http.api.FilterableStreamingHttpLoadBalancedConnection; import io.servicetalk.http.api.HttpLoadBalancerFactory; import io.servicetalk.http.api.HttpProviders; @@ -52,7 +53,8 @@ public final SingleAddressHttpClientBuilder newBuilder(U address, try { HttpLoadBalancerFactory loadBalancerFactory = DefaultHttpLoadBalancerFactory.Builder.from( defaultLoadBalancer(serviceName)).build(); - return builder.loadBalancerFactory(loadBalancerFactory); + builder = builder.loadBalancerFactory(loadBalancerFactory); + return new LoadBalancerIgnoringBuilder(builder, serviceName); } catch (Throwable ex) { LOGGER.warn("Failed to enabled DefaultLoadBalancer for client to address {}.", address, ex); } @@ -91,4 +93,23 @@ protected String clientNameFromAddress(U address) { } return serviceName; } + + private static final class LoadBalancerIgnoringBuilder + extends DelegatingSingleAddressHttpClientBuilder { + + private final String serviceName; + + LoadBalancerIgnoringBuilder(final SingleAddressHttpClientBuilder delegate, final String serviceName) { + super(delegate); + this.serviceName = serviceName; + } + + @Override + public SingleAddressHttpClientBuilder loadBalancerFactory( + HttpLoadBalancerFactory loadBalancerFactory) { + LOGGER.info("Ignoring load balancer factory for client to {} which has DefaultLoadBalancer enabled.", + serviceName); + return this; + } + } } diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java index ac87160c52..766c871aaf 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java @@ -33,7 +33,7 @@ final class DefaultLoadBalancerObserver implements LoadBalancerObserver { private final String clientName; - public DefaultLoadBalancerObserver(final String clientName) { + DefaultLoadBalancerObserver(final String clientName) { this.clientName = requireNonNull(clientName, "clientName"); } diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java index 4d38468cf9..dbed0fa5b3 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/package-info.java @@ -13,4 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@ElementsAreNonnullByDefault package io.servicetalk.loadbalancer.experimental; + +import io.servicetalk.annotations.ElementsAreNonnullByDefault; diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/package-info.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/package-info.java index 07dfa84e2d..9370c66ea7 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/package-info.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/package-info.java @@ -13,4 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@ElementsAreNonnullByDefault package io.servicetalk.loadbalancer; + +import io.servicetalk.annotations.ElementsAreNonnullByDefault; From cc73f16f29b7770d509fb09581677f681e4c3683 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Tue, 23 Apr 2024 10:26:05 -0600 Subject: [PATCH 4/8] Remove unused build imports --- .../README.adoc | 3 ++ .../build.gradle | 10 ------ .../gradle/checkstyle/suppressions.xml | 26 -------------- .../gradle/spotbugs/test-exclusions.xml | 35 ------------------- 4 files changed, 3 insertions(+), 71 deletions(-) delete mode 100644 servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml delete mode 100644 servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml diff --git a/servicetalk-loadbalancer-experimental-provider/README.adoc b/servicetalk-loadbalancer-experimental-provider/README.adoc index cefe188e42..8ae34eae5c 100644 --- a/servicetalk-loadbalancer-experimental-provider/README.adoc +++ b/servicetalk-loadbalancer-experimental-provider/README.adoc @@ -3,6 +3,9 @@ This package provides providers for enabling the DefaultLoadBalancer via system properties to allow for easy experimentation that doesn't require a recompilation of the application. +> WARNING: this package is only for experimentation and will be removed in the future. + + == Enabling DefaultLoadBalancer via System Properties === Dynamically Loading the DefaultHttpLoadBalancerProvider diff --git a/servicetalk-loadbalancer-experimental-provider/build.gradle b/servicetalk-loadbalancer-experimental-provider/build.gradle index 38b4c04762..3a35b7d5ee 100644 --- a/servicetalk-loadbalancer-experimental-provider/build.gradle +++ b/servicetalk-loadbalancer-experimental-provider/build.gradle @@ -31,14 +31,4 @@ dependencies { implementation project(":servicetalk-utils-internal") implementation "com.google.code.findbugs:jsr305" implementation "org.slf4j:slf4j-api" - - testImplementation testFixtures(project(":servicetalk-concurrent-api")) - testImplementation testFixtures(project(":servicetalk-concurrent-internal")) - testImplementation project(":servicetalk-concurrent-test-internal") - testImplementation project(":servicetalk-test-resources") - testImplementation "org.junit.jupiter:junit-jupiter-api" - testImplementation "org.junit.jupiter:junit-jupiter-params" - testImplementation "org.apache.logging.log4j:log4j-core" - testImplementation "org.hamcrest:hamcrest:$hamcrestVersion" - testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" } diff --git a/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml b/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml deleted file mode 100644 index 78094076e5..0000000000 --- a/servicetalk-loadbalancer-experimental-provider/gradle/checkstyle/suppressions.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - - - - diff --git a/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml b/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml deleted file mode 100644 index 7e276c24b9..0000000000 --- a/servicetalk-loadbalancer-experimental-provider/gradle/spotbugs/test-exclusions.xml +++ /dev/null @@ -1,35 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - From 0aeb3978b141f8fd4dc11da8dbc70d2f557a83a0 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Tue, 23 Apr 2024 10:36:12 -0600 Subject: [PATCH 5/8] One more cleanup --- servicetalk-loadbalancer-experimental-provider/build.gradle | 2 +- .../experimental/DefaultHttpLoadBalancerProvider.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/servicetalk-loadbalancer-experimental-provider/build.gradle b/servicetalk-loadbalancer-experimental-provider/build.gradle index 3a35b7d5ee..b7fdcf591a 100644 --- a/servicetalk-loadbalancer-experimental-provider/build.gradle +++ b/servicetalk-loadbalancer-experimental-provider/build.gradle @@ -1,5 +1,5 @@ /* - * Copyright © 2018-2019 Apple Inc. and the ServiceTalk project authors + * 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. diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java index 78142b4439..1bb62df98f 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java @@ -30,6 +30,10 @@ import static java.util.Objects.requireNonNull; +/** + * A client builder provider that supports enabling the new `DefaultLoadBalancer` in applications via property flags. + * See the packages README.md for more details. + */ public class DefaultHttpLoadBalancerProvider implements HttpProviders.SingleAddressHttpClientBuilderProvider { private static final Logger LOGGER = LoggerFactory.getLogger(DefaultHttpLoadBalancerProvider.class); From 7edb3ecb1ff35da1a8fdd7112f08b190e262b1c4 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Thu, 25 Apr 2024 10:55:48 -0600 Subject: [PATCH 6/8] First Michael feedback --- .../README.adoc | 4 ++-- .../DefaultHttpLoadBalancerProvider.java | 4 ++-- .../DefaultLoadBalancerProviderConfig.java | 15 +++++++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/servicetalk-loadbalancer-experimental-provider/README.adoc b/servicetalk-loadbalancer-experimental-provider/README.adoc index 8ae34eae5c..33729ac2f3 100644 --- a/servicetalk-loadbalancer-experimental-provider/README.adoc +++ b/servicetalk-loadbalancer-experimental-provider/README.adoc @@ -16,7 +16,7 @@ part of your application bundle and also include a file in the resources as foll resources/META-INF/services/io.servicetalk.http.api.HttpProviders$SingleAddressHttpClientBuilderProvider ``` -The contents of this must should contain the line +The contents of this must contain the line ``` io.servicetalk.loadbalancer.experimental.DefaultHttpLoadBalancerProvider @@ -38,7 +38,7 @@ system property to a comma separated list: io.servicetalk.loadbalancer.experimental.clientsEnabledFor=service1,service2 ``` -The specific names will depend on how the client is build. If the client is built using a `HostAndPort`, the names are +The specific names will depend on how the client is built. If the client is built using a `HostAndPort`, the names are only the host component. If the client is built using some other unresolved address form then the string representation of that is used. diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java index 1bb62df98f..b88d6e9517 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultHttpLoadBalancerProvider.java @@ -111,8 +111,8 @@ private static final class LoadBalancerIgnoringBuilder @Override public SingleAddressHttpClientBuilder loadBalancerFactory( HttpLoadBalancerFactory loadBalancerFactory) { - LOGGER.info("Ignoring load balancer factory for client to {} which has DefaultLoadBalancer enabled.", - serviceName); + LOGGER.info("Ignoring http load balancer factory of type {} for client to {} which has " + + "DefaultLoadBalancer enabled.", loadBalancerFactory.getClass(), serviceName); return this; } } diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java index c4dfd32e97..e800630a11 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java @@ -125,14 +125,21 @@ private DefaultLoadBalancerProviderConfig(Properties properties) { } private LBPolicy getLBPolicy() { - return getString(PROP_LOAD_BALANCING_POLICY, "p2c") - .equalsIgnoreCase(LBPolicy.P2C.name()) ? LBPolicy.P2C : LBPolicy.RoundRobin; + final String configuredLbName = getString(PROP_LOAD_BALANCING_POLICY, LBPolicy.P2C.name()); + if (configuredLbName.equalsIgnoreCase(LBPolicy.P2C.name())) { + return LBPolicy.P2C; + } else if (configuredLbName.equalsIgnoreCase(LBPolicy.RoundRobin.name())) { + return LBPolicy.RoundRobin; + } else { + LOGGER.warn("Unrecognized load balancer policy name: {}. Defaulting to P2C.", configuredLbName); + return LBPolicy.P2C; + } } private Set getClientsEnabledFor(String propertyValue) { final Set result = new HashSet<>(); // if enabled for all there is no need to parse. - if (!"all".equals(propertyValue)) { + if (!"*".equals(propertyValue)) { for (String serviceName : propertyValue.split(",")) { String trimmed = serviceName.trim(); if (!trimmed.isEmpty()) { @@ -152,7 +159,7 @@ LoadBalancingPolicy getLoadBalancing } boolean enabledForServiceName(String serviceName) { - return "all".equals(rawClientsEnabledFor) || clientsEnabledFor.contains(serviceName); + return "*".equals(rawClientsEnabledFor) || clientsEnabledFor.contains(serviceName); } OutlierDetectorConfig outlierDetectorConfig() { From 3e52a3015256e084067986c4b2f378b0e9e983af Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Thu, 25 Apr 2024 11:06:39 -0600 Subject: [PATCH 7/8] drop 'experimentalLoadBalancer' from property names --- .../DefaultLoadBalancerProviderConfig.java | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java index e800630a11..206b874c43 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerProviderConfig.java @@ -49,27 +49,23 @@ private enum LBPolicy { private static final String PROP_CLIENTS_ENABLED_FOR = "clientsEnabledFor"; private static final String PROP_FAILED_CONNECTIONS_THRESHOLD = "healthCheckFailedConnectionsThreshold"; - private static final String PROP_LOAD_BALANCING_POLICY = "clientExperimentalLoadBalancerPolicy"; - private static final String PROP_EWMA_HALF_LIFE_MS = "experimentalLoadBalancerEwmaHalfLifeMs"; - private static final String PROP_CONSECUTIVE_5XX = "experimentalLoadBalancerConsecutive5xx"; - private static final String PROP_INTERVAL_MS = "experimentalLoadBalancerIntervalMs"; - private static final String PROP_BASE_EJECTION_TIME_MS = "experimentalLoadBalancerBaseEjectionTimeMs"; - private static final String PROP_MAX_EJECTION_PERCENT = "experimentalLoadBalancerMaxEjectionPercent"; - private static final String PROP_ENFORCING_CONSECUTIVE_5XX = "experimentalLoadBalancerEnforcingConsecutive5xx"; - private static final String PROP_ENFORCING_SUCCESS_RATE = "experimentalLoadBalancerEnforcingSuccessRate"; - private static final String PROP_SUCCESS_RATE_MIN_HOSTS = "experimentalLoadBalancerSuccessRateMinimumHosts"; - private static final String PROP_SUCCESS_RATE_REQUEST_VOL = "experimentalLoadBalancerSuccessRateRequestVolume"; - private static final String PROP_SUCCESS_RATE_STDEV_FACTOR = "experimentalLoadBalancerSuccessRateStdevFactor"; - private static final String PROP_FAILURE_PERCENTAGE_THRESHOLD = - "experimentalLoadBalancerFailurePercentageThreshold"; - private static final String PROP_ENFORCING_FAILURE_PERCENTAGE = - "experimentalLoadBalancerEnforcingFailurePercentage"; - private static final String PROP_FAILURE_PERCENTAGE_MIN_HOSTS = - "experimentalLoadBalancerFailurePercentageMinimumHosts"; - private static final String PROP_FAILURE_PERCENTAGE_REQUEST_VOL = - "experimentalLoadBalancerFailurePercentageRequestVolume"; - private static final String PROP_MAX_EJECTION_TIME_MS = "experimentalLoadBalancerMaxEjectionTimeMs"; - private static final String PROP_EJECTION_TIME_JITTER_MS = "experimentalLoadBalancerEjectionTimeJitterMs"; + private static final String PROP_LOAD_BALANCING_POLICY = "policy"; + private static final String PROP_EWMA_HALF_LIFE_MS = "ewmaHalfLifeMs"; + private static final String PROP_CONSECUTIVE_5XX = "consecutive5xx"; + private static final String PROP_INTERVAL_MS = "intervalMs"; + private static final String PROP_BASE_EJECTION_TIME_MS = "baseEjectionTimeMs"; + private static final String PROP_MAX_EJECTION_PERCENT = "maxEjectionPercent"; + private static final String PROP_ENFORCING_CONSECUTIVE_5XX = "enforcingConsecutive5xx"; + private static final String PROP_ENFORCING_SUCCESS_RATE = "enforcingSuccessRate"; + private static final String PROP_SUCCESS_RATE_MIN_HOSTS = "successRateMinimumHosts"; + private static final String PROP_SUCCESS_RATE_REQUEST_VOL = "successRateRequestVolume"; + private static final String PROP_SUCCESS_RATE_STDEV_FACTOR = "successRateStdevFactor"; + private static final String PROP_FAILURE_PERCENTAGE_THRESHOLD = "failurePercentageThreshold"; + private static final String PROP_ENFORCING_FAILURE_PERCENTAGE = "enforcingFailurePercentage"; + private static final String PROP_FAILURE_PERCENTAGE_MIN_HOSTS = "failurePercentageMinimumHosts"; + private static final String PROP_FAILURE_PERCENTAGE_REQUEST_VOL = "failurePercentageRequestVolume"; + private static final String PROP_MAX_EJECTION_TIME_MS = "maxEjectionTimeMs"; + private static final String PROP_EJECTION_TIME_JITTER_MS = "ejectionTimeJitterMs"; static final DefaultLoadBalancerProviderConfig INSTANCE = new DefaultLoadBalancerProviderConfig(); From 4cae1a434c22ff39eb3b7d5e85431549778a1319 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Fri, 26 Apr 2024 11:05:17 -0600 Subject: [PATCH 8/8] More Michael feedback --- .../experimental/DefaultLoadBalancerObserver.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java index 766c871aaf..911d881a64 100644 --- a/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java +++ b/servicetalk-loadbalancer-experimental-provider/src/main/java/io/servicetalk/loadbalancer/experimental/DefaultLoadBalancerObserver.java @@ -44,14 +44,14 @@ public HostObserver hostObserver(Object resolvedAddress) { @Override public void onNoHostsAvailable() { - LOGGER.debug("{}- No hosts available", clientName); + LOGGER.debug("{}- onNoHostsAvailable()", clientName); } @Override public void onServiceDiscoveryEvent(Collection> events, int oldHostSetSize, int newHostSetSize) { - LOGGER.debug("{}- service discovery events. Old host set size: {}, new host set size: {}, events: {}", - clientName, oldHostSetSize, newHostSetSize, events); + LOGGER.debug("{}- onServiceDiscoveryEvent(events: {}, oldHostSetSize: {}, newHostSetSize: {})", + clientName, events, oldHostSetSize, newHostSetSize); } @Override @@ -93,7 +93,7 @@ public void onExpiredHostRemoved(int connectionCount) { @Override public void onHostMarkedUnhealthy(@Nullable Throwable cause) { - LOGGER.debug("{}:{}- onExpiredHostRemoved(ex)", clientName, resolvedAddress, cause); + LOGGER.debug("{}:{}- onHostMarkedUnhealthy(ex)", clientName, resolvedAddress, cause); } @Override