From cfecc4754b603e950f6f65ee1a2deb697c3f9a61 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 26 Jul 2024 09:19:14 -0700 Subject: [PATCH] Focus MultiChildLB updates around ResolvedAddresses of children This makes ClusterManagerLB more straight-forward, focusing on just the things that are relevant to it, and it avoids specialized map key handling in updateChildrenWithResolvedAddresses(). --- .../io/grpc/util/MultiChildLoadBalancer.java | 94 +++++-------------- .../grpc/xds/ClusterManagerLoadBalancer.java | 27 +++--- 2 files changed, 37 insertions(+), 84 deletions(-) diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 5c37b4a34e..626c2e1104 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -16,7 +16,6 @@ package io.grpc.util; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.ConnectivityState.CONNECTING; import static io.grpc.ConnectivityState.IDLE; @@ -80,20 +79,20 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { /** * Override to utilize parsing of the policy configuration or alternative helper/lb generation. + * Override this if keys are not Endpoints or if child policies have configuration. */ - protected Map createChildLbMap(ResolvedAddresses resolvedAddresses) { - Map childLbMap = new HashMap<>(); - List addresses = resolvedAddresses.getAddresses(); - for (EquivalentAddressGroup eag : addresses) { - Endpoint endpoint = new Endpoint(eag); // keys need to be just addresses - ChildLbState existingChildLbState = childLbStates.get(endpoint); - if (existingChildLbState != null) { - childLbMap.put(endpoint, existingChildLbState); - } else { - childLbMap.put(endpoint, createChildLbState(endpoint)); - } + protected Map createChildAddressesMap( + ResolvedAddresses resolvedAddresses) { + Map childAddresses = new HashMap<>(); + for (EquivalentAddressGroup eag : resolvedAddresses.getAddresses()) { + ResolvedAddresses addresses = resolvedAddresses.toBuilder() + .setAddresses(Collections.singletonList(eag)) + .setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build()) + .setLoadBalancingPolicyConfig(null) + .build(); + childAddresses.put(new Endpoint(eag), addresses); } - return childLbMap; + return childAddresses; } /** @@ -128,39 +127,6 @@ 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 - * @return a fully loaded ResolvedAddresses object for the specified key - */ - protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) { - Endpoint endpointKey; - if (key instanceof EquivalentAddressGroup) { - endpointKey = new Endpoint((EquivalentAddressGroup) key); - } else { - checkArgument(key instanceof Endpoint, "key is wrong type"); - endpointKey = (Endpoint) key; - } - - // Retrieve the non-stripped version - EquivalentAddressGroup eagToUse = null; - for (EquivalentAddressGroup currEag : resolvedAddresses.getAddresses()) { - if (endpointKey.equals(new Endpoint(currEag))) { - eagToUse = currEag; - break; - } - } - - checkNotNull(eagToUse, key + " no longer present in load balancer children"); - - return resolvedAddresses.toBuilder() - .setAddresses(Collections.singletonList(eagToUse)) - .setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build()) - .setLoadBalancingPolicyConfig(null) - .build(); - } - /** * Handle the name resolution error. * @@ -192,41 +158,31 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer { ResolvedAddresses resolvedAddresses) { logger.log(Level.FINE, "Received resolution result: {0}", resolvedAddresses); - // Subclass handles any special manipulation to create appropriate types of keyed ChildLbStates - Map newChildren = createChildLbMap(resolvedAddresses); + Map newChildAddresses = createChildAddressesMap(resolvedAddresses); // Handle error case - if (newChildren.isEmpty()) { + if (newChildAddresses.isEmpty()) { Status unavailableStatus = Status.UNAVAILABLE.withDescription( "NameResolver returned no usable address. " + resolvedAddresses); handleNameResolutionError(unavailableStatus); return new AcceptResolvedAddrRetVal(unavailableStatus, null); } - addMissingChildren(newChildren); + updateChildrenWithResolvedAddresses(newChildAddresses); - updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren); - - return new AcceptResolvedAddrRetVal(Status.OK, getRemovedChildren(newChildren.keySet())); + return new AcceptResolvedAddrRetVal(Status.OK, getRemovedChildren(newChildAddresses.keySet())); } - private void addMissingChildren(Map newChildren) { - // Do adds and identify reused children - for (Map.Entry entry : newChildren.entrySet()) { - final Object key = entry.getKey(); - if (!childLbStates.containsKey(key)) { - childLbStates.put(key, entry.getValue()); - } - } - } - - private void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses, - Map newChildren) { - for (Map.Entry entry : newChildren.entrySet()) { + private void updateChildrenWithResolvedAddresses( + Map newChildAddresses) { + for (Map.Entry entry : newChildAddresses.entrySet()) { ChildLbState childLbState = childLbStates.get(entry.getKey()); - ResolvedAddresses childAddresses = getChildAddresses(entry.getKey(), resolvedAddresses); - childLbState.setResolvedAddresses(childAddresses); // update child - childLbState.lb.handleResolvedAddresses(childAddresses); // update child LB + if (childLbState == null) { + childLbState = createChildLbState(entry.getKey()); + childLbStates.put(entry.getKey(), childLbState); + } + childLbState.setResolvedAddresses(entry.getValue()); // update child + childLbState.lb.handleResolvedAddresses(entry.getValue()); // update child LB } } diff --git a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java index 5aafd8240a..c175b847c6 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java @@ -70,31 +70,28 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer { } @Override - protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) { - ClusterManagerConfig config = (ClusterManagerConfig) - resolvedAddresses.getLoadBalancingPolicyConfig(); - Object childConfig = config.childPolicies.get(key); - return resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(childConfig).build(); + protected ChildLbState createChildLbState(Object key) { + return new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE); } @Override - protected Map createChildLbMap(ResolvedAddresses resolvedAddresses) { + protected Map createChildAddressesMap( + ResolvedAddresses resolvedAddresses) { ClusterManagerConfig config = (ClusterManagerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); - Map newChildPolicies = new HashMap<>(); + Map childAddresses = new HashMap<>(); if (config != null) { - for (String key : config.childPolicies.keySet()) { - ChildLbState child = getChildLbState(key); - if (child == null) { - child = new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE); - } - newChildPolicies.put(key, child); + for (Map.Entry childPolicy : config.childPolicies.entrySet()) { + ResolvedAddresses addresses = resolvedAddresses.toBuilder() + .setLoadBalancingPolicyConfig(childPolicy.getValue()) + .build(); + childAddresses.put(childPolicy.getKey(), addresses); } } logger.log( XdsLogLevel.INFO, - "Received cluster_manager lb config: child names={0}", newChildPolicies.keySet()); - return newChildPolicies; + "Received cluster_manager lb config: child names={0}", childAddresses.keySet()); + return childAddresses; } /**