diff options
author | Jihun Cho <jihuncho@google.com> | 2018-10-12 15:10:33 -0700 |
---|---|---|
committer | Eric Anderson <ejona@google.com> | 2018-10-24 07:36:39 -0700 |
commit | 0981ba68ee5e9217506cfcca7ff730c056f5c92d (patch) | |
tree | 56f681a4de49011255fd769c046ced4e820be4f6 | |
parent | 092bf2e39aaf960df2c6335667f4048452ec1fcc (diff) | |
download | grpc-grpc-java-0981ba68ee5e9217506cfcca7ff730c056f5c92d.tar.gz |
Revert "core: DnsNameResolver caches refresh (#4812)"
This reverts commit 189991012bbff42b975c51045c32efceaa07f462.
3 files changed, 8 insertions, 232 deletions
diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index dfa815525..dc1b783fd 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.Throwables; import com.google.common.base.Verify; import io.grpc.Attributes; @@ -44,7 +43,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; @@ -93,19 +91,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. - * - * <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 @@ -135,8 +120,6 @@ 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") @@ -145,11 +128,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<ExecutorService> executorResource, ProxyDetector proxyDetector, - Stopwatch stopwatch) { + Resource<ExecutorService> 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; @@ -172,8 +154,6 @@ final class DnsNameResolver extends NameResolver { port = nameUri.getPort(); } this.proxyDetector = proxyDetector; - this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch"); - this.networkAddressCacheTtlNanos = getNetworkAddressCacheTtlNanos(); } @Override @@ -203,13 +183,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; } @@ -239,10 +212,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)); @@ -289,23 +258,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 d0db539d4..cddbe3f3b 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 6f99cc3bc..3f7adfa57 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -28,13 +28,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; @@ -57,8 +54,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; @@ -110,25 +105,21 @@ 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, 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; } @@ -136,19 +127,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 @@ -200,8 +178,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<InetAddress> answer1 = createAddressList(2); final List<InetAddress> answer2 = createAddressList(1); String name = "foo.googleapis.com"; @@ -229,156 +206,6 @@ public class DnsNameResolverTest { } @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"; final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001); @@ -521,8 +348,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); |