grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

core: turn Channel into TF if resolver gives empty addresses with policy selection cannot handle it (#7073) 

The original service config error handling design was unclear about the case when an updated resolution result with valid service config and empty address is returned to Channel, and the selected LB policy does not accept empty addresses. Existing implementation silently triggers resolver backoff after LB policy is changed, while leaving Channel being CONNECTING state, if there was an old service config resolved. This change converge the behaviors of whether the received service config is the first one or an update to a previous config, in both cases, Channel goes into TRANSIENT_FAILURE if the selected LB policy cannot handle empty addresses.
This commit is contained in:
Chengyuan Zhang 2020-05-30 01:23:37 +00:00 committed by GitHub
parent 0201c5a9a7
commit 683eed671e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 8 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
View File

@ -1369,7 +1369,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
"Resolved address: {0}, config={1}",
servers,
resolutionResult.getAttributes());
ResolutionState lastResolutionStateCopy = lastResolutionState;
if (lastResolutionState != ResolutionState.SUCCESS) {
channelLogger.log(ChannelLogLevel.INFO, "Address resolved: {0}", servers);
@ -1457,14 +1456,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
.build());
if (!handleResult.isOk()) {
if (servers.isEmpty() && lastResolutionStateCopy == ResolutionState.SUCCESS) {
// lb doesn't expose that it needs address or not, because for some LB it is not
// deterministic. Assuming lb needs address if LB returns error when the address is
// empty and it is not the first resolution.
scheduleExponentialBackOffInSyncContext();
} else {
handleErrorInSyncContext(handleResult.augmentDescription(resolver + " was used"));
}
handleErrorInSyncContext(handleResult.augmentDescription(resolver + " was used"));
}
}
}

12
core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java
View File

@ -274,17 +274,19 @@ public class ServiceConfigErrorHandlingTest {
assertThat(channel.getState(true)).isEqualTo(ConnectivityState.IDLE);
reset(mockLoadBalancer);
Map<String, ?> ignoredServiceConfig =
parseJson("{\"loadBalancingConfig\": [{\"round_robin\": {}}]}");
nameResolverFactory.nextRawServiceConfig.set(ignoredServiceConfig);
nameResolverFactory.servers.clear();
// 2nd resolution
nameResolverFactory.allResolved();
// 2nd service config without address should be ignored
// 2nd service config without addresses
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(Status.class);
verify(mockLoadBalancer, never()).handleResolvedAddresses(any(ResolvedAddresses.class));
verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class));
verify(mockLoadBalancer).handleNameResolutionError(statusCaptor.capture());
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.UNAVAILABLE);
assertThat(statusCaptor.getValue().getDescription())
.contains("NameResolver returned no usable address.");
assertThat(channel.getState(true)).isEqualTo(ConnectivityState.TRANSIENT_FAILURE);
assertWithMessage("Empty address should schedule NameResolver retry")
.that(getNameResolverRefresh())
.isNotNull();