diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index 490bd8255a..05011adb37 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -20,7 +20,6 @@ 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; @@ -43,7 +42,6 @@ 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; @@ -90,19 +88,6 @@ 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. - * - *

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 @@ -130,8 +115,6 @@ final class DnsNameResolver extends NameResolver { private final String host; private final int port; private final Resource executorResource; - private final long networkAddressCacheTtlNanos; - private final Stopwatch stopwatch; @GuardedBy("this") private boolean shutdown; @GuardedBy("this") @@ -140,11 +123,10 @@ 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 executorResource, ProxyDetector proxyDetector, - Stopwatch stopwatch) { + Resource executorResource, + ProxyDetector proxyDetector) { // 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; @@ -167,8 +149,6 @@ final class DnsNameResolver extends NameResolver { port = nameUri.getPort(); } this.proxyDetector = proxyDetector; - this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch"); - this.networkAddressCacheTtlNanos = getNetworkAddressCacheTtlNanos(); } @Override @@ -198,13 +178,6 @@ 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; } @@ -234,10 +207,6 @@ 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)); @@ -284,23 +253,6 @@ 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) { diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index d0db539d41..cddbe3f3b0 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java @@ -17,7 +17,6 @@ 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; @@ -53,8 +52,7 @@ public final class DnsNameResolverProvider extends NameResolverProvider { name, params, GrpcUtil.SHARED_CHANNEL_EXECUTOR, - GrpcUtil.getDefaultProxyDetector(), - Stopwatch.createUnstarted()); + GrpcUtil.getDefaultProxyDetector()); } else { return null; } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 44bde22222..97ed67db1d 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -27,13 +27,10 @@ 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; @@ -55,8 +52,6 @@ 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; @@ -108,25 +103,21 @@ public class DnsNameResolverTest { private NameResolver.Listener mockListener; @Captor private ArgumentCaptor> resultCaptor; - @Nullable - private String networkaddressCacheTtlPropertyValue; private DnsNameResolver newResolver(String name, int port) { - return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted()); + return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR); } private DnsNameResolver newResolver( String name, int port, - ProxyDetector proxyDetector, - Stopwatch stopwatch) { + ProxyDetector proxyDetector) { DnsNameResolver dnsResolver = new DnsNameResolver( null, name, Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(), fakeExecutorResource, - proxyDetector, - stopwatch); + proxyDetector); return dnsResolver; } @@ -134,19 +125,6 @@ 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 @@ -198,8 +176,7 @@ public class DnsNameResolverTest { } @Test - public void resolve_neverCache() throws Exception { - System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "0"); + public void resolve() throws Exception { final List answer1 = createAddressList(2); final List answer2 = createAddressList(1); String name = "foo.googleapis.com"; @@ -226,156 +203,6 @@ 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 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 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 answer1 = createAddressList(2); - final List 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 answer1 = createAddressList(2); - final List 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"; @@ -502,8 +329,7 @@ public class DnsNameResolverTest { "password"); when(alwaysDetectProxy.proxyFor(any(SocketAddress.class))) .thenReturn(proxyParameters); - DnsNameResolver resolver = - newResolver(name, port, alwaysDetectProxy, Stopwatch.createUnstarted()); + DnsNameResolver resolver = newResolver(name, port, alwaysDetectProxy); AddressResolver mockAddressResolver = mock(AddressResolver.class); when(mockAddressResolver.resolveAddress(Matchers.anyString())).thenThrow(new AssertionError()); resolver.setAddressResolver(mockAddressResolver);