mirror of https://github.com/grpc/grpc-java.git
Revert "core: DnsNameResolver caches refresh (#4812)"
This reverts commit 189991012b.
This commit is contained in:
Jihun Cho
creamsoup
parent
c24f2fd25b
commit
0e8cf58d1a
|
|
@ -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.
|
||||
*
|
||||
* <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
|
||||
|
|
@ -130,8 +115,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")
|
||||
|
|
@ -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<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;
|
||||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<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;
|
||||
}
|
||||
|
||||
|
|
@ -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<InetAddress> answer1 = createAddressList(2);
|
||||
final List<InetAddress> 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<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";
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue