From b5989a54014b9066defb26d4cca79432a5dba1be Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 25 Jul 2024 15:46:02 -0700 Subject: [PATCH] util: MultiChildLb children should always start with a NoResult picker That's the obvious default, and all current usages use (something equivalent to) that default. --- .../io/grpc/util/MultiChildLoadBalancer.java | 22 +++++-------------- .../io/grpc/util/RoundRobinLoadBalancer.java | 4 ++-- .../grpc/xds/ClusterManagerLoadBalancer.java | 6 ++--- .../io/grpc/xds/LeastRequestLoadBalancer.java | 8 +++---- .../io/grpc/xds/RingHashLoadBalancer.java | 4 ++-- .../xds/WeightedRoundRobinLoadBalancer.java | 11 +++++----- 6 files changed, 21 insertions(+), 34 deletions(-) diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 951721adce..51144b7c01 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -91,8 +91,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { if (existingChildLbState != null) { childLbMap.put(endpoint, existingChildLbState); } else { - childLbMap.put(endpoint, - createChildLbState(endpoint, null, getInitialPicker(), resolvedAddresses)); + childLbMap.put(endpoint, createChildLbState(endpoint, null, resolvedAddresses)); } } return childLbMap; @@ -102,8 +101,8 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { * Override to create an instance of a subclass. */ protected ChildLbState createChildLbState(Object key, Object policyConfig, - SubchannelPicker initialPicker, ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, pickFirstLbProvider, policyConfig, initialPicker); + ResolvedAddresses resolvedAddresses) { + return new ChildLbState(key, pickFirstLbProvider, policyConfig); } /** @@ -187,15 +186,6 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { child.lb.handleNameResolutionError(error); } - /** - * Creates a picker representing the state before any connections have been established. - * - *

Override to produce a custom picker. - */ - protected SubchannelPicker getInitialPicker() { - return new FixedResultPicker(PickResult.withNoResult()); - } - /** * Creates a new picker representing an error status. * @@ -365,12 +355,10 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { private final LoadBalancer lb; private ConnectivityState currentState; - private SubchannelPicker currentPicker; + private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult()); - public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig, - SubchannelPicker initialPicker) { + public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig) { this.key = key; - this.currentPicker = initialPicker; this.config = childConfig; this.lb = policyFactory.newLoadBalancer(createChildHelper()); this.currentState = CONNECTING; diff --git a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java index 765e2a4d4b..a8d829cfb8 100644 --- a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java @@ -97,8 +97,8 @@ final class RoundRobinLoadBalancer extends MultiChildLoadBalancer { @Override protected ChildLbState createChildLbState(Object key, Object policyConfig, - SubchannelPicker initialPicker, ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, pickFirstLbProvider, policyConfig, initialPicker) { + ResolvedAddresses resolvedAddresses) { + return new ChildLbState(key, pickFirstLbProvider, policyConfig) { @Override protected ChildLbStateHelper createChildHelper() { return new ChildLbStateHelper() { diff --git a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java index 9e9ca5e1da..50669cfeeb 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java @@ -85,7 +85,7 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { ChildLbState child = getChildLbState(entry.getKey()); if (child == null) { child = new ClusterManagerLbState(entry.getKey(), - entry.getValue().getProvider(), entry.getValue().getConfig(), getInitialPicker()); + entry.getValue().getProvider(), entry.getValue().getConfig()); } newChildPolicies.put(entry.getKey(), child); } @@ -202,8 +202,8 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { ScheduledHandle deletionTimer; public ClusterManagerLbState(Object key, LoadBalancerProvider policyProvider, - Object childConfig, SubchannelPicker initialPicker) { - super(key, policyProvider, childConfig, initialPicker); + Object childConfig) { + super(key, policyProvider, childConfig); } @Override diff --git a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java index a11622d492..52fa1298e6 100644 --- a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java @@ -127,8 +127,8 @@ final class LeastRequestLoadBalancer extends MultiChildLoadBalancer { @Override protected ChildLbState createChildLbState(Object key, Object policyConfig, - SubchannelPicker initialPicker, ResolvedAddresses unused) { - return new LeastRequestLbState(key, pickFirstLbProvider, policyConfig, initialPicker); + ResolvedAddresses unused) { + return new LeastRequestLbState(key, pickFirstLbProvider, policyConfig); } private void updateBalancingState(ConnectivityState state, SubchannelPicker picker) { @@ -321,8 +321,8 @@ final class LeastRequestLoadBalancer extends MultiChildLoadBalancer { private final AtomicInteger activeRequests = new AtomicInteger(0); public LeastRequestLbState(Object key, LoadBalancerProvider policyProvider, - Object childConfig, SubchannelPicker initialPicker) { - super(key, policyProvider, childConfig, initialPicker); + Object childConfig) { + super(key, policyProvider, childConfig); } int getActiveRequests() { diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java index 72618b7bba..4e380dcd3d 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -228,8 +228,8 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer { @Override protected ChildLbState createChildLbState(Object key, Object policyConfig, - SubchannelPicker initialPicker, ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, lazyLbFactory, null, EMPTY_PICKER); + ResolvedAddresses resolvedAddresses) { + return new ChildLbState(key, lazyLbFactory, null); } private Status validateAddrList(List addrList) { diff --git a/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java b/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java index 388eca2579..65cd146fdd 100644 --- a/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java @@ -149,9 +149,8 @@ final class WeightedRoundRobinLoadBalancer extends MultiChildLoadBalancer { @Override protected ChildLbState createChildLbState(Object key, Object policyConfig, - SubchannelPicker initialPicker, ResolvedAddresses unused) { - ChildLbState childLbState = new WeightedChildLbState(key, pickFirstLbProvider, policyConfig, - initialPicker); + ResolvedAddresses unused) { + ChildLbState childLbState = new WeightedChildLbState(key, pickFirstLbProvider, policyConfig); return childLbState; } @@ -290,9 +289,9 @@ final class WeightedRoundRobinLoadBalancer extends MultiChildLoadBalancer { private OrcaReportListener orcaReportListener; - public WeightedChildLbState(Object key, LoadBalancerProvider policyProvider, Object childConfig, - SubchannelPicker initialPicker) { - super(key, policyProvider, childConfig, initialPicker); + public WeightedChildLbState( + Object key, LoadBalancerProvider policyProvider, Object childConfig) { + super(key, policyProvider, childConfig); } @Override