From bf3d1b9777e8810c44a96f08304fffb98eacf7a6 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Fri, 12 Jul 2024 10:20:07 -0600 Subject: [PATCH 1/4] loadbalancer-experimental: add LB observer method for when the host set changes Motivation: It can be valuable to know what the current host set is but we don't have a way to directly observer that right now. Modifications: Add a method to the LoadBalancerObserver that can capture when the host set changes and use it in DefaultLoadBalancer. --- .../DefaultLoadBalancerObserver.java | 13 ++++++ .../loadbalancer/DefaultLoadBalancer.java | 6 ++- .../loadbalancer/LoadBalancerObserver.java | 40 +++++++++++++++++-- .../NoopLoadBalancerObserver.java | 5 +++ 4 files changed, 59 insertions(+), 5 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 e68f110140..f96f25eef4 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 @@ -59,6 +59,19 @@ public void onNoActiveHostsAvailable(int hostSetSize, NoActiveHostException exce LOGGER.debug("{}- No active hosts available. Host set size: {}.", lbDescription, hostSetSize, exception); } + @Override + public void onHostSetChanged(Collection newHosts) { + if (LOGGER.isDebugEnabled()) { + int healthyCount = 0; + for (Host host : newHosts) { + if (host.isHealthy()) { + healthyCount++; + } + } + LOGGER.debug("{}- onHostSetChanged(total: {}, healthy: {})", lbDescription, newHosts.size(), healthyCount); + } + } + private final class HostObserverImpl implements HostObserver { private final Object resolvedAddress; diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java index df8d48aa69..dcadb54f4e 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java @@ -508,7 +508,9 @@ private void sequentialUpdateUsedHosts(List host : nextHosts) { host.loadBalancingWeight(host.serviceDiscoveryWeight()); } - this.hostSelector = hostSelector.rebuildWithHosts(priorityStrategy.prioritize(usedHosts)); + nextHosts = priorityStrategy.prioritize(nextHosts); + this.hostSelector = hostSelector.rebuildWithHosts(nextHosts); + loadBalancerObserver.onHostSetChanged(nextHosts); } @Override @@ -649,7 +651,7 @@ List> hosts() { } static final class PrioritizedHostImpl - implements Host, PrioritizedHost { + implements Host, PrioritizedHost, LoadBalancerObserver.Host { private final Host delegate; private int priority; private double serviceDiscoveryWeight; diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerObserver.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerObserver.java index e4b5c5c1fc..8e1b363a4b 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerObserver.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/LoadBalancerObserver.java @@ -55,7 +55,17 @@ void onServiceDiscoveryEvent(Collection> eve void onNoActiveHostsAvailable(int hostSetSize, NoActiveHostException exception); /** - * An observer for {@link Host} events. + * Callback for when the set of hosts used by the load balancer has changed. This set may not + * exactly reflect the state of the service discovery system due to filtering of zero-weight + * hosts and forms of sub-setting and thus may only represent the hosts that the selection + * algorithm may use. + * @param newHosts the new set of hosts used by the selection algorithm. + */ + default void onHostSetChanged(Collection newHosts) { + } + + /** + * An observer for {@link io.servicetalk.loadbalancer.Host} events. */ interface HostObserver { @@ -84,14 +94,38 @@ interface HostObserver { void onExpiredHostRemoved(int connectionCount); /** - * Callback for when a {@link Host} transitions from healthy to unhealthy. + * Callback for when a {@link io.servicetalk.loadbalancer.Host} transitions from healthy to unhealthy. * @param cause the most recent cause of the transition. */ void onHostMarkedUnhealthy(@Nullable Throwable cause); /** - * Callback for when a {@link Host} transitions from unhealthy to healthy. + * Callback for when a {@link io.servicetalk.loadbalancer.Host} transitions from unhealthy to healthy. */ void onHostRevived(); } + + /** + * A description of a host. + */ + interface Host { + /** + * The address of the host. + * @return the address of the host. + */ + Object address(); + + /** + * Determine the health status of this host. + * @return whether the host considers itself healthy enough to serve traffic. This is best effort and does not + * guarantee that the request will succeed. + */ + boolean isHealthy(); + + /** + * The weight of the host, relative to the weights of associated hosts as used for load balancing. + * @return the relative weight of the host. + */ + double loadBalancingWeight(); + } } diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/NoopLoadBalancerObserver.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/NoopLoadBalancerObserver.java index 29693dd45b..234b7bbbcb 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/NoopLoadBalancerObserver.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/NoopLoadBalancerObserver.java @@ -51,6 +51,11 @@ public void onServiceDiscoveryEvent(Collection newHosts) { + // noop + } + private static final class NoopHostObserver implements LoadBalancerObserver.HostObserver { private static final HostObserver INSTANCE = new NoopHostObserver(); From 8659949c90f96f42af2f37ad40bd37b4976a3aa9 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Fri, 12 Jul 2024 15:47:39 -0600 Subject: [PATCH 2/4] Some feedback --- .../loadbalancer/experimental/DefaultLoadBalancerObserver.java | 3 ++- .../java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java | 2 +- 2 files changed, 3 insertions(+), 2 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 f96f25eef4..41684254df 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 @@ -68,7 +68,8 @@ public void onHostSetChanged(Collection newHosts) { healthyCount++; } } - LOGGER.debug("{}- onHostSetChanged(total: {}, healthy: {})", lbDescription, newHosts.size(), healthyCount); + LOGGER.debug("{}- onHostSetChanged(newHosts: {}). Size: {}, healthy: ", lbDescription, newHosts, + newHosts.size(), healthyCount); } } diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java index dcadb54f4e..6b15239b58 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java @@ -510,7 +510,7 @@ private void sequentialUpdateUsedHosts(List Date: Mon, 15 Jul 2024 09:50:23 -0600 Subject: [PATCH 3/4] Fix pmd --- .../loadbalancer/experimental/DefaultLoadBalancerObserver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 41684254df..fc0c565322 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 @@ -68,7 +68,7 @@ public void onHostSetChanged(Collection newHosts) { healthyCount++; } } - LOGGER.debug("{}- onHostSetChanged(newHosts: {}). Size: {}, healthy: ", lbDescription, newHosts, + LOGGER.debug("{}- onHostSetChanged(newHosts: {}). Size: {}, healthy: {}", lbDescription, newHosts, newHosts.size(), healthyCount); } } From b8efad102a7ca02db082d0d4723a6b46cd8b6ba3 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Mon, 15 Jul 2024 12:58:29 -0600 Subject: [PATCH 4/4] Some feedback --- .../experimental/DefaultLoadBalancerObserver.java | 4 ++-- .../java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java | 2 +- 2 files changed, 3 insertions(+), 3 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 fc0c565322..2c9c565f7e 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 @@ -68,8 +68,8 @@ public void onHostSetChanged(Collection newHosts) { healthyCount++; } } - LOGGER.debug("{}- onHostSetChanged(newHosts: {}). Size: {}, healthy: {}", lbDescription, newHosts, - newHosts.size(), healthyCount); + LOGGER.debug("{}- onHostSetChanged(host set size: {}, healthy: {}). New hosts: {}", lbDescription, + newHosts.size(), healthyCount, newHosts); } } diff --git a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java index 6b15239b58..b95c0b57b9 100644 --- a/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java +++ b/servicetalk-loadbalancer-experimental/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java @@ -510,7 +510,7 @@ private void sequentialUpdateUsedHosts(List