Skip to content

Commit 0a7fa14

Browse files
creamsoupcreamsoup
committed
core: Roll forward "core: DnsNameResolver caches refresh (grpc#4812)"
This reverts commit 0e8cf58.
1 parent 52d6a6e commit 0a7fa14

File tree

3 files changed

+235
-12
lines changed

3 files changed

+235
-12
lines changed

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

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.annotations.VisibleForTesting;
2222
import com.google.common.base.MoreObjects;
2323
import com.google.common.base.Preconditions;
24+
import com.google.common.base.Stopwatch;
2425
import com.google.common.base.Throwables;
2526
import com.google.common.base.Verify;
2627
import io.grpc.Attributes;
@@ -44,6 +45,7 @@
4445
import java.util.Random;
4546
import java.util.Set;
4647
import java.util.concurrent.ExecutorService;
48+
import java.util.concurrent.TimeUnit;
4749
import java.util.concurrent.atomic.AtomicReference;
4850
import java.util.logging.Level;
4951
import java.util.logging.Logger;
@@ -90,6 +92,19 @@ final class DnsNameResolver extends NameResolver {
9092
private static final String JNDI_TXT_PROPERTY =
9193
System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_service_config", "false");
9294

95+
/**
96+
* Java networking system properties name for caching DNS result.
97+
*
98+
* <p>Default value is -1 (cache forever) if security manager is installed. If security manager is
99+
* not installed, the ttl value is {@code null} which falls back to {@link
100+
* #DEFAULT_NETWORK_CACHE_TTL_SECONDS gRPC default value}.
101+
*/
102+
@VisibleForTesting
103+
static final String NETWORKADDRESS_CACHE_TTL_PROPERTY = "networkaddress.cache.ttl";
104+
/** Default DNS cache duration if network cache ttl value is not specified ({@code null}). */
105+
@VisibleForTesting
106+
static final long DEFAULT_NETWORK_CACHE_TTL_SECONDS = 30;
107+
93108
@VisibleForTesting
94109
static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY);
95110
@VisibleForTesting
@@ -124,11 +139,11 @@ final class DnsNameResolver extends NameResolver {
124139
@GuardedBy("this")
125140
private Listener listener;
126141

127-
private final Runnable resolveRunnable = new Resolve(this);
142+
private final Runnable resolveRunnable;
128143

129144
DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params,
130-
Resource<ExecutorService> executorResource,
131-
ProxyDetector proxyDetector) {
145+
Resource<ExecutorService> executorResource, ProxyDetector proxyDetector,
146+
Stopwatch stopwatch) {
132147
// TODO: if a DNS server is provided as nsAuthority, use it.
133148
// https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
134149
this.executorResource = executorResource;
@@ -151,6 +166,7 @@ final class DnsNameResolver extends NameResolver {
151166
port = nameUri.getPort();
152167
}
153168
this.proxyDetector = proxyDetector;
169+
this.resolveRunnable = new Resolve(this, stopwatch);
154170
}
155171

156172
@Override
@@ -176,9 +192,14 @@ public final synchronized void refresh() {
176192
static final class Resolve implements Runnable {
177193

178194
private final DnsNameResolver resolver;
195+
private final Stopwatch stopwatch;
196+
private final long cacheTtlNanos;
197+
private ResolutionResults cachedResolutionResults = null;
179198

180-
Resolve(DnsNameResolver resolver) {
199+
Resolve(DnsNameResolver resolver, Stopwatch stopwatch) {
181200
this.resolver = resolver;
201+
this.stopwatch = Preconditions.checkNotNull(stopwatch, "stopwatch");
202+
this.cacheTtlNanos = getNetworkAddressCacheTtlNanos();
182203
}
183204

184205
@Override
@@ -188,7 +209,7 @@ public void run() {
188209
}
189210
Listener savedListener;
190211
synchronized (resolver) {
191-
if (resolver.shutdown) {
212+
if (resolver.shutdown || !cacheRefreshRequired()) {
192213
return;
193214
}
194215
savedListener = resolver.listener;
@@ -203,6 +224,29 @@ public void run() {
203224
}
204225
}
205226

227+
private boolean cacheRefreshRequired() {
228+
return cachedResolutionResults == null
229+
|| cacheTtlNanos == 0
230+
|| (cacheTtlNanos > 0 && stopwatch.elapsed(TimeUnit.NANOSECONDS) > cacheTtlNanos);
231+
}
232+
233+
/** Returns value of network address cache ttl property. */
234+
private static long getNetworkAddressCacheTtlNanos() {
235+
String cacheTtlPropertyValue = System.getProperty(NETWORKADDRESS_CACHE_TTL_PROPERTY);
236+
long cacheTtl = DEFAULT_NETWORK_CACHE_TTL_SECONDS;
237+
if (cacheTtlPropertyValue != null) {
238+
try {
239+
cacheTtl = Long.parseLong(cacheTtlPropertyValue);
240+
} catch (NumberFormatException e) {
241+
logger.log(
242+
Level.WARNING,
243+
"Property({0}) valid is not valid number format({1}), fall back to default({2})",
244+
new Object[] {NETWORKADDRESS_CACHE_TTL_PROPERTY, cacheTtlPropertyValue, cacheTtl});
245+
}
246+
}
247+
return cacheTtl > 0 ? TimeUnit.SECONDS.toNanos(cacheTtl) : cacheTtl;
248+
}
249+
206250
@VisibleForTesting
207251
void resolveInternal(Listener savedListener) {
208252
InetSocketAddress destination =
@@ -239,6 +283,10 @@ void resolveInternal(Listener savedListener) {
239283
enableSrv,
240284
enableTxt,
241285
resolver.host);
286+
cachedResolutionResults = resolutionResults;
287+
if (cacheTtlNanos > 0) {
288+
stopwatch.reset().start();
289+
}
242290
if (logger.isLoggable(Level.FINER)) {
243291
logger.finer("Found DNS results " + resolutionResults + " for " + resolver.host);
244292
}

‎core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.internal;
1818

1919
import com.google.common.base.Preconditions;
20+
import com.google.common.base.Stopwatch;
2021
import io.grpc.Attributes;
2122
import io.grpc.NameResolverProvider;
2223
import java.net.URI;
@@ -52,7 +53,8 @@ public DnsNameResolver newNameResolver(URI targetUri, Attributes params) {
5253
name,
5354
params,
5455
GrpcUtil.SHARED_CHANNEL_EXECUTOR,
55-
GrpcUtil.getDefaultProxyDetector());
56+
GrpcUtil.getDefaultProxyDetector(),
57+
Stopwatch.createUnstarted());
5658
} else {
5759
return null;
5860
}

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

Lines changed: 179 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@
3030
import static org.mockito.Mockito.verifyNoMoreInteractions;
3131
import static org.mockito.Mockito.when;
3232

33+
import com.google.common.base.Stopwatch;
3334
import com.google.common.collect.Iterables;
3435
import com.google.common.net.InetAddresses;
36+
import com.google.common.testing.FakeTicker;
3537
import io.grpc.Attributes;
3638
import io.grpc.EquivalentAddressGroup;
3739
import io.grpc.NameResolver;
@@ -56,6 +58,8 @@
5658
import java.util.Map;
5759
import java.util.Random;
5860
import java.util.concurrent.ExecutorService;
61+
import java.util.concurrent.TimeUnit;
62+
import javax.annotation.Nullable;
5963
import org.junit.After;
6064
import org.junit.Before;
6165
import org.junit.Rule;
@@ -107,28 +111,45 @@ public void close(ExecutorService instance) {
107111
private NameResolver.Listener mockListener;
108112
@Captor
109113
private ArgumentCaptor<List<EquivalentAddressGroup>> resultCaptor;
114+
@Nullable
115+
private String networkaddressCacheTtlPropertyValue;
110116

111117
private DnsNameResolver newResolver(String name, int port) {
112-
return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR);
118+
return newResolver(name, port, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted());
113119
}
114120

115121
private DnsNameResolver newResolver(
116122
String name,
117123
int port,
118-
ProxyDetector proxyDetector) {
124+
ProxyDetector proxyDetector,
125+
Stopwatch stopwatch) {
119126
DnsNameResolver dnsResolver = new DnsNameResolver(
120127
null,
121128
name,
122129
Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(),
123130
fakeExecutorResource,
124-
proxyDetector);
131+
proxyDetector,
132+
stopwatch);
125133
return dnsResolver;
126134
}
127135

128136
@Before
129137
public void setUp() {
130138
MockitoAnnotations.initMocks(this);
131139
DnsNameResolver.enableJndi = true;
140+
networkaddressCacheTtlPropertyValue =
141+
System.getProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
142+
}
143+
144+
@After
145+
public void restoreSystemProperty() {
146+
if (networkaddressCacheTtlPropertyValue == null) {
147+
System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
148+
} else {
149+
System.setProperty(
150+
DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY,
151+
networkaddressCacheTtlPropertyValue);
152+
}
132153
}
133154

134155
@After
@@ -180,7 +201,8 @@ public void invalidDnsName_containsUnderscore() {
180201
}
181202

182203
@Test
183-
public void resolve() throws Exception {
204+
public void resolve_neverCache() throws Exception {
205+
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "0");
184206
final List<InetAddress> answer1 = createAddressList(2);
185207
final List<InetAddress> answer2 = createAddressList(1);
186208
String name = "foo.googleapis.com";
@@ -219,7 +241,7 @@ public List<InetAddress> resolveAddress(String host) throws Exception {
219241
}
220242
});
221243

222-
new DnsNameResolver.Resolve(nrf).resolveInternal(mockListener);
244+
new DnsNameResolver.Resolve(nrf, Stopwatch.createUnstarted()).resolveInternal(mockListener);
223245

224246
ArgumentCaptor<Status> ac = ArgumentCaptor.forClass(Status.class);
225247
verify(mockListener).onError(ac.capture());
@@ -228,6 +250,156 @@ public List<InetAddress> resolveAddress(String host) throws Exception {
228250
assertThat(ac.getValue().getDescription()).contains("No DNS backend or balancer addresses");
229251
}
230252

253+
@Test
254+
public void resolve_cacheForever() throws Exception {
255+
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "-1");
256+
final List<InetAddress> answer1 = createAddressList(2);
257+
String name = "foo.googleapis.com";
258+
FakeTicker fakeTicker = new FakeTicker();
259+
260+
DnsNameResolver resolver =
261+
newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
262+
AddressResolver mockResolver = mock(AddressResolver.class);
263+
when(mockResolver.resolveAddress(Matchers.anyString()))
264+
.thenReturn(answer1)
265+
.thenThrow(new AssertionError("should not called twice"));
266+
resolver.setAddressResolver(mockResolver);
267+
268+
resolver.start(mockListener);
269+
assertEquals(1, fakeExecutor.runDueTasks());
270+
verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
271+
assertAnswerMatches(answer1, 81, resultCaptor.getValue());
272+
assertEquals(0, fakeClock.numPendingTasks());
273+
274+
fakeTicker.advance(1, TimeUnit.DAYS);
275+
resolver.refresh();
276+
assertEquals(1, fakeExecutor.runDueTasks());
277+
verifyNoMoreInteractions(mockListener);
278+
assertAnswerMatches(answer1, 81, resultCaptor.getValue());
279+
assertEquals(0, fakeClock.numPendingTasks());
280+
281+
resolver.shutdown();
282+
283+
verify(mockResolver).resolveAddress(Matchers.anyString());
284+
}
285+
286+
@Test
287+
public void resolve_usingCache() throws Exception {
288+
long ttl = 60;
289+
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
290+
final List<InetAddress> answer = createAddressList(2);
291+
String name = "foo.googleapis.com";
292+
FakeTicker fakeTicker = new FakeTicker();
293+
294+
DnsNameResolver resolver =
295+
newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
296+
AddressResolver mockResolver = mock(AddressResolver.class);
297+
when(mockResolver.resolveAddress(Matchers.anyString()))
298+
.thenReturn(answer)
299+
.thenThrow(new AssertionError("should not reach here."));
300+
resolver.setAddressResolver(mockResolver);
301+
302+
resolver.start(mockListener);
303+
assertEquals(1, fakeExecutor.runDueTasks());
304+
verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
305+
assertAnswerMatches(answer, 81, resultCaptor.getValue());
306+
assertEquals(0, fakeClock.numPendingTasks());
307+
308+
// this refresh should return cached result
309+
fakeTicker.advance(ttl - 1, TimeUnit.SECONDS);
310+
resolver.refresh();
311+
assertEquals(1, fakeExecutor.runDueTasks());
312+
verifyNoMoreInteractions(mockListener);
313+
assertAnswerMatches(answer, 81, resultCaptor.getValue());
314+
assertEquals(0, fakeClock.numPendingTasks());
315+
316+
resolver.shutdown();
317+
318+
verify(mockResolver).resolveAddress(Matchers.anyString());
319+
}
320+
321+
@Test
322+
public void resolve_cacheExpired() throws Exception {
323+
long ttl = 60;
324+
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, Long.toString(ttl));
325+
final List<InetAddress> answer1 = createAddressList(2);
326+
final List<InetAddress> answer2 = createAddressList(1);
327+
String name = "foo.googleapis.com";
328+
FakeTicker fakeTicker = new FakeTicker();
329+
330+
DnsNameResolver resolver =
331+
newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
332+
AddressResolver mockResolver = mock(AddressResolver.class);
333+
when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
334+
resolver.setAddressResolver(mockResolver);
335+
336+
resolver.start(mockListener);
337+
assertEquals(1, fakeExecutor.runDueTasks());
338+
verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
339+
assertAnswerMatches(answer1, 81, resultCaptor.getValue());
340+
assertEquals(0, fakeClock.numPendingTasks());
341+
342+
fakeTicker.advance(ttl + 1, TimeUnit.SECONDS);
343+
resolver.refresh();
344+
assertEquals(1, fakeExecutor.runDueTasks());
345+
verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class));
346+
assertAnswerMatches(answer2, 81, resultCaptor.getValue());
347+
assertEquals(0, fakeClock.numPendingTasks());
348+
349+
resolver.shutdown();
350+
351+
verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
352+
}
353+
354+
@Test
355+
public void resolve_invalidTtlPropertyValue() throws Exception {
356+
System.setProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY, "not_a_number");
357+
resolveDefaultValue();
358+
}
359+
360+
@Test
361+
public void resolve_noPropertyValue() throws Exception {
362+
System.clearProperty(DnsNameResolver.NETWORKADDRESS_CACHE_TTL_PROPERTY);
363+
resolveDefaultValue();
364+
}
365+
366+
private void resolveDefaultValue() throws Exception {
367+
final List<InetAddress> answer1 = createAddressList(2);
368+
final List<InetAddress> answer2 = createAddressList(1);
369+
String name = "foo.googleapis.com";
370+
FakeTicker fakeTicker = new FakeTicker();
371+
372+
DnsNameResolver resolver =
373+
newResolver(name, 81, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(fakeTicker));
374+
AddressResolver mockResolver = mock(AddressResolver.class);
375+
when(mockResolver.resolveAddress(Matchers.anyString())).thenReturn(answer1).thenReturn(answer2);
376+
resolver.setAddressResolver(mockResolver);
377+
378+
resolver.start(mockListener);
379+
assertEquals(1, fakeExecutor.runDueTasks());
380+
verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class));
381+
assertAnswerMatches(answer1, 81, resultCaptor.getValue());
382+
assertEquals(0, fakeClock.numPendingTasks());
383+
384+
fakeTicker.advance(DnsNameResolver.DEFAULT_NETWORK_CACHE_TTL_SECONDS, TimeUnit.SECONDS);
385+
resolver.refresh();
386+
assertEquals(1, fakeExecutor.runDueTasks());
387+
verifyNoMoreInteractions(mockListener);
388+
assertAnswerMatches(answer1, 81, resultCaptor.getValue());
389+
assertEquals(0, fakeClock.numPendingTasks());
390+
391+
fakeTicker.advance(1, TimeUnit.SECONDS);
392+
resolver.refresh();
393+
assertEquals(1, fakeExecutor.runDueTasks());
394+
verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class));
395+
assertAnswerMatches(answer2, 81, resultCaptor.getValue());
396+
assertEquals(0, fakeClock.numPendingTasks());
397+
398+
resolver.shutdown();
399+
400+
verify(mockResolver, times(2)).resolveAddress(Matchers.anyString());
401+
}
402+
231403
@Test
232404
public void resolveAll_nullResourceResolver() throws Exception {
233405
final String hostname = "addr.fake";
@@ -371,7 +543,8 @@ public void doNotResolveWhenProxyDetected() throws Exception {
371543
"password");
372544
when(alwaysDetectProxy.proxyFor(any(SocketAddress.class)))
373545
.thenReturn(proxyParameters);
374-
DnsNameResolver resolver = newResolver(name, port, alwaysDetectProxy);
546+
DnsNameResolver resolver =
547+
newResolver(name, port, alwaysDetectProxy, Stopwatch.createUnstarted());
375548
AddressResolver mockAddressResolver = mock(AddressResolver.class);
376549
when(mockAddressResolver.resolveAddress(Matchers.anyString())).thenThrow(new AssertionError());
377550
resolver.setAddressResolver(mockAddressResolver);

0 commit comments

Comments
 (0)