From 01389774d55b4784deda71b22135e0ec3e5ceb9b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 25 Jul 2024 18:21:26 -0700 Subject: [PATCH] util: Remove child policy config from MultiChildLB state The child policy config should be refreshed every address update, so it shouldn't be stored in the ChildLbState. In addition, none of the current usages actually used what was stored in the ChildLbState in a meaningful way (it was always null). ResolvedAddresses was also removed from createChildLbState(), as nothing in it should be needed for creation; it varies over time and the values passed at creation are immutable. --- .../io/grpc/util/MultiChildLoadBalancer.java | 25 ++++++------------- .../io/grpc/util/RoundRobinLoadBalancer.java | 5 ++-- .../grpc/xds/ClusterManagerLoadBalancer.java | 10 +++----- .../io/grpc/xds/LeastRequestLoadBalancer.java | 10 +++----- .../io/grpc/xds/RingHashLoadBalancer.java | 5 ++-- .../xds/WeightedRoundRobinLoadBalancer.java | 11 +++----- 6 files changed, 23 insertions(+), 43 deletions(-) diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 5dfb81ee4b..4e89e53741 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -90,7 +90,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { if (existingChildLbState != null) { childLbMap.put(endpoint, existingChildLbState); } else { - childLbMap.put(endpoint, createChildLbState(endpoint, null, resolvedAddresses)); + childLbMap.put(endpoint, createChildLbState(endpoint)); } } return childLbMap; @@ -99,9 +99,8 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { /** * Override to create an instance of a subclass. */ - protected ChildLbState createChildLbState(Object key, Object policyConfig, - ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, pickFirstLbProvider, policyConfig); + protected ChildLbState createChildLbState(Object key) { + return new ChildLbState(key, pickFirstLbProvider); } /** @@ -133,11 +132,9 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { * Override this if your keys are not of type Endpoint. * @param key Key to identify the ChildLbState * @param resolvedAddresses list of addresses which include attributes - * @param childConfig a load balancing policy config. This field is optional. * @return a fully loaded ResolvedAddresses object for the specified key */ - protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses, - Object childConfig) { + protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) { Endpoint endpointKey; if (key instanceof EquivalentAddressGroup) { endpointKey = new Endpoint((EquivalentAddressGroup) key); @@ -160,7 +157,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { return resolvedAddresses.toBuilder() .setAddresses(Collections.singletonList(eagToUse)) .setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build()) - .setLoadBalancingPolicyConfig(childConfig) + .setLoadBalancingPolicyConfig(null) .build(); } @@ -226,10 +223,8 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { private void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses, Map newChildren) { for (Map.Entry entry : newChildren.entrySet()) { - Object childConfig = entry.getValue().getConfig(); ChildLbState childLbState = childLbStates.get(entry.getKey()); - ResolvedAddresses childAddresses = - getChildAddresses(entry.getKey(), resolvedAddresses, childConfig); + ResolvedAddresses childAddresses = getChildAddresses(entry.getKey(), resolvedAddresses); childLbState.setResolvedAddresses(childAddresses); // update child childLbState.lb.handleResolvedAddresses(childAddresses); // update child LB } @@ -328,15 +323,13 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { public class ChildLbState { private final Object key; private ResolvedAddresses resolvedAddresses; - private final Object config; private final LoadBalancer lb; private ConnectivityState currentState; private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult()); - public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig) { + public ChildLbState(Object key, LoadBalancer.Factory policyFactory) { this.key = key; - this.config = childConfig; this.lb = policyFactory.newLoadBalancer(createChildHelper()); this.currentState = CONNECTING; } @@ -400,10 +393,6 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { resolvedAddresses = newAddresses; } - private Object getConfig() { - return config; - } - @VisibleForTesting public final ResolvedAddresses getResolvedAddresses() { return resolvedAddresses; diff --git a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java index 0804978cf7..22940e875a 100644 --- a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java @@ -96,9 +96,8 @@ final class RoundRobinLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ChildLbState createChildLbState(Object key, Object policyConfig, - ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, pickFirstLbProvider, policyConfig) { + protected ChildLbState createChildLbState(Object key) { + return new ChildLbState(key, pickFirstLbProvider) { @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 6b155545b1..44fa5313a0 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java @@ -70,8 +70,7 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses, - Object unusedChildConfig) { + protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) { ClusterManagerConfig config = (ClusterManagerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); Object childConfig = config.childPolicies.get(key); @@ -87,7 +86,7 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { for (String key : config.childPolicies.keySet()) { ChildLbState child = getChildLbState(key); if (child == null) { - child = new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE, null); + child = new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE); } newChildPolicies.put(key, child); } @@ -204,9 +203,8 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { @Nullable ScheduledHandle deletionTimer; - public ClusterManagerLbState(Object key, LoadBalancer.Factory policyFactory, - Object childConfig) { - super(key, policyFactory, childConfig); + public ClusterManagerLbState(Object key, LoadBalancer.Factory policyFactory) { + super(key, policyFactory); } @Override diff --git a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java index 52fa1298e6..6c13530ff4 100644 --- a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java @@ -126,9 +126,8 @@ final class LeastRequestLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ChildLbState createChildLbState(Object key, Object policyConfig, - ResolvedAddresses unused) { - return new LeastRequestLbState(key, pickFirstLbProvider, policyConfig); + protected ChildLbState createChildLbState(Object key) { + return new LeastRequestLbState(key, pickFirstLbProvider); } private void updateBalancingState(ConnectivityState state, SubchannelPicker picker) { @@ -320,9 +319,8 @@ final class LeastRequestLoadBalancer extends MultiChildLoadBalancer { protected class LeastRequestLbState extends ChildLbState { private final AtomicInteger activeRequests = new AtomicInteger(0); - public LeastRequestLbState(Object key, LoadBalancerProvider policyProvider, - Object childConfig) { - super(key, policyProvider, childConfig); + public LeastRequestLbState(Object key, LoadBalancerProvider policyProvider) { + super(key, policyProvider); } 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 74ea9dbb11..4f93974b52 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -219,9 +219,8 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ChildLbState createChildLbState(Object key, Object policyConfig, - ResolvedAddresses resolvedAddresses) { - return new ChildLbState(key, lazyLbFactory, null); + protected ChildLbState createChildLbState(Object key) { + return new ChildLbState(key, lazyLbFactory); } 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 65cd146fdd..73764c63c8 100644 --- a/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java @@ -148,10 +148,8 @@ final class WeightedRoundRobinLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ChildLbState createChildLbState(Object key, Object policyConfig, - ResolvedAddresses unused) { - ChildLbState childLbState = new WeightedChildLbState(key, pickFirstLbProvider, policyConfig); - return childLbState; + protected ChildLbState createChildLbState(Object key) { + return new WeightedChildLbState(key, pickFirstLbProvider); } @Override @@ -289,9 +287,8 @@ final class WeightedRoundRobinLoadBalancer extends MultiChildLoadBalancer { private OrcaReportListener orcaReportListener; - public WeightedChildLbState( - Object key, LoadBalancerProvider policyProvider, Object childConfig) { - super(key, policyProvider, childConfig); + public WeightedChildLbState(Object key, LoadBalancerProvider policyProvider) { + super(key, policyProvider); } @Override