core: DnsNameResolver caches refresh (#4812)

DnsNameResolver caches refresh using java security property networkaddress.cache.ttl.

Resolves #4745

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

@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
 import com.google.common.base.Verify;
 import io.grpc.Attributes;
 import io.grpc.EquivalentAddressGroup;
@@ -42,6 +43,7 @@ import java.util.Map.Entry;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -88,6 +90,19 @@ final class DnsNameResolver extends NameResolver {
   private static final String JNDI_TXT_PROPERTY =
       System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_service_config", "false");
 
+  /**
+   * Java networking system properties name for caching DNS result.
+   *
+   * <p>Default value is -1 (cache forever) if security manager is installed. If security manager is
+   * not installed, the ttl value is {@code null} which falls back to {@link
+   * #DEFAULT_NETWORK_CACHE_TTL_SECONDS gRPC default value}.
+   */
+  @VisibleForTesting
+  static final String NETWORKADDRESS_CACHE_TTL_PROPERTY = "networkaddress.cache.ttl";
+  /** Default DNS cache duration if network cache ttl value is not specified ({@code null}). */
+  @VisibleForTesting
+  static final long DEFAULT_NETWORK_CACHE_TTL_SECONDS = 30;
+
   @VisibleForTesting
   static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY);
   @VisibleForTesting
@@ -115,6 +130,8 @@ final class DnsNameResolver extends NameResolver {
   private final String host;
   private final int port;
   private final Resource<ExecutorService> executorResource;
+  private final long networkAddressCacheTtlNanos;
+  private final Stopwatch stopwatch;
   @GuardedBy("this")
   private boolean shutdown;
   @GuardedBy("this")
@@ -123,10 +140,11 @@ final class DnsNameResolver extends NameResolver {
   private boolean resolving;
   @GuardedBy("this")
   private Listener listener;
+  private ResolutionResults cachedResolutionResults;
 
   DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params,
-      Resource<ExecutorService> executorResource,
+      Resource<ExecutorService> executorResource, ProxyDetector proxyDetector,
-      ProxyDetector proxyDetector) {
+      Stopwatch stopwatch) {
     // TODO: if a DNS server is provided as nsAuthority, use it.
     // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
     this.executorResource = executorResource;
@@ -149,6 +167,8 @@ final class DnsNameResolver extends NameResolver {
       port = nameUri.getPort();
     }
     this.proxyDetector = proxyDetector;
+    this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch");
+    this.networkAddressCacheTtlNanos = getNetworkAddressCacheTtlNanos();
   }
 
   @Override
@@ -178,6 +198,13 @@ final class DnsNameResolver extends NameResolver {
           if (shutdown) {
             return;
           }
+          boolean resourceRefreshRequired = cachedResolutionResults == null
+              || networkAddressCacheTtlNanos == 0
+              || (networkAddressCacheTtlNanos > 0
+                  && stopwatch.elapsed(TimeUnit.NANOSECONDS) > networkAddressCacheTtlNanos);
+          if (!resourceRefreshRequired) {
+            return;
+          }
           savedListener = listener;
           resolving = true;
         }
@@ -207,6 +234,10 @@ final class DnsNameResolver extends NameResolver {
             }
             resolutionResults =
                 resolveAll(addressResolver, resourceResolver, enableSrv, enableTxt, host);
+            cachedResolutionResults = resolutionResults;
+            if (networkAddressCacheTtlNanos > 0) {
+              stopwatch.reset().start();
+            }
           } catch (Exception e) {
             savedListener.onError(
                 Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));
@@ -253,6 +284,23 @@ final class DnsNameResolver extends NameResolver {
       }
     };
 
+  /** Returns value of network address cache ttl property. */
+  private static long getNetworkAddressCacheTtlNanos() {
+    String cacheTtlPropertyValue = System.getProperty(NETWORKADDRESS_CACHE_TTL_PROPERTY);
+    long cacheTtl = DEFAULT_NETWORK_CACHE_TTL_SECONDS;
+    if (cacheTtlPropertyValue != null) {
+      try {
+        cacheTtl = Long.parseLong(cacheTtlPropertyValue);
+      } catch (NumberFormatException e) {
+        logger.log(
+            Level.WARNING,
+            "Property({0}) valid is not valid number format({1}), fall back to default({2})",
+            new Object[] {NETWORKADDRESS_CACHE_TTL_PROPERTY, cacheTtlPropertyValue, cacheTtl});
+      }
+    }
+    return cacheTtl > 0 ? TimeUnit.SECONDS.toNanos(cacheTtl) : cacheTtl;
+  }
+
   @GuardedBy("this")
   private void resolve() {
     if (resolving || shutdown) {

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

@@ -17,6 +17,7 @@
 package io.grpc.internal;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
 import io.grpc.Attributes;
 import io.grpc.NameResolverProvider;
 import java.net.URI;
@@ -52,7 +53,8 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
           name,
           params,
           GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-          GrpcUtil.getDefaultProxyDetector());
+          GrpcUtil.getDefaultProxyDetector(),
+          Stopwatch.createUnstarted());
     } else {
       return null;
     }

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

@@ -27,10 +27,13 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
+import com.google.common.base.Stopwatch;
 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.EquivalentAddressGroup;
 import io.grpc.NameResolver;
@@ -52,6 +55,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import javax.annotation.Nullable;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -103,21 +108,25 @@ public class DnsNameResolverTest {
   private NameResolver.Listener mockListener;
   @Captor
   private ArgumentCaptor<List<EquivalentAddressGroup>> resultCaptor;
+  @Nullable
+  private String networkaddressCacheTtlPropertyValue;
 
   private DnsNameResolver newResolver(String name, int port) {
-    return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR);
+    return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted());
   }
 
   private DnsNameResolver newResolver(
       String name,
       int port,
-      ProxyDetector proxyDetector) {
+      ProxyDetector proxyDetector,
+      Stopwatch stopwatch) {
     DnsNameResolver dnsResolver = new DnsNameResolver(
         null,
         name,
         Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(),
         fakeExecutorResource,
-        proxyDetector);
+        proxyDetector,
+        stopwatch);
     return dnsResolver;
   }
 
@@ -125,6 +134,19 @@ public class DnsNameResolverTest {
   public void setUp() {
     MockitoAnnotations.initMocks(this);
     DnsNameResolver.enableJndi = true;
+    networkaddressCacheTtlPropertyValue =
+        System.getProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
+  }
+
+  @After
+  public void restoreSystemProperty() {
+    if (networkaddressCacheTtlPropertyValue == null) {
+      System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
+    } else {
+      System.setProperty(
+          DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY,
+          networkaddressCacheTtlPropertyValue);
+    }
   }
 
   @After
@@ -176,7 +198,8 @@ public class DnsNameResolverTest {
   }
 
   @Test
-  public void resolve() throws Exception {
+  public void resolve_neverCache() throws Exception {
+    System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "0");
     final List<InetAddress> answer1 = createAddressList(2);
     final List<InetAddress> answer2 = createAddressList(1);
     String name = "foo.googleapis.com";
@@ -203,6 +226,156 @@ public class DnsNameResolverTest {
     verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
   }
 
+  @Test
+  public void resolve_cacheForever() throws Exception {
+    System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1");
+    final List<InetAddress> answer1 = createAddressList(2);
+    String name = "foo.googleapis.com";
+    FakeTicker fakeTicker = new FakeTicker();
+
+    DnsNameResolver resolver =
+        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    AddressResolver mockResolver = mock(AddressResolver.class);
+    when(mockResolver.resolveAddress(Matchers.anyString()))
+        .thenReturn(answer1)
+        .thenThrow(new AssertionError("should not called twice"));
+    resolver.setAddressResolver(mockResolver);
+
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer1, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    fakeTicker.advance(1, TimeUnit.DAYS);
+    resolver.refresh();
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verifyNoMoreInteractions(mockListener);
+    assertAnswerMatches(answer1, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    resolver.shutdown();
+
+    verify(mockResolver).resolveAddress(Matchers.anyString());
+  }
+
+  @Test
+  public void resolve_usingCache() throws Exception {
+    long ttl = 60;
+    System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
+    final List<InetAddress> answer = createAddressList(2);
+    String name = "foo.googleapis.com";
+    FakeTicker fakeTicker = new FakeTicker();
+
+    DnsNameResolver resolver =
+        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    AddressResolver mockResolver = mock(AddressResolver.class);
+    when(mockResolver.resolveAddress(Matchers.anyString()))
+        .thenReturn(answer)
+        .thenThrow(new AssertionError("should not reach here."));
+    resolver.setAddressResolver(mockResolver);
+
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    // this refresh should return cached result
+    fakeTicker.advance(ttl - 1, TimeUnit.SECONDS);
+    resolver.refresh();
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verifyNoMoreInteractions(mockListener);
+    assertAnswerMatches(answer, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    resolver.shutdown();
+
+    verify(mockResolver).resolveAddress(Matchers.anyString());
+  }
+
+  @Test
+  public void resolve_cacheExpired() throws Exception {
+    long ttl = 60;
+    System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
+    final List<InetAddress> answer1 = createAddressList(2);
+    final List<InetAddress> answer2 = createAddressList(1);
+    String name = "foo.googleapis.com";
+    FakeTicker fakeTicker = new FakeTicker();
+
+    DnsNameResolver resolver =
+        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    AddressResolver mockResolver = mock(AddressResolver.class);
+    when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
+    resolver.setAddressResolver(mockResolver);
+
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer1, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    fakeTicker.advance(ttl + 1, TimeUnit.SECONDS);
+    resolver.refresh();
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer2, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    resolver.shutdown();
+
+    verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
+  }
+
+  @Test
+  public void resolve_invalidTtlPropertyValue() throws Exception {
+    System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number");
+    resolveDefaultValue();
+  }
+
+  @Test
+  public void resolve_noPropertyValue() throws Exception {
+    System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
+    resolveDefaultValue();
+  }
+
+  private void resolveDefaultValue() throws Exception {
+    final List<InetAddress> answer1 = createAddressList(2);
+    final List<InetAddress> answer2 = createAddressList(1);
+    String name = "foo.googleapis.com";
+    FakeTicker fakeTicker = new FakeTicker();
+
+    DnsNameResolver resolver =
+        newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+    AddressResolver mockResolver = mock(AddressResolver.class);
+    when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
+    resolver.setAddressResolver(mockResolver);
+
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer1, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    fakeTicker.advance(DnsNameResolver.DEFAULT_NETWORK_CACHE_TTL_SECONDS, TimeUnit.SECONDS);
+    resolver.refresh();
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verifyNoMoreInteractions(mockListener);
+    assertAnswerMatches(answer1, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    fakeTicker.advance(1, TimeUnit.SECONDS);
+    resolver.refresh();
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class));
+    assertAnswerMatches(answer2, 81, resultCaptor.getValue());
+    assertEquals(0, fakeClock.numPendingTasks());
+
+    resolver.shutdown();
+
+    verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
+  }
+
   @Test
   public void resolveAll_nullResourceResolver() throws Exception {
     final String hostname = "addr.fake";
@@ -329,7 +502,8 @@ public class DnsNameResolverTest {
         "password");
     when(alwaysDetectProxy.proxyFor(any(SocketAddress.class)))
         .thenReturn(proxyParameters);
-    DnsNameResolver resolver = newResolver(name, port, alwaysDetectProxy);
+    DnsNameResolver resolver =
+        newResolver(name, port, alwaysDetectProxy, Stopwatch.createUnstarted());
     AddressResolver mockAddressResolver = mock(AddressResolver.class);
     when(mockAddressResolver.resolveAddress(Matchers.anyString())).thenThrow(new AssertionError());
     resolver.setAddressResolver(mockAddressResolver);