Response to code review

api/src/main/java/io/grpc/LoadBalancer.java

@@ -115,6 +115,19 @@ public abstract class LoadBalancer {
   @NameResolver.ResolutionResultAttr
   public static final Attributes.Key<Map<String, ?>> ATTR_HEALTH_CHECKING_CONFIG =
       Attributes.Key.create("internal:health-checking-config");
+
+  public static final SubchannelPicker EMPTY_PICKER = new SubchannelPicker() {
+    @Override
+    public PickResult pickSubchannel(PickSubchannelArgs args) {
+      return PickResult.withNoResult();
+    }
+
+    @Override
+    public String toString() {
+      return "EMPTY_PICKER";
+    }
+  };
+
   private int recursionCount;
 
   /**
@@ -1398,4 +1411,26 @@ public abstract static class Factory {
      */
     public abstract LoadBalancer newLoadBalancer(Helper helper);
   }
+
+  public static final class ErrorPicker extends SubchannelPicker {
+
+    private final Status error;
+
+    public ErrorPicker(Status error) {
+      this.error = checkNotNull(error, "error");
+    }
+
+    @Override
+    public PickResult pickSubchannel(PickSubchannelArgs args) {
+      return PickResult.withError(error);
+    }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add("error", error)
+          .toString();
+    }
+  }
+
 }

util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020 The gRPC Authors
+ * Copyright 2023 The gRPC Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,7 +39,10 @@
 import javax.annotation.Nullable;
 
 /**
- * The top-level load balancing policy.
+ * A base load balancing policy for those policies which has multiple children such as
+ * ClusterManager or the petiole policies.
+ *
+ * @since 1.58
  */
 public abstract class MultiChildLoadBalancer extends LoadBalancer {
 
@@ -60,9 +63,13 @@ protected MultiChildLoadBalancer(Helper helper) {
     logger.log(Level.FINE, "Created");
   }
 
-  protected abstract SubchannelPicker getInitialPicker();
+  protected SubchannelPicker getInitialPicker() {
+    return EMPTY_PICKER;
+  }
 
-  protected abstract SubchannelPicker getErrorPicker(Status error);
+  protected SubchannelPicker getErrorPicker(Status error)  {
+    return new ErrorPicker(error);
+  }
 
   protected abstract Map<Object, PolicySelection> getPolicySelectionMap(
       ResolvedAddresses resolvedAddresses);

xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java

@@ -39,7 +39,6 @@
 import io.grpc.xds.XdsClusterResource.CdsUpdate;
 import io.grpc.xds.XdsClusterResource.CdsUpdate.ClusterType;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;

xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java

@@ -17,7 +17,6 @@
 package io.grpc.xds;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static io.grpc.xds.XdsSubchannelPickers.BUFFER_PICKER;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
@@ -46,7 +45,6 @@
 import io.grpc.xds.ThreadSafeRandom.ThreadSafeRandomImpl;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
 import io.grpc.xds.XdsNameResolverProvider.CallCounterProvider;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import io.grpc.xds.internal.security.SslContextProviderSupplier;
 import io.grpc.xds.orca.OrcaPerRequestUtil;
 import io.grpc.xds.orca.OrcaPerRequestUtil.OrcaPerRequestReportListener;
@@ -173,7 +171,7 @@ public void shutdown() {
   private final class ClusterImplLbHelper extends ForwardingLoadBalancerHelper {
     private final AtomicLong inFlights;
     private ConnectivityState currentState = ConnectivityState.IDLE;
-    private SubchannelPicker currentPicker = BUFFER_PICKER;
+    private SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
     private List<DropOverload> dropPolicies = Collections.emptyList();
     private long maxConcurrentRequests = DEFAULT_PER_CLUSTER_MAX_CONCURRENT_REQUESTS;
     @Nullable

xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java

@@ -16,16 +16,13 @@
 
 package io.grpc.xds;
 
-import static io.grpc.xds.XdsSubchannelPickers.BUFFER_PICKER;
-
 import com.google.common.base.MoreObjects;
 import io.grpc.InternalLogId;
 import io.grpc.Status;
 import io.grpc.internal.ServiceConfigUtil.PolicySelection;
 import io.grpc.util.MultiChildLoadBalancer;
 import io.grpc.xds.ClusterManagerLoadBalancerProvider.ClusterManagerConfig;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -45,7 +42,7 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer {
 
   @Override
   protected SubchannelPicker getInitialPicker() {
-    return BUFFER_PICKER;
+    return EMPTY_PICKER;
   }
 
   @Override

xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java

@@ -55,7 +55,6 @@
 import io.grpc.xds.XdsClient.ResourceWatcher;
 import io.grpc.xds.XdsEndpointResource.EdsUpdate;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;

xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java

@@ -21,7 +21,6 @@
 import static io.grpc.ConnectivityState.IDLE;
 import static io.grpc.ConnectivityState.READY;
 import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
-import static io.grpc.xds.XdsSubchannelPickers.BUFFER_PICKER;
 
 import io.grpc.ConnectivityState;
 import io.grpc.InternalLogId;
@@ -36,7 +35,6 @@
 import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig;
 import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -149,7 +147,7 @@ private void tryNextPriority() {
         ChildLbState child =
             new ChildLbState(priority, priorityConfigs.get(priority).ignoreReresolution);
         children.put(priority, child);
-        updateOverallState(priority, CONNECTING, BUFFER_PICKER);
+        updateOverallState(priority, CONNECTING, LoadBalancer.EMPTY_PICKER);
         // Calling the child's updateResolvedAddresses() can result in tryNextPriority() being
         // called recursively. We need to be sure to be done with processing here before it is
         // called.
@@ -210,7 +208,7 @@ private final class ChildLbState {
     @Nullable ScheduledHandle deletionTimer;
     @Nullable String policy;
     ConnectivityState connectivityState = CONNECTING;
-    SubchannelPicker picker = BUFFER_PICKER;
+    SubchannelPicker picker = LoadBalancer.EMPTY_PICKER;
 
     ChildLbState(final String priority, boolean ignoreReresolution) {
       this.priority = priority;

xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java

@@ -39,7 +39,6 @@
 import io.grpc.Status;
 import io.grpc.SynchronizationContext;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Collections;

xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java

@@ -21,7 +21,6 @@
 import static io.grpc.ConnectivityState.IDLE;
 import static io.grpc.ConnectivityState.READY;
 import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
-import static io.grpc.xds.XdsSubchannelPickers.BUFFER_PICKER;
 
 import com.google.common.collect.ImmutableMap;
 import io.grpc.ConnectivityState;
@@ -34,7 +33,6 @@
 import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedPolicySelection;
 import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedTargetConfig;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -159,7 +157,7 @@ private void updateOverallBalancingState() {
       if (overallState == TRANSIENT_FAILURE) {
         picker = new WeightedRandomPicker(errorPickers);
       } else {
-        picker = XdsSubchannelPickers.BUFFER_PICKER;
+        picker = LoadBalancer.EMPTY_PICKER;
       }
     } else {
       picker = new WeightedRandomPicker(childPickers);
@@ -191,7 +189,7 @@ private static ConnectivityState aggregateState(
   private final class ChildHelper extends ForwardingLoadBalancerHelper {
     String name;
     ConnectivityState currentState = CONNECTING;
-    SubchannelPicker currentPicker = BUFFER_PICKER;
+    SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
 
     private ChildHelper(String name) {
       this.name = name;

xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java

@@ -32,7 +32,6 @@
 import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedPolicySelection;
 import io.grpc.xds.WeightedTargetLoadBalancerProvider.WeightedTargetConfig;
 import io.grpc.xds.XdsLogger.XdsLogLevel;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;

xds/src/test/java/io/grpc/xds/ClusterManagerLoadBalancerTest.java

@@ -54,7 +54,6 @@
 import io.grpc.internal.ServiceConfigUtil.PolicySelection;
 import io.grpc.testing.TestMethodDescriptors;
 import io.grpc.xds.ClusterManagerLoadBalancerProvider.ClusterManagerConfig;
-import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;