Revert "core: android use smaller(2s) DNS cache TTL (#4943)"

This reverts commit ecb206f277.
This commit is contained in:
Jihun Cho 2018-10-12 15:10:07 -07:00 committed by creamsoup
parent ecb206f277
commit c24f2fd25b
3 changed files with 17 additions and 111 deletions

View File

@ -95,16 +95,13 @@ final class DnsNameResolver extends NameResolver {
* *
* <p>Default value is -1 (cache forever) if security manager is installed. If security manager is * <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 * not installed, the ttl value is {@code null} which falls back to {@link
* #DEFAULT_NETWORK_CACHE_TTL_SECONDS gRPC default value} or {@link * #DEFAULT_NETWORK_CACHE_TTL_SECONDS gRPC default value}.
* #DEFAULT_ANDROID_NETWORK_CACHE_TTL_SECONDS android default value}.
*/ */
@VisibleForTesting @VisibleForTesting
static final String NETWORKADDRESS_CACHE_TTL_PROPERTY = "networkaddress.cache.ttl"; static final String NETWORKADDRESS_CACHE_TTL_PROPERTY = "networkaddress.cache.ttl";
/** Default DNS cache duration if network cache ttl value is not specified ({@code null}). */ /** Default DNS cache duration if network cache ttl value is not specified ({@code null}). */
@VisibleForTesting @VisibleForTesting
static final long DEFAULT_NETWORK_CACHE_TTL_SECONDS = 30; static final long DEFAULT_NETWORK_CACHE_TTL_SECONDS = 30;
@VisibleForTesting
static final long DEFAULT_ANDROID_NETWORK_CACHE_TTL_SECONDS = 2;
@VisibleForTesting @VisibleForTesting
static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY); static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY);
@ -147,7 +144,7 @@ final class DnsNameResolver extends NameResolver {
DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params, DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params,
Resource<ExecutorService> executorResource, ProxyDetector proxyDetector, Resource<ExecutorService> executorResource, ProxyDetector proxyDetector,
Stopwatch stopwatch, boolean isAndroid) { Stopwatch stopwatch) {
// TODO: if a DNS server is provided as nsAuthority, use it. // TODO: if a DNS server is provided as nsAuthority, use it.
// https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
this.executorResource = executorResource; this.executorResource = executorResource;
@ -171,7 +168,7 @@ final class DnsNameResolver extends NameResolver {
} }
this.proxyDetector = proxyDetector; this.proxyDetector = proxyDetector;
this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch"); this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch");
this.networkAddressCacheTtlNanos = getNetworkAddressCacheTtlNanos(isAndroid); this.networkAddressCacheTtlNanos = getNetworkAddressCacheTtlNanos();
} }
@Override @Override
@ -288,10 +285,9 @@ final class DnsNameResolver extends NameResolver {
}; };
/** Returns value of network address cache ttl property. */ /** Returns value of network address cache ttl property. */
private static long getNetworkAddressCacheTtlNanos(boolean isAndroid) { private static long getNetworkAddressCacheTtlNanos() {
String cacheTtlPropertyValue = System.getProperty(NETWORKADDRESS_CACHE_TTL_PROPERTY); String cacheTtlPropertyValue = System.getProperty(NETWORKADDRESS_CACHE_TTL_PROPERTY);
long cacheTtl = long cacheTtl = DEFAULT_NETWORK_CACHE_TTL_SECONDS;
isAndroid ? DEFAULT_ANDROID_NETWORK_CACHE_TTL_SECONDS : DEFAULT_NETWORK_CACHE_TTL_SECONDS;
if (cacheTtlPropertyValue != null) { if (cacheTtlPropertyValue != null) {
try { try {
cacheTtl = Long.parseLong(cacheTtlPropertyValue); cacheTtl = Long.parseLong(cacheTtlPropertyValue);

View File

@ -19,7 +19,6 @@ package io.grpc.internal;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.base.Stopwatch; import com.google.common.base.Stopwatch;
import io.grpc.Attributes; import io.grpc.Attributes;
import io.grpc.InternalServiceProviders;
import io.grpc.NameResolverProvider; import io.grpc.NameResolverProvider;
import java.net.URI; import java.net.URI;
@ -55,8 +54,7 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
params, params,
GrpcUtil.SHARED_CHANNEL_EXECUTOR, GrpcUtil.SHARED_CHANNEL_EXECUTOR,
GrpcUtil.getDefaultProxyDetector(), GrpcUtil.getDefaultProxyDetector(),
Stopwatch.createUnstarted(), Stopwatch.createUnstarted());
InternalServiceProviders.isAndroid(getClass().getClassLoader()));
} else { } else {
return null; return null;
} }

View File

@ -112,13 +112,7 @@ public class DnsNameResolverTest {
private String networkaddressCacheTtlPropertyValue; private String networkaddressCacheTtlPropertyValue;
private DnsNameResolver newResolver(String name, int port) { private DnsNameResolver newResolver(String name, int port) {
return return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted());
newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(), false);
}
private DnsNameResolver newResolver(String name, int port, boolean isAndroid) {
return newResolver(
name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(), isAndroid);
} }
private DnsNameResolver newResolver( private DnsNameResolver newResolver(
@ -126,23 +120,13 @@ public class DnsNameResolverTest {
int port, int port,
ProxyDetector proxyDetector, ProxyDetector proxyDetector,
Stopwatch stopwatch) { Stopwatch stopwatch) {
return newResolver(name, port, proxyDetector, stopwatch, false);
}
private DnsNameResolver newResolver(
String name,
int port,
ProxyDetector proxyDetector,
Stopwatch stopwatch,
boolean isAndroid) {
DnsNameResolver dnsResolver = new DnsNameResolver( DnsNameResolver dnsResolver = new DnsNameResolver(
null, null,
name, name,
Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(), Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(),
fakeExecutorResource, fakeExecutorResource,
proxyDetector, proxyDetector,
stopwatch, stopwatch);
isAndroid);
return dnsResolver; return dnsResolver;
} }
@ -215,16 +199,12 @@ public class DnsNameResolverTest {
@Test @Test
public void resolve_neverCache() throws Exception { public void resolve_neverCache() throws Exception {
resolve_neverCache(false);
}
private void resolve_neverCache(boolean isAndroid) throws Exception {
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "0"); System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "0");
final List<InetAddress> answer1 = createAddressList(2); final List<InetAddress> answer1 = createAddressList(2);
final List<InetAddress> answer2 = createAddressList(1); final List<InetAddress> answer2 = createAddressList(1);
String name = "foo.googleapis.com"; String name = "foo.googleapis.com";
DnsNameResolver resolver = newResolver(name, 81, isAndroid); DnsNameResolver resolver = newResolver(name, 81);
AddressResolver mockResolver = mock(AddressResolver.class); AddressResolver mockResolver = mock(AddressResolver.class);
when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2); when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
resolver.setAddressResolver(mockResolver); resolver.setAddressResolver(mockResolver);
@ -246,29 +226,15 @@ public class DnsNameResolverTest {
verify(mockResolver, times(2)).resolveAddress(Matchers.anyString()); verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
} }
@Test
public void resolve_androidNeverCache() throws Exception {
resolve_neverCache(true);
}
@Test @Test
public void resolve_cacheForever() throws Exception { public void resolve_cacheForever() throws Exception {
resolve_cacheForever(false);
}
private void resolve_cacheForever(boolean isAndroid) throws Exception {
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1"); System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1");
final List<InetAddress> answer1 = createAddressList(2); final List<InetAddress> answer1 = createAddressList(2);
String name = "foo.googleapis.com"; String name = "foo.googleapis.com";
FakeTicker fakeTicker = new FakeTicker(); FakeTicker fakeTicker = new FakeTicker();
DnsNameResolver resolver = DnsNameResolver resolver =
newResolver( newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
name,
81,
GrpcUtil.NOOP_PROXY_DETECTOR,
Stopwatch.createUnstarted(fakeTicker),
isAndroid);
AddressResolver mockResolver = mock(AddressResolver.class); AddressResolver mockResolver = mock(AddressResolver.class);
when(mockResolver.resolveAddress(Matchers.anyString())) when(mockResolver.resolveAddress(Matchers.anyString()))
.thenReturn(answer1) .thenReturn(answer1)
@ -293,17 +259,8 @@ public class DnsNameResolverTest {
verify(mockResolver).resolveAddress(Matchers.anyString()); verify(mockResolver).resolveAddress(Matchers.anyString());
} }
@Test
public void resolve_androidCacheForever() throws Exception {
resolve_cacheForever(true);
}
@Test @Test
public void resolve_usingCache() throws Exception { public void resolve_usingCache() throws Exception {
resolve_usingCache(false);
}
private void resolve_usingCache(boolean isAndroid) throws Exception {
long ttl = 60; long ttl = 60;
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl)); System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
final List<InetAddress> answer = createAddressList(2); final List<InetAddress> answer = createAddressList(2);
@ -311,12 +268,7 @@ public class DnsNameResolverTest {
FakeTicker fakeTicker = new FakeTicker(); FakeTicker fakeTicker = new FakeTicker();
DnsNameResolver resolver = DnsNameResolver resolver =
newResolver( newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
name,
81,
GrpcUtil.NOOP_PROXY_DETECTOR,
Stopwatch.createUnstarted(fakeTicker),
isAndroid);
AddressResolver mockResolver = mock(AddressResolver.class); AddressResolver mockResolver = mock(AddressResolver.class);
when(mockResolver.resolveAddress(Matchers.anyString())) when(mockResolver.resolveAddress(Matchers.anyString()))
.thenReturn(answer) .thenReturn(answer)
@ -342,17 +294,8 @@ public class DnsNameResolverTest {
verify(mockResolver).resolveAddress(Matchers.anyString()); verify(mockResolver).resolveAddress(Matchers.anyString());
} }
@Test
public void resolve_androidUsingCache() throws Exception {
resolve_usingCache(true);
}
@Test @Test
public void resolve_cacheExpired() throws Exception { public void resolve_cacheExpired() throws Exception {
resolve_cacheExpired(false);
}
private void resolve_cacheExpired(boolean isAndroid) throws Exception {
long ttl = 60; long ttl = 60;
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl)); System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
final List<InetAddress> answer1 = createAddressList(2); final List<InetAddress> answer1 = createAddressList(2);
@ -361,12 +304,7 @@ public class DnsNameResolverTest {
FakeTicker fakeTicker = new FakeTicker(); FakeTicker fakeTicker = new FakeTicker();
DnsNameResolver resolver = DnsNameResolver resolver =
newResolver( newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
name,
81,
GrpcUtil.NOOP_PROXY_DETECTOR,
Stopwatch.createUnstarted(fakeTicker),
isAndroid);
AddressResolver mockResolver = mock(AddressResolver.class); AddressResolver mockResolver = mock(AddressResolver.class);
when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2); when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
resolver.setAddressResolver(mockResolver); resolver.setAddressResolver(mockResolver);
@ -389,48 +327,26 @@ public class DnsNameResolverTest {
verify(mockResolver, times(2)).resolveAddress(Matchers.anyString()); verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
} }
@Test
public void resolve_androidCacheExpired() throws Exception {
resolve_cacheExpired(true);
}
@Test @Test
public void resolve_invalidTtlPropertyValue() throws Exception { public void resolve_invalidTtlPropertyValue() throws Exception {
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number"); System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number");
resolveDefaultValue(false); resolveDefaultValue();
}
@Test
public void resolve_androidInvalidTtlPropertyValue() throws Exception {
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number");
resolveDefaultValue(true);
} }
@Test @Test
public void resolve_noPropertyValue() throws Exception { public void resolve_noPropertyValue() throws Exception {
System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY); System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
resolveDefaultValue(false); resolveDefaultValue();
} }
@Test private void resolveDefaultValue() throws Exception {
public void resolve_androidNoPropertyValue() throws Exception {
System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
resolveDefaultValue(true);
}
private void resolveDefaultValue(boolean isAndroid) throws Exception {
final List<InetAddress> answer1 = createAddressList(2); final List<InetAddress> answer1 = createAddressList(2);
final List<InetAddress> answer2 = createAddressList(1); final List<InetAddress> answer2 = createAddressList(1);
String name = "foo.googleapis.com"; String name = "foo.googleapis.com";
FakeTicker fakeTicker = new FakeTicker(); FakeTicker fakeTicker = new FakeTicker();
DnsNameResolver resolver = DnsNameResolver resolver =
newResolver( newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
name,
81,
GrpcUtil.NOOP_PROXY_DETECTOR,
Stopwatch.createUnstarted(fakeTicker),
isAndroid);
AddressResolver mockResolver = mock(AddressResolver.class); AddressResolver mockResolver = mock(AddressResolver.class);
when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2); when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
resolver.setAddressResolver(mockResolver); resolver.setAddressResolver(mockResolver);
@ -441,11 +357,7 @@ public class DnsNameResolverTest {
assertAnswerMatches(answer1, 81, resultCaptor.getValue()); assertAnswerMatches(answer1, 81, resultCaptor.getValue());
assertEquals(0, fakeClock.numPendingTasks()); assertEquals(0, fakeClock.numPendingTasks());
long defaultTtl = fakeTicker.advance(DnsNameResolver.DEFAULT_NETWORK_CACHE_TTL_SECONDS, TimeUnit.SECONDS);
isAndroid
? DnsNameResolver.DEFAULT_ANDROID_NETWORK_CACHE_TTL_SECONDS
: DnsNameResolver.DEFAULT_NETWORK_CACHE_TTL_SECONDS;
fakeTicker.advance(defaultTtl, TimeUnit.SECONDS);
resolver.refresh(); resolver.refresh();
assertEquals(1, fakeExecutor.runDueTasks()); assertEquals(1, fakeExecutor.runDueTasks());
verifyNoMoreInteractions(mockListener); verifyNoMoreInteractions(mockListener);