diff options
author | Terry Wilson <tmwilson@google.com> | 2023-07-12 15:08:13 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-12 15:08:13 -0700 |
commit | e72741ec117646138945239fb5a05177f8577254 (patch) | |
tree | 2fd36988f423c7c903d89961d1375b40fb1f711c | |
parent | 81ee3d6db7eacc0f7898b5f95212de14324e89a4 (diff) | |
download | grpc-grpc-java-e72741ec117646138945239fb5a05177f8577254.tar.gz |
core: Apply RetryingNameResolver in ManagedChannelImpl (#10371) (#10376)
Wrapping the DnsNameResolver in DnsNameResolverProvider can cause
problems to external name resolvers that delegate to a DnsResolver
already wrapped in RetryingNameResolver. ManagedChannelImpl would
end up wrapping these name resolvers again, causing an exception
later from a RetryingNameResolver safeguard that checks for double
wrapping.
5 files changed, 15 insertions, 37 deletions
diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index da0410f3e..1d127d567 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java @@ -53,19 +53,13 @@ public final class DnsNameResolverProvider extends NameResolverProvider { Preconditions.checkArgument(targetPath.startsWith("/"), "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri); String name = targetPath.substring(1); - return new RetryingNameResolver( - new DnsNameResolver( - targetUri.getAuthority(), - name, - args, - GrpcUtil.SHARED_CHANNEL_EXECUTOR, - Stopwatch.createUnstarted(), - InternalServiceProviders.isAndroid(getClass().getClassLoader())), - new BackoffPolicyRetryScheduler( - new ExponentialBackoffPolicy.Provider(), - args.getScheduledExecutorService(), - args.getSynchronizationContext()), - args.getSynchronizationContext()); + return new DnsNameResolver( + targetUri.getAuthority(), + name, + args, + GrpcUtil.SHARED_CHANNEL_EXECUTOR, + Stopwatch.createUnstarted(), + InternalServiceProviders.isAndroid(getClass().getClassLoader())); } else { return null; } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 58f2b32c8..4224a0881 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -749,21 +749,14 @@ final class ManagedChannelImpl extends ManagedChannel implements NameResolver.Factory nameResolverFactory, NameResolver.Args nameResolverArgs) { NameResolver resolver = getNameResolver(target, nameResolverFactory, nameResolverArgs); - // 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 - // removed from ManagedChannelImpl. + // We wrap the name resolver in a RetryingNameResolver to give it the ability to retry failures. // TODO: After a transition period, all NameResolver implementations that need retry should use // RetryingNameResolver directly and this step can be removed. - NameResolver usedNameResolver; - if (resolver instanceof RetryingNameResolver) { - usedNameResolver = resolver; - } else { - usedNameResolver = new RetryingNameResolver(resolver, + NameResolver usedNameResolver = new RetryingNameResolver(resolver, new BackoffPolicyRetryScheduler(new ExponentialBackoffPolicy.Provider(), nameResolverArgs.getScheduledExecutorService(), nameResolverArgs.getSynchronizationContext()), nameResolverArgs.getSynchronizationContext()); - } if (overrideAuthority == null) { return usedNameResolver; diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index b07d7131c..aff10ce93 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -61,9 +61,7 @@ public class DnsNameResolverProviderTest { @Test public void newNameResolver() { assertSame(DnsNameResolver.class, - ((RetryingNameResolver) provider.newNameResolver( - URI.create("dns:///localhost:443"), args)) - .getRetriedNameResolver().getClass()); + provider.newNameResolver(URI.create("dns:///localhost:443"), args).getClass()); assertNull( provider.newNameResolver(URI.create("notdns:///localhost:443"), args)); } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 33b127a46..87f704e12 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -1293,8 +1293,7 @@ public class DnsNameResolverTest { } private void testValidUri(URI uri, String exportedAuthority, int expectedPort) { - DnsNameResolver resolver = (DnsNameResolver) ((RetryingNameResolver) provider.newNameResolver( - uri, args)).getRetriedNameResolver(); + DnsNameResolver resolver = (DnsNameResolver) provider.newNameResolver(uri, args); assertNotNull(resolver); assertEquals(expectedPort, resolver.getPort()); assertEquals(exportedAuthority, resolver.getServiceAuthority()); diff --git a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java index da3ed7c50..01deda156 100644 --- a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java +++ b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java @@ -542,7 +542,7 @@ public class ServiceConfigErrorHandlingTest { final URI expectedUri; final List<EquivalentAddressGroup> servers; final boolean resolvedAtStart; - final ArrayList<RetryingNameResolver> resolvers = new ArrayList<>(); + final ArrayList<FakeNameResolver> resolvers = new ArrayList<>(); final AtomicReference<Map<String, ?>> nextRawServiceConfig = new AtomicReference<>(); final AtomicReference<Attributes> nextAttributes = new AtomicReference<>(Attributes.EMPTY); @@ -561,13 +561,7 @@ public class ServiceConfigErrorHandlingTest { return null; } assertEquals(DEFAULT_PORT, args.getDefaultPort()); - RetryingNameResolver resolver = new RetryingNameResolver( - new FakeNameResolver(args.getServiceConfigParser()), - new BackoffPolicyRetryScheduler( - new FakeBackoffPolicyProvider(), - args.getScheduledExecutorService(), - args.getSynchronizationContext()), - args.getSynchronizationContext()); + FakeNameResolver resolver = new FakeNameResolver(args.getServiceConfigParser()); resolvers.add(resolver); return resolver; } @@ -578,8 +572,8 @@ public class ServiceConfigErrorHandlingTest { } void allResolved() { - for (RetryingNameResolver resolver : resolvers) { - ((FakeNameResolver)resolver.getRetriedNameResolver()).resolved(); + for (FakeNameResolver resolver : resolvers) { + resolver.resolved(); } } |