core: Enable dns "caching" on Android

DnsNameResolver discards refresh requests if it has been too soon after
the last refresh, because the result is assumed to be identical to the
previous fetch. Android itself will adhere to the RR's TTL, so
requesting too frequently shouldn't have been causing too much I/O, but
it could be causing extra CPU usage. Having some lower limit will reduce
the number of useless address updates into the LB tree.

30 seconds is the same as regular Java and Go/C++ (which copied Java as
a "reasonable" value). Note that other languages _delay_ the refresh
instead of _discarding_ the refresh, but there's no reason why the
existing discard behavior would cause much problem on Android vs normal
Java. Chrome apparently uses 1 minute, so this really looks like it
shouldn't cause problems as long as AndroidChannelBuilder is being used.

Author: ejona86

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

@@ -100,7 +100,7 @@ public class DnsNameResolver extends NameResolver {
    * not installed, the ttl value is {@code null} which falls back to {@link
    * #DEFAULT_NETWORK_CACHE_TTL_SECONDS gRPC default value}.
    *
-   * <p>For android, gRPC doesn't attempt to cache; this property value will be ignored.
+   * <p>For android, gRPC uses a fixed value; this property value will be ignored.
    */
   @VisibleForTesting
   static final String NETWORKADDRESS_CACHE_TTL_PROPERTY = "networkaddress.cache.ttl";
@@ -451,12 +451,14 @@ private static final List<String> getHostnamesFromChoice(Map<String, ?> serviceC
 
   /**
    * Returns value of network address cache ttl property if not Android environment. For android,
-   * DnsNameResolver does not cache the dns lookup result.
+   * DnsNameResolver uses a fixed value.
    */
   private static long getNetworkAddressCacheTtlNanos(boolean isAndroid) {
     if (isAndroid) {
-      // on Android, ignore dns cache.
-      return 0;
+      // On Android, use fixed value. If the network used changes this value shouldn't matter, as
+      // channel.enterIdle() should be called and this name resolver instance will be discarded. The
+      // new name resolver instance will then re-request.
+      return TimeUnit.SECONDS.toNanos(DEFAULT_NETWORK_CACHE_TTL_SECONDS);
     }
 
     String cacheTtlPropertyValue = System.getProperty(NETWORKADDRESS_CACHE_TTL_PROPERTY);

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

@@ -151,13 +151,6 @@ private RetryingNameResolver newResolver(String name, int defaultPort) {
         name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted());
   }
 
-  private RetryingNameResolver newResolver(String name, int defaultPort, boolean isAndroid) {
-    return newResolver(
-        name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(),
-        isAndroid);
-  }
-
-
   private RetryingNameResolver newResolver(
       String name,
       int defaultPort,
@@ -227,30 +220,14 @@ public void invalidDnsName_containsUnderscore() {
     }
   }
 
-  @Test
-  public void resolve_androidIgnoresPropertyValue() throws Exception {
-    flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "2");
-    resolveNeverCache(true);
-  }
-
-  @Test
-  public void resolve_androidIgnoresPropertyValueCacheForever() throws Exception {
-    flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1");
-    resolveNeverCache(true);
-  }
-
   @Test
   public void resolve_neverCache() throws Exception {
     flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "0");
-    resolveNeverCache(false);
-  }
-
-  private void resolveNeverCache(boolean isAndroid) throws Exception {
     final List<InetAddress> answer1 = createAddressList(2);
     final List<InetAddress> answer2 = createAddressList(1);
     String name = "foo.googleapis.com";
 
-    RetryingNameResolver resolver = newResolver(name, 81, isAndroid);
+    RetryingNameResolver resolver = newResolver(name, 81);
     DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer1).thenReturn(answer2);
@@ -443,26 +420,38 @@ public void resolve_cacheExpired() throws Exception {
     verify(mockResolver, times(2)).resolveAddress(anyString());
   }
 
+  @Test
+  public void resolve_androidIgnoresPropertyValue() throws Exception {
+    flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "2");
+    resolveDefaultValue(true);
+  }
+
+  @Test
+  public void resolve_androidIgnoresPropertyValueCacheForever() throws Exception {
+    flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1");
+    resolveDefaultValue(true);
+  }
+
   @Test
   public void resolve_invalidTtlPropertyValue() throws Exception {
     flagResetRule.setSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number");
-    resolveDefaultValue();
+    resolveDefaultValue(false);
   }
 
   @Test
   public void resolve_noPropertyValue() throws Exception {
     flagResetRule.clearSystemPropertyForTest(NETWORKADDRESS_CACHE_TTL_PROPERTY);
-    resolveDefaultValue();
+    resolveDefaultValue(false);
   }
 
-  private void resolveDefaultValue() throws Exception {
+  private void resolveDefaultValue(boolean isAndroid) throws Exception {
     final List<InetAddress> answer1 = createAddressList(2);
     final List<InetAddress> answer2 = createAddressList(1);
     String name = "foo.googleapis.com";
     FakeTicker fakeTicker = new FakeTicker();
 
     RetryingNameResolver resolver = newResolver(
-        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
+        name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker), isAndroid);
     DnsNameResolver dnsResolver = (DnsNameResolver) resolver.getRetriedNameResolver();
     AddressResolver mockResolver = mock(AddressResolver.class);
     when(mockResolver.resolveAddress(anyString())).thenReturn(answer1).thenReturn(answer2);