core: round robin should ignore name resolution error for channel state change when there are READY subchannels (#7595)

Round robin is keeping use of READY subchannels even if there is name resolution error. However, it moves Channel state to TRANSIENT_ERROR.

In hierarchical load balancers, the upstream LB policy may need to aggregate pickers from multiple downstream round_robin LB policy while filtering out non-ready subchannels. It cannot infer if the subchannel can be used just from the SubchannelPicker interface. It relies on the state that the round_robin intends to set channel to.

So the change is to match the readiness of the picker/subchannel with the state that round_robin tries to update. It will completely ignore name resolution error if there are READY subchannels.

Author: voidzcy

core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java

@@ -130,9 +130,9 @@ public void onSubchannelState(ConnectivityStateInfo state) {
 
   @Override
   public void handleNameResolutionError(Status error) {
-    // ready pickers aren't affected by status changes
-    updateBalancingState(TRANSIENT_FAILURE,
-        currentPicker instanceof ReadyPicker ? currentPicker : new EmptyPicker(error));
+    if (currentState != READY)  {
+      updateBalancingState(TRANSIENT_FAILURE, new EmptyPicker(error));
+    }
   }
 
   private void processSubchannelState(Subchannel subchannel, ConnectivityStateInfo stateInfo) {

core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java

@@ -381,13 +381,12 @@ public void nameResolutionErrorWithActiveChannels() throws Exception {
     loadBalancer.handleNameResolutionError(Status.NOT_FOUND.withDescription("nameResolutionError"));
 
     verify(mockHelper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
-    verify(mockHelper, times(3))
+    verify(mockHelper, times(2))
         .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture());
 
     Iterator<ConnectivityState> stateIterator = stateCaptor.getAllValues().iterator();
     assertEquals(CONNECTING, stateIterator.next());
     assertEquals(READY, stateIterator.next());
-    assertEquals(TRANSIENT_FAILURE, stateIterator.next());
 
     LoadBalancer.PickResult pickResult = pickerCaptor.getValue().pickSubchannel(mockArgs);
     assertEquals(readySubchannel, pickResult.getSubchannel());