Skip to content

Commit

Permalink
core: round robin should ignore name resolution error for channel sta…
Browse files Browse the repository at this point in the history
…te 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.
  • Loading branch information
voidzcy committed Nov 4, 2020
1 parent 8020a73 commit 10dc41a
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java
Expand Up @@ -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) {
Expand Down
Expand Up @@ -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());
Expand Down

0 comments on commit 10dc41a

Please sign in to comment.