Move name resolution retry from managed channel to name resolver (take #2) (#9812)

This change has these main aspects to it:

1. Removal of any name resolution responsibility from ManagedChannelImpl
2. Creation of a new RetryScheduler to own generic retry logic
     - Can also be used outside the name resolution context
3. Creation of a new RetryingNameScheduler that can be used to wrap any
   polling name resolver to add retry capability
4. A new facility in NameResolver to allow implementations to notify
   listeners on the success of name resolution attempts
     - RetryingNameScheduler relies on this

core/src/main/java/io/grpc/internal/BackoffPolicyRetryScheduler.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright 2023 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+import io.grpc.SynchronizationContext;
+import io.grpc.SynchronizationContext.ScheduledHandle;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Schedules a retry operation according to a {@link BackoffPolicy}. The retry is run within a
+ * {@link SynchronizationContext}. At most one retry is scheduled at a time.
+ */
+final class BackoffPolicyRetryScheduler implements RetryScheduler {
+  private final ScheduledExecutorService scheduledExecutorService;
+  private final SynchronizationContext syncContext;
+  private final BackoffPolicy.Provider policyProvider;
+
+  private BackoffPolicy policy;
+  private ScheduledHandle scheduledHandle;
+
+  private static final Logger logger = Logger.getLogger(
+      BackoffPolicyRetryScheduler.class.getName());
+
+  BackoffPolicyRetryScheduler(BackoffPolicy.Provider policyProvider,
+      ScheduledExecutorService scheduledExecutorService,
+      SynchronizationContext syncContext) {
+    this.policyProvider = policyProvider;
+    this.scheduledExecutorService = scheduledExecutorService;
+    this.syncContext = syncContext;
+  }
+
+  /**
+   * Schedules a future retry operation. Only allows one retry to be scheduled at any given time.
+   */
+  @Override
+  public void schedule(Runnable retryOperation) {
+    syncContext.throwIfNotInThisSynchronizationContext();
+
+    if (policy == null) {
+      policy = policyProvider.get();
+    }
+    // If a retry is already scheduled, take no further action.
+    if (scheduledHandle != null && scheduledHandle.isPending()) {
+      return;
+    }
+    long delayNanos = policy.nextBackoffNanos();
+    scheduledHandle = syncContext.schedule(retryOperation, delayNanos, TimeUnit.NANOSECONDS,
+        scheduledExecutorService);
+    logger.log(Level.FINE, "Scheduling DNS resolution backoff for {0}ns", delayNanos);
+  }
+
+  /**
+   * Resets the {@link BackoffPolicyRetryScheduler} and cancels any pending retry task. The policy
+   * will be cleared thus also resetting any state associated with it (e.g. a backoff multiplier).
+   */
+  @Override
+  public void reset() {
+    syncContext.throwIfNotInThisSynchronizationContext();
+
+    syncContext.execute(() -> {
+      if (scheduledHandle != null && scheduledHandle.isPending()) {
+        scheduledHandle.cancel();
+      }
+      policy = null;
+    });
+  }
+
+}

core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java

@@ -47,19 +47,25 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
   private static final String SCHEME = "dns";
 
   @Override
-  public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
+  public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
     if (SCHEME.equals(targetUri.getScheme())) {
       String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
       Preconditions.checkArgument(targetPath.startsWith("/"),
           "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri);
       String name = targetPath.substring(1);
-      return new DnsNameResolver(
+      return new RetryingNameResolver(
-          targetUri.getAuthority(),
+          new DnsNameResolver(
-          name,
+              targetUri.getAuthority(),
-          args,
+              name,
-          GrpcUtil.SHARED_CHANNEL_EXECUTOR,
+              args,
-          Stopwatch.createUnstarted(),
+              GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-          InternalServiceProviders.isAndroid(getClass().getClassLoader()));
+              Stopwatch.createUnstarted(),
+              InternalServiceProviders.isAndroid(getClass().getClassLoader())),
+          new BackoffPolicyRetryScheduler(
+              new ExponentialBackoffPolicy.Provider(),
+              args.getScheduledExecutorService(),
+              args.getSynchronizationContext()),
+          args.getSynchronizationContext());
     } else {
       return null;
     }

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

@@ -85,6 +85,7 @@ import io.grpc.internal.ManagedChannelServiceConfig.MethodInfo;
 import io.grpc.internal.ManagedChannelServiceConfig.ServiceConfigConvertedSelector;
 import io.grpc.internal.RetriableStream.ChannelBufferMeter;
 import io.grpc.internal.RetriableStream.Throttle;
+import io.grpc.internal.RetryingNameResolver.ResolutionResultListener;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
@@ -367,7 +368,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
       checkState(lbHelper != null, "lbHelper is null");
     }
     if (nameResolver != null) {
-      cancelNameResolverBackoff();
       nameResolver.shutdown();
       nameResolverStarted = false;
       if (channelIsActive) {
@@ -450,42 +450,10 @@ final class ManagedChannelImpl extends ManagedChannel implements
     idleTimer.reschedule(idleTimeoutMillis, TimeUnit.MILLISECONDS);
   }
 
-  // Run from syncContext
-  @VisibleForTesting
-  class DelayedNameResolverRefresh implements Runnable {
-    @Override
-    public void run() {
-      scheduledNameResolverRefresh = null;
-      refreshNameResolution();
-    }
-  }
-
-  // Must be used from syncContext
-  @Nullable private ScheduledHandle scheduledNameResolverRefresh;
-  // The policy to control backoff between name resolution attempts. Non-null when an attempt is
-  // scheduled. Must be used from syncContext
-  @Nullable private BackoffPolicy nameResolverBackoffPolicy;
-
-  // Must be run from syncContext
-  private void cancelNameResolverBackoff() {
-    syncContext.throwIfNotInThisSynchronizationContext();
-    if (scheduledNameResolverRefresh != null) {
-      scheduledNameResolverRefresh.cancel();
-      scheduledNameResolverRefresh = null;
-      nameResolverBackoffPolicy = null;
-    }
-  }
-
   /**
-   * Force name resolution refresh to happen immediately and reset refresh back-off. Must be run
+   * Force name resolution refresh to happen immediately. Must be run
    * from syncContext.
    */
-  private void refreshAndResetNameResolution() {
-    syncContext.throwIfNotInThisSynchronizationContext();
-    cancelNameResolverBackoff();
-    refreshNameResolution();
-  }
-
   private void refreshNameResolution() {
     syncContext.throwIfNotInThisSynchronizationContext();
     if (nameResolverStarted) {
@@ -783,7 +751,24 @@ final class ManagedChannelImpl extends ManagedChannel implements
     if (overrideAuthority == null) {
       return resolver;
     }
-    return new ForwardingNameResolver(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
+    // removed from ManagedChannelImpl.
+    // 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,
+          new BackoffPolicyRetryScheduler(new ExponentialBackoffPolicy.Provider(),
+              nameResolverArgs.getScheduledExecutorService(),
+              nameResolverArgs.getSynchronizationContext()),
+          nameResolverArgs.getSynchronizationContext());
+    }
+
+    return new ForwardingNameResolver(usedNameResolver) {
       @Override
       public String getServiceAuthority() {
         return overrideAuthority;
@@ -1291,7 +1276,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
   // Must be called from syncContext
   private void handleInternalSubchannelState(ConnectivityStateInfo newState) {
     if (newState.getState() == TRANSIENT_FAILURE || newState.getState() == IDLE) {
-      refreshAndResetNameResolution();
+      refreshNameResolution();
     }
   }
 
@@ -1338,9 +1323,8 @@ final class ManagedChannelImpl extends ManagedChannel implements
         if (shutdown.get()) {
           return;
         }
-        if (scheduledNameResolverRefresh != null && scheduledNameResolverRefresh.isPending()) {
+        if (nameResolverStarted) {
-          checkState(nameResolverStarted, "name resolver must be started");
+          refreshNameResolution();
-          refreshAndResetNameResolution();
         }
         for (InternalSubchannel subchannel : subchannels) {
           subchannel.resetConnectBackoff();
@@ -1496,7 +1480,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
       final class LoadBalancerRefreshNameResolution implements Runnable {
         @Override
         public void run() {
-          refreshAndResetNameResolution();
+          ManagedChannelImpl.this.refreshNameResolution();
         }
       }
 
@@ -1727,7 +1711,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
     }
   }
 
-  private final class NameResolverListener extends NameResolver.Listener2 {
+  final class NameResolverListener extends NameResolver.Listener2 {
     final LbHelperImpl helper;
     final NameResolver resolver;
 
@@ -1759,8 +1743,9 @@ final class ManagedChannelImpl extends ManagedChannel implements
             lastResolutionState = ResolutionState.SUCCESS;
           }
 
-          nameResolverBackoffPolicy = null;
           ConfigOrError configOrError = resolutionResult.getServiceConfig();
+          ResolutionResultListener resolutionResultListener = resolutionResult.getAttributes()
+              .get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
           InternalConfigSelector resolvedConfigSelector =
               resolutionResult.getAttributes().get(InternalConfigSelector.KEY);
           ManagedChannelServiceConfig validServiceConfig =
@@ -1817,6 +1802,9 @@ final class ManagedChannelImpl extends ManagedChannel implements
                 // we later check for these error codes when investigating pick results in
                 // GrpcUtil.getTransportFromPickResult().
                 onError(configOrError.getError());
+                if (resolutionResultListener != null) {
+                  resolutionResultListener.resolutionAttempted(false);
+                }
                 return;
               } else {
                 effectiveServiceConfig = lastServiceConfig;
@@ -1861,15 +1849,15 @@ final class ManagedChannelImpl extends ManagedChannel implements
             }
             Attributes attributes = attrBuilder.build();
 
-            boolean addressesAccepted = helper.lb.tryAcceptResolvedAddresses(
+            boolean lastAddressesAccepted = helper.lb.tryAcceptResolvedAddresses(
                 ResolvedAddresses.newBuilder()
                     .setAddresses(servers)
                     .setAttributes(attributes)
                     .setLoadBalancingPolicyConfig(effectiveServiceConfig.getLoadBalancingConfig())
                     .build());
-
+            // If a listener is provided, let it know if the addresses were accepted.
-            if (!addressesAccepted) {
+            if (resolutionResultListener != null) {
-              scheduleExponentialBackOffInSyncContext();
+              resolutionResultListener.resolutionAttempted(lastAddressesAccepted);
             }
           }
         }
@@ -1905,29 +1893,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
       }
 
       helper.lb.handleNameResolutionError(error);
-
-      scheduleExponentialBackOffInSyncContext();
-    }
-
-    private void scheduleExponentialBackOffInSyncContext() {
-      if (scheduledNameResolverRefresh != null && scheduledNameResolverRefresh.isPending()) {
-        // The name resolver may invoke onError multiple times, but we only want to
-        // schedule one backoff attempt
-        // TODO(ericgribkoff) Update contract of NameResolver.Listener or decide if we
-        // want to reset the backoff interval upon repeated onError() calls
-        return;
-      }
-      if (nameResolverBackoffPolicy == null) {
-        nameResolverBackoffPolicy = backoffPolicyProvider.get();
-      }
-      long delayNanos = nameResolverBackoffPolicy.nextBackoffNanos();
-      channelLogger.log(
-          ChannelLogLevel.DEBUG,
-          "Scheduling DNS resolution backoff for {0} ns", delayNanos);
-      scheduledNameResolverRefresh =
-          syncContext.schedule(
-              new DelayedNameResolverRefresh(), delayNanos, TimeUnit.NANOSECONDS,
-              transportFactory .getScheduledExecutorService());
     }
   }
 

core/src/main/java/io/grpc/internal/RetryScheduler.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2023 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+/**
+ * This interface is used to schedule future retry attempts for a failed operation. The retry delay
+ * and the number of attempt is defined by implementing classes. Implementations should assure
+ * that only one future retry operation is ever scheduled at a time.
+ */
+public interface RetryScheduler {
+
+  /**
+   * A request to schedule a future retry (or retries) for a failed operation. Noop if an operation
+   * has already been scheduled.
+   */
+  void schedule(Runnable retryOperation);
+
+  /**
+   * Resets the scheduler, effectively cancelling any future retry operation.
+   */
+  void reset();
+}

core/src/main/java/io/grpc/internal/RetryingNameResolver.java

@@ -0,0 +1,125 @@
+/*
+ * Copyright 2023 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+import com.google.common.annotations.VisibleForTesting;
+import io.grpc.Attributes;
+import io.grpc.NameResolver;
+import io.grpc.Status;
+import io.grpc.SynchronizationContext;
+
+/**
+ * This wrapper class can add retry capability to any polling {@link NameResolver} implementation
+ * that supports calling {@link ResolutionResultListener}s with the outcome of each resolution.
+ *
+ * <p>The {@link NameResolver} used with this
+ */
+final class RetryingNameResolver extends ForwardingNameResolver {
+
+  private final NameResolver retriedNameResolver;
+  private final RetryScheduler retryScheduler;
+  private final SynchronizationContext syncContext;
+
+  static final Attributes.Key<ResolutionResultListener> RESOLUTION_RESULT_LISTENER_KEY
+      = Attributes.Key.create(
+          "io.grpc.internal.RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY");
+
+  /**
+   * Creates a new {@link RetryingNameResolver}.
+   *
+   * @param retriedNameResolver A {@link NameResolver} that will have failed attempt retried.
+   * @param retryScheduler Used to schedule the retry attempts.
+   */
+  RetryingNameResolver(NameResolver retriedNameResolver, RetryScheduler retryScheduler,
+      SynchronizationContext syncContext) {
+    super(retriedNameResolver);
+    this.retriedNameResolver = retriedNameResolver;
+    this.retryScheduler = retryScheduler;
+    this.syncContext = syncContext;
+  }
+
+  @Override
+  public void start(Listener2 listener) {
+    super.start(new RetryingListener(listener));
+  }
+
+  @Override
+  public void shutdown() {
+    super.shutdown();
+    retryScheduler.reset();
+  }
+
+  /**
+   * Used to get the underlying {@link NameResolver} that is getting its failed attempts retried.
+   */
+  @VisibleForTesting
+  NameResolver getRetriedNameResolver() {
+    return retriedNameResolver;
+  }
+
+  @VisibleForTesting
+  class DelayedNameResolverRefresh implements Runnable {
+    @Override
+    public void run() {
+      refresh();
+    }
+  }
+
+  private class RetryingListener extends Listener2 {
+    private Listener2 delegateListener;
+
+    RetryingListener(Listener2 delegateListener) {
+      this.delegateListener = delegateListener;
+    }
+
+    @Override
+    public void onResult(ResolutionResult resolutionResult) {
+      // If the resolution result listener is already an attribute it indicates that a name resolver
+      // has already been wrapped with this class. This indicates a misconfiguration.
+      if (resolutionResult.getAttributes().get(RESOLUTION_RESULT_LISTENER_KEY) != null) {
+        throw new IllegalStateException(
+            "RetryingNameResolver can only be used once to wrap a NameResolver");
+      }
+
+      delegateListener.onResult(resolutionResult.toBuilder().setAttributes(
+              resolutionResult.getAttributes().toBuilder()
+                  .set(RESOLUTION_RESULT_LISTENER_KEY, new ResolutionResultListener()).build())
+          .build());
+    }
+
+    @Override
+    public void onError(Status error) {
+      delegateListener.onError(error);
+      syncContext.execute(() -> retryScheduler.schedule(new DelayedNameResolverRefresh()));
+    }
+  }
+
+  /**
+   * Simple callback class to store in {@link ResolutionResult} attributes so that
+   * ManagedChannel can indicate if the resolved addresses were accepted. Temporary until
+   * the Listener2.onResult() API can be changed to return a boolean for this purpose.
+   */
+  class ResolutionResultListener {
+    public void resolutionAttempted(boolean successful) {
+      if (successful) {
+        retryScheduler.reset();
+      } else {
+        retryScheduler.schedule(new DelayedNameResolverRefresh());
+      }
+    }
+  }
+}

core/src/test/java/io/grpc/internal/BackoffPolicyRetrySchedulerTest.java

@@ -0,0 +1,109 @@
+/*
+ * Copyright 2023 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+
+import io.grpc.SynchronizationContext;
+import java.lang.Thread.UncaughtExceptionHandler;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for {@link BackoffPolicyRetryScheduler}.
+ */
+@RunWith(JUnit4.class)
+public class BackoffPolicyRetrySchedulerTest {
+
+  private final FakeClock fakeClock = new FakeClock();
+
+  private BackoffPolicyRetryScheduler scheduler;
+  private final SynchronizationContext syncContext = new SynchronizationContext(
+      mock(UncaughtExceptionHandler.class));
+
+  @Before
+  public void setup() {
+    scheduler = new BackoffPolicyRetryScheduler(new FakeBackoffPolicyProvider(),
+        fakeClock.getScheduledExecutorService(), syncContext);
+  }
+
+  @Test
+  public void schedule() {
+    AtomicInteger retryCount = new AtomicInteger();
+    Runnable retry = retryCount::incrementAndGet;
+    syncContext.execute(() -> scheduler.schedule(retry));
+
+    fakeClock.forwardTime(2, TimeUnit.NANOSECONDS);
+
+    assertThat(retryCount.get()).isEqualTo(1);
+  }
+
+  @Test
+  public void schedule_noMultiple() {
+    AtomicInteger retryCount = new AtomicInteger();
+    Runnable retry = retryCount::incrementAndGet;
+
+    // We schedule multiple retries...
+    syncContext.execute(() -> scheduler.schedule(retry));
+    syncContext.execute(() -> scheduler.schedule(retry));
+
+    fakeClock.forwardTime(2, TimeUnit.NANOSECONDS);
+
+    // But only one of them should have run.
+    assertThat(retryCount.get()).isEqualTo(1);
+  }
+
+  @Test
+  public void reset() {
+    AtomicInteger retryCount = new AtomicInteger();
+    Runnable retry = retryCount::incrementAndGet;
+    Runnable retryTwo = () -> {
+      retryCount.getAndAdd(2);
+    };
+
+    // We schedule one retry.
+    syncContext.execute(() -> scheduler.schedule(retry));
+
+    // But then reset.
+    syncContext.execute(() -> scheduler.reset());
+
+    // And schedule a different retry.
+    syncContext.execute(() -> scheduler.schedule(retryTwo));
+
+    fakeClock.forwardTime(2, TimeUnit.NANOSECONDS);
+
+    // The retry after the reset should have been run.
+    assertThat(retryCount.get()).isEqualTo(2);
+  }
+
+  private static class FakeBackoffPolicyProvider implements BackoffPolicy.Provider {
+    @Override
+    public BackoffPolicy get() {
+      return new BackoffPolicy() {
+        @Override
+        public long nextBackoffNanos() {
+          return 1;
+        }
+      };
+    }
+  }
+}

core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java

@@ -33,6 +33,8 @@ import org.junit.runners.JUnit4;
 /** Unit tests for {@link DnsNameResolverProvider}. */
 @RunWith(JUnit4.class)
 public class DnsNameResolverProviderTest {
+  private final FakeClock fakeClock = new FakeClock();
+
   private final SynchronizationContext syncContext = new SynchronizationContext(
       new Thread.UncaughtExceptionHandler() {
         @Override
@@ -46,6 +48,7 @@ public class DnsNameResolverProviderTest {
       .setSynchronizationContext(syncContext)
       .setServiceConfigParser(mock(ServiceConfigParser.class))
       .setChannelLogger(mock(ChannelLogger.class))
+      .setScheduledExecutorService(fakeClock.getScheduledExecutorService())
       .build();
 
   private DnsNameResolverProvider provider = new DnsNameResolverProvider();
@@ -58,7 +61,9 @@ public class DnsNameResolverProviderTest {
   @Test
   public void newNameResolver() {
     assertSame(DnsNameResolver.class,
-        provider.newNameResolver(URI.create("dns:///localhost:443"), args).getClass());
+        ((RetryingNameResolver) provider.newNameResolver(
+            URI.create("dns:///localhost:443"), args))
+            .getRetriedNameResolver().getClass());
     assertNull(
         provider.newNameResolver(URI.create("notdns:///localhost:443"), args));
   }

core/src/test/java/io/grpc/internal/DnsNameResolverTest.java

@@ -25,6 +25,8 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -37,7 +39,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.net.InetAddresses;
 import com.google.common.testing.FakeTicker;
-import io.grpc.Attributes;
 import io.grpc.ChannelLogger;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.HttpConnectProxiedSocketAddress;
@@ -111,17 +112,18 @@ public class DnsNameResolverTest {
           throw new AssertionError(e);
         }
       });
-  private final NameResolver.Args args = NameResolver.Args.newBuilder()
-      .setDefaultPort(DEFAULT_PORT)
-      .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR)
-      .setSynchronizationContext(syncContext)
-      .setServiceConfigParser(mock(ServiceConfigParser.class))
-      .setChannelLogger(mock(ChannelLogger.class))
-      .build();
 
   private final DnsNameResolverProvider provider = new DnsNameResolverProvider();
   private final FakeClock fakeClock = new FakeClock();
   private final FakeClock fakeExecutor = new FakeClock();
+  private static final FakeClock.TaskFilter NAME_RESOLVER_REFRESH_TASK_FILTER =
+      new FakeClock.TaskFilter() {
+        @Override
+        public boolean shouldAccept(Runnable command) {
+          return command.toString().contains(
+              RetryingNameResolver.DelayedNameResolverRefresh.class.getName());
+        }
+      };
 
   private final FakeExecutorResource fakeExecutorResource = new FakeExecutorResource();
 
@@ -138,6 +140,15 @@ public class DnsNameResolverTest {
     public void close(Executor instance) {}
   }
 
+  private final NameResolver.Args args = NameResolver.Args.newBuilder()
+      .setDefaultPort(DEFAULT_PORT)
+      .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR)
+      .setSynchronizationContext(syncContext)
+      .setServiceConfigParser(mock(ServiceConfigParser.class))
+      .setChannelLogger(mock(ChannelLogger.class))
+      .setScheduledExecutorService(fakeExecutor.getScheduledExecutorService())
+      .build();
+
   @Mock
   private NameResolver.Listener2 mockListener;
   @Captor
@@ -149,18 +160,18 @@ public class DnsNameResolverTest {
   @Mock
   private RecordFetcher recordFetcher;
 
-  private DnsNameResolver newResolver(String name, int defaultPort) {
+  private RetryingNameResolver newResolver(String name, int defaultPort) {
     return newResolver(
         name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted());
   }
 
-  private DnsNameResolver newResolver(String name, int defaultPort, boolean isAndroid) {
+  private RetryingNameResolver newResolver(String name, int defaultPort, boolean isAndroid) {
     return newResolver(
         name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(),
         isAndroid);
   }
 
-  private DnsNameResolver newResolver(
+  private RetryingNameResolver newResolver(
       String name,
       int defaultPort,
       ProxyDetector proxyDetector,
@@ -168,7 +179,7 @@ public class DnsNameResolverTest {
     return newResolver(name, defaultPort, proxyDetector, stopwatch, false);
   }
 
-  private DnsNameResolver newResolver(
+  private RetryingNameResolver newResolver(
       String name,
       final int defaultPort,
       final ProxyDetector proxyDetector,
@@ -181,21 +192,31 @@ public class DnsNameResolverTest {
             .setSynchronizationContext(syncContext)
             .setServiceConfigParser(mock(ServiceConfigParser.class))
             .setChannelLogger(mock(ChannelLogger.class))
+            .setScheduledExecutorService(fakeExecutor.getScheduledExecutorService())
             .build();
     return newResolver(name, stopwatch, isAndroid, args);
   }
 
-  private DnsNameResolver newResolver(
+  private RetryingNameResolver newResolver(
       String name,
       Stopwatch stopwatch,
       boolean isAndroid,
       NameResolver.Args args) {
-    DnsNameResolver dnsResolver =
+    DnsNameResolver dnsResolver = new DnsNameResolver(null, name, args, fakeExecutorResource,
-        new DnsNameResolver(
+        stopwatch, isAndroid);
-            null, name, args, fakeExecutorResource, stopwatch, isAndroid);
     // By default, using the mocked ResourceResolver to avoid I/O
     dnsResolver.setResourceResolver(new JndiResourceResolver(recordFetcher));
-    return dnsResolver;
+
+    // In practice the DNS name resolver provider always wraps the resolver in a
+    // RetryingNameResolver which adds retry capabilities to it. We use the same setup here.
+    return new RetryingNameResolver(
+        dnsResolver,
+        new BackoffPolicyRetryScheduler(
+            new ExponentialBackoffPolicy.Provider(),
+            fakeExecutor.getScheduledExecutorService(),
+            syncContext
+        ),
+        syncContext);
   }
 
   @Before
@@ -203,6 +224,15 @@ public class DnsNameResolverTest {
     DnsNameResolver.enableJndi = true;
     networkaddressCacheTtlPropertyValue =
         System.getProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
+
+    // By default the mock listener processes the result successfully.
+    doAnswer(invocation -> {
+      ResolutionResult result = invocation.getArgument(0);
+      syncContext.execute(
+          () -> result.getAttributes().get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)
+              .resolutionAttempted(true));
+      return null;
+    }).when(mockListener).onResult(isA(ResolutionResult.class));
   }
 
   @After
@@ -216,12 +246,6 @@ public class DnsNameResolverTest {
     }
   }
 
-  @After
-  public void noMorePendingTasks() {
-    assertEquals(0, fakeClock.numPendingTasks());
-    assertEquals(0, fakeExecutor.numPendingTasks());
-  }
-
   @Test
   public void invalidDnsName() throws Exception {
     testInvalidUri(new URI("dns", null, "/[invalid]", null));
@@ -287,10 +311,11 @@ public class DnsNameResolverTest {
     final List<InetAddress> answer2 = createAddressList(1);
     String name = "foo.googleapis.com";
 
-    DnsNameResolver resolver = newResolver(name, 81, isAndroid);
+    RetryingNameResolver resolver = newResolver(name, 81, isAndroid);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer1).thenReturn(answer2);
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -303,8 +328,9 @@ public class DnsNameResolverTest {
     verify(mockListener, times(2)).onResult(resultCaptor.capture());
     assertAnswerMatches(answer2, 81, resultCaptor.getValue());
     assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     verify(mockResolver, times(2)).resolveAddress(anyString());
   }
@@ -313,18 +339,20 @@ public class DnsNameResolverTest {
   public void testExecutor_default() throws Exception {
     final List<InetAddress> answer = createAddressList(2);
 
-    DnsNameResolver resolver = newResolver("foo.googleapis.com", 81);
+    RetryingNameResolver resolver = newResolver("foo.googleapis.com", 81);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer);
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onResult(resultCaptor.capture());
     assertAnswerMatches(answer, 81, resultCaptor.getValue());
     assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     assertThat(fakeExecutorResource.createCount.get()).isEqualTo(1);
   }
@@ -341,6 +369,7 @@ public class DnsNameResolverTest {
             .setSynchronizationContext(syncContext)
             .setServiceConfigParser(mock(ServiceConfigParser.class))
             .setChannelLogger(mock(ChannelLogger.class))
+            .setScheduledExecutorService(fakeExecutor.getScheduledExecutorService())
             .setOffloadExecutor(
                 new Executor() {
                   @Override
@@ -351,19 +380,21 @@ public class DnsNameResolverTest {
                 })
             .build();
 
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver("foo.googleapis.com", Stopwatch.createUnstarted(), false, args);
+        "foo.googleapis.com", Stopwatch.createUnstarted(), false, args);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer);
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(0, fakeExecutor.runDueTasks());
     verify(mockListener).onResult(resultCaptor.capture());
     assertAnswerMatches(answer, 81, resultCaptor.getValue());
     assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     assertThat(fakeExecutorResource.createCount.get()).isEqualTo(0);
     assertThat(executions.get()).isEqualTo(1);
@@ -376,13 +407,14 @@ public class DnsNameResolverTest {
     String name = "foo.googleapis.com";
     FakeTicker fakeTicker = new FakeTicker();
 
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString()))
         .thenReturn(answer1)
         .thenThrow(new AssertionError("should not called twice"));
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -396,7 +428,7 @@ public class DnsNameResolverTest {
     assertEquals(0, fakeClock.numPendingTasks());
     verifyNoMoreInteractions(mockListener);
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     verify(mockResolver).resolveAddress(anyString());
   }
@@ -409,13 +441,14 @@ public class DnsNameResolverTest {
     String name = "foo.googleapis.com";
     FakeTicker fakeTicker = new FakeTicker();
 
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString()))
         .thenReturn(answer)
         .thenThrow(new AssertionError("should not reach here."));
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -430,7 +463,7 @@ public class DnsNameResolverTest {
     assertEquals(0, fakeClock.numPendingTasks());
     verifyNoMoreInteractions(mockListener);
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     verify(mockResolver).resolveAddress(anyString());
   }
@@ -444,12 +477,13 @@ public class DnsNameResolverTest {
     String name = "foo.googleapis.com";
     FakeTicker fakeTicker = new FakeTicker();
 
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer1)
         .thenReturn(answer2);
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -463,8 +497,9 @@ public class DnsNameResolverTest {
     verify(mockListener, times(2)).onResult(resultCaptor.capture());
     assertAnswerMatches(answer2, 81, resultCaptor.getValue());
     assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     verify(mockResolver, times(2)).resolveAddress(anyString());
   }
@@ -487,11 +522,12 @@ public class DnsNameResolverTest {
     String name = "foo.googleapis.com";
     FakeTicker fakeTicker = new FakeTicker();
 
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer1).thenReturn(answer2);
-    resolver.setAddressResolver(mockResolver);
+    dnsResolver.setAddressResolver(mockResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -511,8 +547,9 @@ public class DnsNameResolverTest {
     verify(mockListener, times(2)).onResult(resultCaptor.capture());
     assertAnswerMatches(answer2, 81, resultCaptor.getValue());
     assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
 
-    resolver.shutdown();
+    syncContext.execute(() -> resolver.shutdown());
 
     verify(mockResolver, times(2)).resolveAddress(anyString());
   }
@@ -520,8 +557,9 @@ public class DnsNameResolverTest {
   @Test
   public void resolve_emptyResult() throws Exception {
     DnsNameResolver.enableTxt = true;
-    DnsNameResolver nr = newResolver("dns:///addr.fake:1234", 443);
+    RetryingNameResolver resolver = newResolver("dns:///addr.fake:1234", 443);
-    nr.setAddressResolver(new AddressResolver() {
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
+    dnsResolver.setAddressResolver(new AddressResolver() {
       @Override
       public List<InetAddress> resolveAddress(String host) throws Exception {
         return Collections.emptyList();
@@ -531,18 +569,60 @@ public class DnsNameResolverTest {
     when(mockResourceResolver.resolveTxt(anyString()))
         .thenReturn(Collections.<String>emptyList());
 
-    nr.setResourceResolver(mockResourceResolver);
+    dnsResolver.setResourceResolver(mockResourceResolver);
 
-    nr.start(mockListener);
+    resolver.start(mockListener);
     assertThat(fakeExecutor.runDueTasks()).isEqualTo(1);
 
     ArgumentCaptor<ResolutionResult> ac = ArgumentCaptor.forClass(ResolutionResult.class);
     verify(mockListener).onResult(ac.capture());
     verifyNoMoreInteractions(mockListener);
     assertThat(ac.getValue().getAddresses()).isEmpty();
-    assertThat(ac.getValue().getAttributes()).isEqualTo(Attributes.EMPTY);
     assertThat(ac.getValue().getServiceConfig()).isNull();
     verify(mockResourceResolver, never()).resolveSrv(anyString());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
+  }
+
+  // Load balancer rejects the empty addresses.
+  @Test
+  public void resolve_emptyResult_notAccepted() throws Exception {
+    doAnswer(invocation -> {
+      ResolutionResult result = invocation.getArgument(0);
+      result.getAttributes().get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)
+          .resolutionAttempted(false);
+      return null;
+    }).when(mockListener).onResult(isA(ResolutionResult.class));
+
+    DnsNameResolver.enableTxt = true;
+    RetryingNameResolver resolver = newResolver("dns:///addr.fake:1234", 443);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
+    dnsResolver.setAddressResolver(new AddressResolver() {
+      @Override
+      public List<InetAddress> resolveAddress(String host) throws Exception {
+        return Collections.emptyList();
+      }
+    });
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    when(mockResourceResolver.resolveTxt(anyString()))
+        .thenReturn(Collections.<String>emptyList());
+
+    dnsResolver.setResourceResolver(mockResourceResolver);
+
+    resolver.start(mockListener);
+    syncContext.execute(() -> assertThat(fakeExecutor.runDueTasks()).isEqualTo(1));
+
+    ArgumentCaptor<ResolutionResult> ac = ArgumentCaptor.forClass(ResolutionResult.class);
+    verify(mockListener).onResult(ac.capture());
+    verifyNoMoreInteractions(mockListener);
+    assertThat(ac.getValue().getAddresses()).isEmpty();
+    assertThat(ac.getValue().getServiceConfig()).isNull();
+    verify(mockResourceResolver, never()).resolveSrv(anyString());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    // A retry should be scheduled
+    assertThat(fakeExecutor.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER)).isEqualTo(1);
   }
 
   @Test
@@ -554,9 +634,10 @@ public class DnsNameResolverTest {
         .thenReturn(Collections.singletonList(backendAddr));
     String name = "foo.googleapis.com";
 
-    DnsNameResolver resolver = newResolver(name, 81);
+    RetryingNameResolver resolver = newResolver(name, 81);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(null);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(null);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onResult(resultCaptor.capture());
@@ -566,8 +647,10 @@ public class DnsNameResolverTest {
             Iterables.getOnlyElement(result.getAddresses()).getAddresses());
     assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
     verify(mockAddressResolver).resolveAddress(name);
-    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
     assertThat(result.getServiceConfig()).isNull();
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
   }
 
   @Test
@@ -578,15 +661,20 @@ public class DnsNameResolverTest {
         .thenThrow(new IOException("no addr"));
     String name = "foo.googleapis.com";
 
-    DnsNameResolver resolver = newResolver(name, 81);
+    RetryingNameResolver resolver = newResolver(name, 81);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(null);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(null);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onError(errorCaptor.capture());
     Status errorStatus = errorCaptor.getValue();
     assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
     assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    // A retry should be scheduled
+    assertThat(fakeExecutor.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER)).isEqualTo(1);
   }
 
   @Test
@@ -613,12 +701,14 @@ public class DnsNameResolverTest {
             .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR)
             .setSynchronizationContext(syncContext)
             .setServiceConfigParser(serviceConfigParser)
+            .setScheduledExecutorService(fakeClock.getScheduledExecutorService())
             .build();
 
     String name = "foo.googleapis.com";
-    DnsNameResolver resolver = newResolver(name, Stopwatch.createUnstarted(), false, args);
+    RetryingNameResolver resolver = newResolver(name, Stopwatch.createUnstarted(), false, args);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(mockResourceResolver);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(mockResourceResolver);
 
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
@@ -631,6 +721,9 @@ public class DnsNameResolverTest {
     assertThat(result.getServiceConfig().getConfig()).isNotNull();
     verify(mockAddressResolver).resolveAddress(name);
     verify(mockResourceResolver).resolveTxt("_grpc_config." + name);
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
   }
 
   @Test
@@ -642,9 +735,10 @@ public class DnsNameResolverTest {
     String name = "foo.googleapis.com";
 
     ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-    DnsNameResolver resolver = newResolver(name, 81);
+    RetryingNameResolver resolver = newResolver(name, 81);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver)resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(mockResourceResolver);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(mockResourceResolver);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onError(errorCaptor.capture());
@@ -652,6 +746,10 @@ public class DnsNameResolverTest {
     assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
     assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
     verify(mockResourceResolver, never()).resolveTxt(anyString());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    // A retry should be scheduled
+    assertThat(fakeExecutor.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER)).isEqualTo(1);
   }
 
   @Test
@@ -666,9 +764,10 @@ public class DnsNameResolverTest {
     when(mockResourceResolver.resolveTxt(anyString()))
         .thenThrow(new Exception("something like javax.naming.NamingException"));
 
-    DnsNameResolver resolver = newResolver(name, 81);
+    RetryingNameResolver resolver = newResolver(name, 81);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(mockResourceResolver);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(mockResourceResolver);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onResult(resultCaptor.capture());
@@ -678,9 +777,11 @@ public class DnsNameResolverTest {
             Iterables.getOnlyElement(result.getAddresses()).getAddresses());
     assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
     verify(mockAddressResolver).resolveAddress(name);
-    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
     assertThat(result.getServiceConfig()).isNull();
     verify(mockResourceResolver).resolveTxt(anyString());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
   }
 
   @Test
@@ -695,9 +796,10 @@ public class DnsNameResolverTest {
     when(mockResourceResolver.resolveTxt(anyString()))
         .thenReturn(Collections.singletonList("grpc_config=something invalid"));
 
-    DnsNameResolver resolver = newResolver(name, 81);
+    RetryingNameResolver resolver = newResolver(name, 81);
-    resolver.setAddressResolver(mockAddressResolver);
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
-    resolver.setResourceResolver(mockResourceResolver);
+    dnsResolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setResourceResolver(mockResourceResolver);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
     verify(mockListener).onResult(resultCaptor.capture());
@@ -707,10 +809,12 @@ public class DnsNameResolverTest {
             Iterables.getOnlyElement(result.getAddresses()).getAddresses());
     assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
     verify(mockAddressResolver).resolveAddress(name);
-    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
     assertThat(result.getServiceConfig()).isNotNull();
     assertThat(result.getServiceConfig().getError()).isNotNull();
     verify(mockResourceResolver).resolveTxt(anyString());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
   }
 
   @Test
@@ -757,11 +861,12 @@ public class DnsNameResolverTest {
               .setPassword("password").build();
         }
       };
-    DnsNameResolver resolver =
+    RetryingNameResolver resolver = newResolver(
-        newResolver(name, port, alwaysDetectProxy, Stopwatch.createUnstarted());
+        name, port, alwaysDetectProxy, Stopwatch.createUnstarted());
+    DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockAddressResolver = mock(AddressResolver.class);
     when(mockAddressResolver.resolveAddress(anyString())).thenThrow(new AssertionError());
-    resolver.setAddressResolver(mockAddressResolver);
+    dnsResolver.setAddressResolver(mockAddressResolver);
     resolver.start(mockListener);
     assertEquals(1, fakeExecutor.runDueTasks());
 
@@ -777,6 +882,9 @@ public class DnsNameResolverTest {
     assertEquals("username", socketAddress.getUsername());
     assertEquals("password", socketAddress.getPassword());
     assertTrue(socketAddress.getTargetAddress().isUnresolved());
+
+    assertEquals(0, fakeClock.numPendingTasks());
+    assertEquals(0, fakeExecutor.numPendingTasks());
   }
 
   @Test
@@ -1185,7 +1293,8 @@ public class DnsNameResolverTest {
   }
 
   private void testValidUri(URI uri, String exportedAuthority, int expectedPort) {
-    DnsNameResolver resolver = provider.newNameResolver(uri, args);
+    DnsNameResolver resolver = (DnsNameResolver) ((RetryingNameResolver) provider.newNameResolver(
+        uri, args)).getRetriedNameResolver();
     assertNotNull(resolver);
     assertEquals(expectedPort, resolver.getPort());
     assertEquals(exportedAuthority, resolver.getServiceAuthority());

core/src/test/java/io/grpc/internal/ForwardingNameResolverTest.java

@@ -81,7 +81,6 @@ public class ForwardingNameResolverTest {
     NameResolver.Listener2 listener = new NameResolver.Listener2() {
       @Override
       public void onResult(ResolutionResult result) {
-
       }
 
       @Override

core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java

@@ -43,6 +43,7 @@ public class ManagedChannelImplGetNameResolverTest {
       .setSynchronizationContext(new SynchronizationContext(mock(UncaughtExceptionHandler.class)))
       .setServiceConfigParser(mock(ServiceConfigParser.class))
       .setChannelLogger(mock(ChannelLogger.class))
+      .setScheduledExecutorService(new FakeClock().getScheduledExecutorService())
       .build();
 
   @Test

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -55,7 +55,6 @@ import static org.mockito.Mockito.when;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
@@ -205,14 +204,6 @@ public class ManagedChannelImplTest {
   private final FakeClock timer = new FakeClock();
   private final FakeClock executor = new FakeClock();
   private final FakeClock balancerRpcExecutor = new FakeClock();
-  private static final FakeClock.TaskFilter NAME_RESOLVER_REFRESH_TASK_FILTER =
-      new FakeClock.TaskFilter() {
-        @Override
-        public boolean shouldAccept(Runnable command) {
-          return command.toString().contains(
-              ManagedChannelImpl.DelayedNameResolverRefresh.class.getName());
-        }
-      };
 
   private final InternalChannelz channelz = new InternalChannelz();
 
@@ -309,10 +300,6 @@ public class ManagedChannelImplTest {
         numExpectedTasks += 1;
       }
 
-      if (getNameResolverRefresh() != null) {
-        numExpectedTasks += 1;
-      }
-
       assertEquals(numExpectedTasks, timer.numPendingTasks());
 
       ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
@@ -1062,139 +1049,6 @@ public class ManagedChannelImplTest {
         TimeUnit.SECONDS.toNanos(ManagedChannelImpl.SUBCHANNEL_SHUTDOWN_DELAY_SECONDS));
   }
 
-  @Test
-  public void nameResolutionFailed() {
-    Status error = Status.UNAVAILABLE.withCause(new Throwable("fake name resolution error"));
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri)
-            .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
-            .setError(error)
-            .build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    // Name resolution is started as soon as channel is created.
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver resolver = nameResolverFactory.resolvers.get(0);
-    verify(mockLoadBalancer).handleNameResolutionError(same(error));
-    assertEquals(1, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER));
-
-    timer.forwardNanos(RECONNECT_BACKOFF_INTERVAL_NANOS - 1);
-    assertEquals(0, resolver.refreshCalled);
-
-    timer.forwardNanos(1);
-    assertEquals(1, resolver.refreshCalled);
-    verify(mockLoadBalancer, times(2)).handleNameResolutionError(same(error));
-
-    // Verify an additional name resolution failure does not schedule another timer
-    resolver.refresh();
-    verify(mockLoadBalancer, times(3)).handleNameResolutionError(same(error));
-    assertEquals(1, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER));
-
-    // Allow the next refresh attempt to succeed
-    resolver.error = null;
-
-    // For the second attempt, the backoff should occur at RECONNECT_BACKOFF_INTERVAL_NANOS * 2
-    timer.forwardNanos(RECONNECT_BACKOFF_INTERVAL_NANOS * 2 - 1);
-    assertEquals(2, resolver.refreshCalled);
-    timer.forwardNanos(1);
-    assertEquals(3, resolver.refreshCalled);
-    assertEquals(0, timer.numPendingTasks());
-
-    // Verify that the successful resolution reset the backoff policy
-    resolver.listener.onError(error);
-    timer.forwardNanos(RECONNECT_BACKOFF_INTERVAL_NANOS - 1);
-    assertEquals(3, resolver.refreshCalled);
-    timer.forwardNanos(1);
-    assertEquals(4, resolver.refreshCalled);
-    assertEquals(0, timer.numPendingTasks());
-  }
-
-  @Test
-  public void nameResolutionFailed_delayedTransportShutdownCancelsBackoff() {
-    Status error = Status.UNAVAILABLE.withCause(new Throwable("fake name resolution error"));
-
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri).setError(error).build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    // Name resolution is started as soon as channel is created.
-    createChannel();
-    verify(mockLoadBalancer).handleNameResolutionError(same(error));
-
-    FakeClock.ScheduledTask nameResolverBackoff = getNameResolverRefresh();
-    assertNotNull(nameResolverBackoff);
-    assertFalse(nameResolverBackoff.isCancelled());
-
-    // Add a pending call to the delayed transport
-    ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
-    Metadata headers = new Metadata();
-    call.start(mockCallListener, headers);
-
-    // The pending call on the delayed transport stops the name resolver backoff from cancelling
-    channel.shutdown();
-    assertFalse(nameResolverBackoff.isCancelled());
-
-    // Notify that a subchannel is ready, which drains the delayed transport
-    SubchannelPicker picker = mock(SubchannelPicker.class);
-    Status status = Status.UNAVAILABLE.withDescription("for test");
-    when(picker.pickSubchannel(any(PickSubchannelArgs.class)))
-        .thenReturn(PickResult.withDrop(status));
-    updateBalancingStateSafely(helper, READY, picker);
-    executor.runDueTasks();
-    verify(mockCallListener).onClose(same(status), any(Metadata.class));
-
-    assertTrue(nameResolverBackoff.isCancelled());
-  }
-
-  @Test
-  public void nameResolverReturnsEmptySubLists_resolutionRetry() throws Exception {
-    // The mock LB is set to reject the addresses.
-    when(mockLoadBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(false);
-
-    // Pass a FakeNameResolverFactory with an empty list and LB config
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri).build();
-    Map<String, Object> rawServiceConfig =
-        parseConfig("{\"loadBalancingConfig\": [ {\"mock_lb\": { \"setting1\": \"high\" } } ] }");
-    ManagedChannelServiceConfig parsedServiceConfig =
-        createManagedChannelServiceConfig(rawServiceConfig, null);
-    nameResolverFactory.nextConfigOrError.set(ConfigOrError.fromConfig(parsedServiceConfig));
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-
-    // A resolution retry has been scheduled
-    assertEquals(1, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER));
-  }
-
-  @Test
-  public void nameResolverReturnsEmptySubLists_optionallyAllowed() throws Exception {
-    // Pass a FakeNameResolverFactory with an empty list and LB config
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri).build();
-    String rawLbConfig = "{ \"setting1\": \"high\" }";
-    Object parsedLbConfig = new Object();
-    Map<String, Object> rawServiceConfig =
-        parseConfig("{\"loadBalancingConfig\": [ {\"mock_lb\": " + rawLbConfig + " } ] }");
-    ManagedChannelServiceConfig parsedServiceConfig =
-        createManagedChannelServiceConfig(
-            rawServiceConfig,
-            new PolicySelection(
-                mockLoadBalancerProvider,
-                parsedLbConfig));
-    nameResolverFactory.nextConfigOrError.set(ConfigOrError.fromConfig(parsedServiceConfig));
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-
-    // LoadBalancer received the empty list and the LB config
-    verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class));
-    ArgumentCaptor<ResolvedAddresses> resultCaptor =
-        ArgumentCaptor.forClass(ResolvedAddresses.class);
-    verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture());
-    assertThat(resultCaptor.getValue().getAddresses()).isEmpty();
-    assertThat(resultCaptor.getValue().getLoadBalancingPolicyConfig()).isEqualTo(parsedLbConfig);
-
-    // A no resolution retry
-    assertEquals(0, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER));
-  }
-
   @Test
   public void loadBalancerThrowsInHandleResolvedAddresses() {
     RuntimeException ex = new RuntimeException("simulated");
@@ -3019,52 +2873,6 @@ public class ManagedChannelImplTest {
     assertEquals(initialRefreshCount + 1, resolver.refreshCalled);
   }
 
-  @Test
-  public void resetConnectBackoff() {
-    // Start with a name resolution failure to trigger backoff attempts
-    Status error = Status.UNAVAILABLE.withCause(new Throwable("fake name resolution error"));
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri).setError(error).build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    // Name resolution is started as soon as channel is created.
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver resolver = nameResolverFactory.resolvers.get(0);
-    verify(mockLoadBalancer).handleNameResolutionError(same(error));
-
-    FakeClock.ScheduledTask nameResolverBackoff = getNameResolverRefresh();
-    assertNotNull("There should be a name resolver backoff task", nameResolverBackoff);
-    assertEquals(0, resolver.refreshCalled);
-
-    // Verify resetConnectBackoff() calls refresh and cancels the scheduled backoff
-    channel.resetConnectBackoff();
-    assertEquals(1, resolver.refreshCalled);
-    assertTrue(nameResolverBackoff.isCancelled());
-
-    // Simulate a race between cancel and the task scheduler. Should be a no-op.
-    nameResolverBackoff.command.run();
-    assertEquals(1, resolver.refreshCalled);
-
-    // Verify that the reconnect policy was recreated and the backoff multiplier reset to 1
-    timer.forwardNanos(RECONNECT_BACKOFF_INTERVAL_NANOS);
-    assertEquals(2, resolver.refreshCalled);
-  }
-
-  @Test
-  public void resetConnectBackoff_noOpWithoutPendingResolverBackoff() {
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri)
-            .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
-            .build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver nameResolver = nameResolverFactory.resolvers.get(0);
-    assertEquals(0, nameResolver.refreshCalled);
-
-    channel.resetConnectBackoff();
-
-    assertEquals(0, nameResolver.refreshCalled);
-  }
-
   @Test
   public void resetConnectBackoff_noOpWhenChannelShutdown() {
     FakeNameResolverFactory nameResolverFactory =
@@ -4517,10 +4325,6 @@ public class ManagedChannelImplTest {
     return instrumented.getStats().get();
   }
 
-  private FakeClock.ScheduledTask getNameResolverRefresh() {
-    return Iterables.getOnlyElement(timer.getPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER), null);
-  }
-
   // Helper methods to call methods from SynchronizationContext
   private static Subchannel createSubchannelSafely(
       final Helper helper, final EquivalentAddressGroup addressGroup, final Attributes attrs,

core/src/test/java/io/grpc/internal/RetryingNameResolverTest.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright 2023 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import io.grpc.NameResolver;
+import io.grpc.NameResolver.Listener2;
+import io.grpc.NameResolver.ResolutionResult;
+import io.grpc.Status;
+import io.grpc.SynchronizationContext;
+import io.grpc.internal.RetryingNameResolver.ResolutionResultListener;
+import java.lang.Thread.UncaughtExceptionHandler;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+/**
+ * Unit test for {@link RetryingNameResolver}.
+ */
+@RunWith(JUnit4.class)
+public class RetryingNameResolverTest {
+
+  @Rule
+  public final MockitoRule mocks = MockitoJUnit.rule();
+
+  @Mock
+  private NameResolver mockNameResolver;
+  @Mock
+  private Listener2 mockListener;
+  @Mock
+  private RetryScheduler mockRetryScheduler;
+  @Captor
+  private ArgumentCaptor<Listener2> listenerCaptor;
+  @Captor
+  private ArgumentCaptor<ResolutionResult> onResultCaptor;
+  private final SynchronizationContext syncContext = new SynchronizationContext(
+      mock(UncaughtExceptionHandler.class));
+
+  private RetryingNameResolver retryingNameResolver;
+
+  @Before
+  public void setup() {
+    retryingNameResolver = new RetryingNameResolver(mockNameResolver, mockRetryScheduler,
+        syncContext);
+  }
+
+  @Test
+  public void startAndShutdown() {
+    retryingNameResolver.start(mockListener);
+    retryingNameResolver.shutdown();
+  }
+
+  // Make sure the ResolutionResultListener callback is added to the ResolutionResult attributes,
+  // and the retry scheduler is reset since the name resolution was successful.
+  @Test
+  public void onResult_sucess() {
+    retryingNameResolver.start(mockListener);
+    verify(mockNameResolver).start(listenerCaptor.capture());
+
+    listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
+    verify(mockListener).onResult(onResultCaptor.capture());
+    ResolutionResultListener resolutionResultListener = onResultCaptor.getValue()
+        .getAttributes()
+        .get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
+    assertThat(resolutionResultListener).isNotNull();
+
+    resolutionResultListener.resolutionAttempted(true);
+    verify(mockRetryScheduler).reset();
+  }
+
+  // Make sure the ResolutionResultListener callback is added to the ResolutionResult attributes,
+  // and that a retry gets scheduled when the resolution results are rejected.
+  @Test
+  public void onResult_failure() {
+    retryingNameResolver.start(mockListener);
+    verify(mockNameResolver).start(listenerCaptor.capture());
+
+    listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
+    verify(mockListener).onResult(onResultCaptor.capture());
+    ResolutionResultListener resolutionResultListener = onResultCaptor.getValue()
+        .getAttributes()
+        .get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
+    assertThat(resolutionResultListener).isNotNull();
+
+    resolutionResultListener.resolutionAttempted(false);
+    verify(mockRetryScheduler).schedule(isA(Runnable.class));
+  }
+
+  // Wrapping a NameResolver more than once is a misconfiguration.
+  @Test
+  public void onResult_failure_doubleWrapped() {
+    NameResolver doubleWrappedResolver = new RetryingNameResolver(retryingNameResolver,
+        mockRetryScheduler, syncContext);
+
+    doubleWrappedResolver.start(mockListener);
+    verify(mockNameResolver).start(listenerCaptor.capture());
+
+    try {
+      listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessageThat().contains("can only be used once");
+      return;
+    }
+    fail("An exception should have been thrown for a double wrapped NAmeResolver");
+  }
+
+  // A retry should get scheduled when name resolution fails.
+  @Test
+  public void onError() {
+    retryingNameResolver.start(mockListener);
+    verify(mockNameResolver).start(listenerCaptor.capture());
+    listenerCaptor.getValue().onError(Status.DEADLINE_EXCEEDED);
+    verify(mockListener).onError(Status.DEADLINE_EXCEEDED);
+    verify(mockRetryScheduler).schedule(isA(Runnable.class));
+  }
+}

core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java

@@ -98,7 +98,7 @@ public class ServiceConfigErrorHandlingTest {
         @Override
         public boolean shouldAccept(Runnable command) {
           return command.toString().contains(
-              ManagedChannelImpl.DelayedNameResolverRefresh.class.getName());
+              RetryingNameResolver.DelayedNameResolverRefresh.class.getName());
         }
       };
 
@@ -542,7 +542,7 @@ public class ServiceConfigErrorHandlingTest {
     final URI expectedUri;
     final List<EquivalentAddressGroup> servers;
     final boolean resolvedAtStart;
-    final ArrayList<FakeNameResolver> resolvers = new ArrayList<>();
+    final ArrayList<RetryingNameResolver> resolvers = new ArrayList<>();
     final AtomicReference<Map<String, ?>> nextRawServiceConfig = new AtomicReference<>();
     final AtomicReference<Attributes> nextAttributes = new AtomicReference<>(Attributes.EMPTY);
 
@@ -561,7 +561,13 @@ public class ServiceConfigErrorHandlingTest {
         return null;
       }
       assertEquals(DEFAULT_PORT, args.getDefaultPort());
-      FakeNameResolver resolver = new FakeNameResolver(args.getServiceConfigParser());
+      RetryingNameResolver resolver = new RetryingNameResolver(
+          new FakeNameResolver(args.getServiceConfigParser()),
+          new BackoffPolicyRetryScheduler(
+              new FakeBackoffPolicyProvider(),
+              args.getScheduledExecutorService(),
+              args.getSynchronizationContext()),
+          args.getSynchronizationContext());
       resolvers.add(resolver);
       return resolver;
     }
@@ -572,8 +578,8 @@ public class ServiceConfigErrorHandlingTest {
     }
 
     void allResolved() {
-      for (FakeNameResolver resolver : resolvers) {
+      for (RetryingNameResolver resolver : resolvers) {
-        resolver.resolved();
+        ((FakeNameResolver)resolver.getRetriedNameResolver()).resolved();
       }
     }
 
@@ -647,7 +653,8 @@ public class ServiceConfigErrorHandlingTest {
   }
 
   private FakeClock.ScheduledTask getNameResolverRefresh() {
-    return Iterables.getOnlyElement(timer.getPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER), null);
+    return Iterables.getOnlyElement(
+       timer.getPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER), null);
   }
 
   private static class FakeLoadBalancer extends LoadBalancer {