core: ManagedChannelImpl to always use RetryingNameResolver (#10328)

ManagedCahnnelImpl did not make sure to use a RetryingNameResolver if
authority was not overriden. This was not a problem for DNS name
resolution as the DNS name resolver factory explicitly returns a
RetryingNameResolver. For polling name resolvers that do not do this in
their factories (like the grpclb name resolver) this meant not having retry
at all.
This commit is contained in:
Terry Wilson 2023-06-29 13:47:23 -07:00 committed by GitHub
parent 443a0502ba
commit 8192ff6103
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 15 deletions

View File

@ -749,9 +749,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
String target, @Nullable final String overrideAuthority,
NameResolver.Factory nameResolverFactory, NameResolver.Args nameResolverArgs) {
NameResolver resolver = getNameResolver(target, nameResolverFactory, nameResolverArgs);
if (overrideAuthority == null) {
return resolver;
}
// If the nameResolver is not already a RetryingNameResolver, then wrap it with it.
// This helps guarantee that name resolution retry remains supported even as it has been
@ -769,6 +766,10 @@ final class ManagedChannelImpl extends ManagedChannel implements
nameResolverArgs.getSynchronizationContext());
}
if (overrideAuthority == null) {
return usedNameResolver;
}
return new ForwardingNameResolver(usedNameResolver) {
@Override
public String getServiceAuthority() {

View File

@ -139,8 +139,9 @@ public class ManagedChannelImplGetNameResolverTest {
private void testValidTarget(String target, String expectedUriString, URI expectedUri) {
NameResolver.Factory nameResolverFactory = new FakeNameResolverFactory(expectedUri.getScheme());
FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
target, null, nameResolverFactory, NAMERESOLVER_ARGS);
FakeNameResolver nameResolver
= (FakeNameResolver) ((RetryingNameResolver) ManagedChannelImpl.getNameResolver(
target, null, nameResolverFactory, NAMERESOLVER_ARGS)).getRetriedNameResolver();
assertNotNull(nameResolver);
assertEquals(expectedUri, nameResolver.uri);
assertEquals(expectedUriString, nameResolver.uri.toString());

View File

@ -280,6 +280,11 @@ public class ManagedChannelImplTest {
ArgumentCaptor.forClass(ClientStreamListener.class);
private void createChannel(ClientInterceptor... interceptors) {
createChannel(false, interceptors);
}
private void createChannel(boolean nameResolutionExpectedToFail,
ClientInterceptor... interceptors) {
checkState(channel == null);
channel = new ManagedChannelImpl(
@ -288,7 +293,7 @@ public class ManagedChannelImplTest {
timer.getTimeProvider());
if (requestConnection) {
int numExpectedTasks = 0;
int numExpectedTasks = nameResolutionExpectedToFail ? 1 : 0;
// Force-exit the initial idle-mode
channel.syncContext.execute(new Runnable() {
@ -3000,7 +3005,7 @@ public class ManagedChannelImplTest {
FakeNameResolverFactory nameResolverFactory =
new FakeNameResolverFactory.Builder(expectedUri).setError(error).build();
channelBuilder.nameResolverFactory(nameResolverFactory);
createChannel();
createChannel(true);
assertThat(getStats(channel).channelTrace.events).contains(new ChannelTrace.Event.Builder()
.setDescription("Failed to resolve name: " + error)
@ -3503,10 +3508,11 @@ public class ManagedChannelImplTest {
ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
helper = helperCaptor.getValue();
verify(mockLoadBalancer).acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(nameResolverFactory.servers)
.build());
verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture());
ResolvedAddresses resolvedAddresses = resolvedAddressCaptor.getValue();
assertThat(resolvedAddresses.getAddresses()).isEqualTo(nameResolverFactory.servers);
assertThat(resolvedAddresses.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)).isNotNull();
// simulating request connection and then transport ready after resolved address
Subchannel subchannel =
@ -3609,10 +3615,11 @@ public class ManagedChannelImplTest {
ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
helper = helperCaptor.getValue();
verify(mockLoadBalancer).acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(nameResolverFactory.servers)
.build());
verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture());
ResolvedAddresses resolvedAddresses = resolvedAddressCaptor.getValue();
assertThat(resolvedAddresses.getAddresses()).isEqualTo(nameResolverFactory.servers);
assertThat(resolvedAddresses.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)).isNotNull();
// simulating request connection and then transport ready after resolved address
Subchannel subchannel =