xds: Delegate more RingHashLB address updates to MultiChildLB

ejona86 · ejona86 · commit fd8734f341a1 · 2024-08-12T16:40:00.000-07:00
Since 0447497 RingHashLB has not used acceptResolvedAddressesInternal(). At the time that was needed because deactivated children were part of MultiChildLB. But in 9de8e44, the logic of RingHashLB and MultiChildLB.acceptResolvedAddressesInternal() converged, so it can now swap back to using the base class for more logic.
diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java
@@ -231,7 +231,7 @@ protected final AcceptResolvedAddrRetVal acceptResolvedAddressesInternal(
     return new AcceptResolvedAddrRetVal(Status.OK, getRemovedChildren(newChildren.keySet()));
   }
 
-  protected final void addMissingChildren(Map<Object, ChildLbState> newChildren) {
+  private void addMissingChildren(Map<Object, ChildLbState> newChildren) {
     // Do adds and identify reused children
     for (Map.Entry<Object, ChildLbState> entry : newChildren.entrySet()) {
       final Object key = entry.getKey();
@@ -241,7 +241,7 @@ protected final void addMissingChildren(Map<Object, ChildLbState> newChildren) {
     }
   }
 
-  protected final void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses,
+  private void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses,
                                                      Map<Object, ChildLbState> newChildren) {
     for (Map.Entry<Object, ChildLbState> entry : newChildren.entrySet()) {
       Object childConfig = entry.getValue().getConfig();
@@ -256,7 +256,7 @@ protected final void updateChildrenWithResolvedAddresses(ResolvedAddresses resol
   /**
    * Identifies which children have been removed (are not part of the newChildKeys).
    */
-  protected final List<ChildLbState> getRemovedChildren(Set<Object> newChildKeys) {
+  private List<ChildLbState> getRemovedChildren(Set<Object> newChildKeys) {
     List<ChildLbState> removedChildren = new ArrayList<>();
     // Do removals
     for (Object key : ImmutableList.copyOf(childLbStates.keySet())) {
diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java
@@ -89,19 +89,11 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
 
     try {
       resolvingAddresses = true;
-      // Subclass handles any special manipulation to create appropriate types of ChildLbStates
-      Map<Object, ChildLbState> newChildren = createChildLbMap(resolvedAddresses);
-
-      if (newChildren.isEmpty()) {
-        addressValidityStatus = Status.UNAVAILABLE.withDescription(
-            "Ring hash lb error: EDS resolution was successful, but there were no valid addresses");
-        handleNameResolutionError(addressValidityStatus);
-        return addressValidityStatus;
+      AcceptResolvedAddrRetVal acceptRetVal = acceptResolvedAddressesInternal(resolvedAddresses);
+      if (!acceptRetVal.status.isOk()) {
+        return acceptRetVal.status;
       }
 
-      addMissingChildren(newChildren);
-      updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren);
-
       // Now do the ringhash specific logic with weights and building the ring
       RingHashConfig config = (RingHashConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
       if (config == null) {
@@ -145,7 +137,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
       // clusters and resolver can remove them in service config.
       updateOverallBalancingState();
 
-      shutdownRemoved(getRemovedChildren(newChildren.keySet()));
+      shutdownRemoved(acceptRetVal.removedChildren);
     } finally {
       this.resolvingAddresses = false;
     }
