mirror of https://github.com/grpc/grpc-java.git
Eliminate NPE after recovering from a temporary name resolution failure. (#11298)
* Eliminate NPE after recovering from a temporary name resolution failure. * Add test case for 2 failing subchannels to make sure it causes channel to go into TF.
This commit is contained in:
parent
0fea7dd32e
commit
9b39b3ec6b
|
|
@ -180,22 +180,32 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void handleNameResolutionError(Status error) {
|
public void handleNameResolutionError(Status error) {
|
||||||
|
if (rawConnectivityState == SHUTDOWN) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
for (SubchannelData subchannelData : subchannels.values()) {
|
for (SubchannelData subchannelData : subchannels.values()) {
|
||||||
subchannelData.getSubchannel().shutdown();
|
subchannelData.getSubchannel().shutdown();
|
||||||
}
|
}
|
||||||
subchannels.clear();
|
subchannels.clear();
|
||||||
|
if (addressIndex != null) {
|
||||||
|
addressIndex.updateGroups(null);
|
||||||
|
}
|
||||||
|
rawConnectivityState = TRANSIENT_FAILURE;
|
||||||
updateBalancingState(TRANSIENT_FAILURE, new Picker(PickResult.withError(error)));
|
updateBalancingState(TRANSIENT_FAILURE, new Picker(PickResult.withError(error)));
|
||||||
}
|
}
|
||||||
|
|
||||||
void processSubchannelState(Subchannel subchannel, ConnectivityStateInfo stateInfo) {
|
void processSubchannelState(Subchannel subchannel, ConnectivityStateInfo stateInfo) {
|
||||||
ConnectivityState newState = stateInfo.getState();
|
ConnectivityState newState = stateInfo.getState();
|
||||||
|
|
||||||
|
SubchannelData subchannelData = subchannels.get(getAddress(subchannel));
|
||||||
// Shutdown channels/previously relevant subchannels can still callback with state updates.
|
// Shutdown channels/previously relevant subchannels can still callback with state updates.
|
||||||
// To prevent pickers from returning these obsolete subchannels, this logic
|
// To prevent pickers from returning these obsolete subchannels, this logic
|
||||||
// is included to check if the current list of active subchannels includes this subchannel.
|
// is included to check if the current list of active subchannels includes this subchannel.
|
||||||
SubchannelData subchannelData = subchannels.get(getAddress(subchannel));
|
|
||||||
if (subchannelData == null || subchannelData.getSubchannel() != subchannel) {
|
if (subchannelData == null || subchannelData.getSubchannel() != subchannel) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (newState == SHUTDOWN) {
|
if (newState == SHUTDOWN) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -657,6 +657,37 @@ public class PickFirstLeafLoadBalancerTest {
|
||||||
assertEquals(mockSubchannel1, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
assertEquals(mockSubchannel1, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void nameResolutionTemporaryError() {
|
||||||
|
List<EquivalentAddressGroup> newServers = Lists.newArrayList(servers.get(0));
|
||||||
|
InOrder inOrder = inOrder(mockHelper, mockSubchannel1);
|
||||||
|
loadBalancer.acceptResolvedAddresses(
|
||||||
|
ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(affinity).build());
|
||||||
|
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
|
||||||
|
SubchannelStateListener stateListener1 = stateListenerCaptor.getValue();
|
||||||
|
stateListener1.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
|
||||||
|
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
|
||||||
|
assertEquals(mockSubchannel1, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
||||||
|
|
||||||
|
loadBalancer.handleNameResolutionError(
|
||||||
|
Status.UNAVAILABLE.withDescription("nameResolutionError"));
|
||||||
|
inOrder.verify(mockHelper).updateBalancingState(
|
||||||
|
eq(TRANSIENT_FAILURE), any(SubchannelPicker.class));
|
||||||
|
|
||||||
|
loadBalancer.acceptResolvedAddresses(
|
||||||
|
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build());
|
||||||
|
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
|
||||||
|
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
|
||||||
|
SubchannelStateListener stateListener2 = stateListenerCaptor.getValue();
|
||||||
|
|
||||||
|
assertNull(pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
||||||
|
|
||||||
|
stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
|
||||||
|
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
|
||||||
|
assertEquals(mockSubchannel1, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void nameResolutionErrorWithStateChanges() {
|
public void nameResolutionErrorWithStateChanges() {
|
||||||
List<EquivalentAddressGroup> newServers = Lists.newArrayList(servers.get(0));
|
List<EquivalentAddressGroup> newServers = Lists.newArrayList(servers.get(0));
|
||||||
|
|
@ -725,6 +756,33 @@ public class PickFirstLeafLoadBalancerTest {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void failChannelWhenSubchannelsFail() {
|
||||||
|
List<EquivalentAddressGroup> newServers = Lists.newArrayList(servers.get(0), servers.get(1));
|
||||||
|
when(mockSubchannel1.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(0)));
|
||||||
|
when(mockSubchannel2.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(1)));
|
||||||
|
|
||||||
|
// accept resolved addresses
|
||||||
|
loadBalancer.acceptResolvedAddresses(
|
||||||
|
ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(affinity).build());
|
||||||
|
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2);
|
||||||
|
verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
|
||||||
|
verify(mockHelper).createSubchannel(createArgsCaptor.capture());
|
||||||
|
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
|
||||||
|
SubchannelStateListener stateListener = stateListenerCaptor.getValue();
|
||||||
|
assertNull(pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
|
||||||
|
|
||||||
|
inOrder.verify(mockSubchannel1).requestConnection();
|
||||||
|
stateListener.onSubchannelState(ConnectivityStateInfo.forTransientFailure(CONNECTION_ERROR));
|
||||||
|
|
||||||
|
inOrder.verify(mockSubchannel2).start(stateListenerCaptor.capture());
|
||||||
|
SubchannelStateListener stateListener2 = stateListenerCaptor.getValue();
|
||||||
|
stateListener2.onSubchannelState(ConnectivityStateInfo.forTransientFailure(CONNECTION_ERROR));
|
||||||
|
|
||||||
|
inOrder.verify(mockHelper).updateBalancingState(eq(TRANSIENT_FAILURE), pickerCaptor.capture());
|
||||||
|
assertEquals(CONNECTION_ERROR, pickerCaptor.getValue().pickSubchannel(mockArgs).getStatus());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void updateAddresses_emptyEagList_returns_false() {
|
public void updateAddresses_emptyEagList_returns_false() {
|
||||||
loadBalancer.acceptResolvedAddresses(
|
loadBalancer.acceptResolvedAddresses(
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue