andreacavalli
/
netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

DnsNameResolver.resolve(...) should notify future as soon as one preferred record was resolved (#9050) 

Motivation:

At the moment resolve(...) does just delegate to resolveAll(...) and so will only notify the future once all records were resolved. This is wasteful as we are only interested in the first record anyway. We should notify the promise as soon as one record that matches the preferred record type is resolved.

Modifications:

- Introduce DnsResolveContext.isCompleteEarly(...) to be able to detect once we should early notify the promise.
- Make use of this early detecting if resolve(...) is called
- Remove FutureListener which could lead to IllegalReferenceCountException due double releases
- add unit test

Result:

Be able to notify about resolved host more quickly.
This commit is contained in:
Norman Maurer 2019-04-15 21:42:04 +02:00 committed by GitHub
parent 4b36a5b08b
commit 075cf8c02e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 99 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolveContext.java
View File

@ -31,13 +31,16 @@ final class DnsAddressResolveContext extends DnsResolveContext<InetAddress> {
private final DnsCache resolveCache;
private final AuthoritativeDnsServerCache authoritativeDnsServerCache;
private final boolean completeEarlyIfPossible;
DnsAddressResolveContext(DnsNameResolver parent, String hostname, DnsRecord[] additionals,
DnsServerAddressStream nameServerAddrs, DnsCache resolveCache,
AuthoritativeDnsServerCache authoritativeDnsServerCache) {
AuthoritativeDnsServerCache authoritativeDnsServerCache,
boolean completeEarlyIfPossible) {
super(parent, hostname, DnsRecord.CLASS_IN, parent.resolveRecordTypes(), additionals, nameServerAddrs);
this.resolveCache = resolveCache;
this.authoritativeDnsServerCache = authoritativeDnsServerCache;
this.completeEarlyIfPossible = completeEarlyIfPossible;
}
@Override
@ -46,7 +49,7 @@ final class DnsAddressResolveContext extends DnsResolveContext<InetAddress> {
DnsRecord[] additionals,
DnsServerAddressStream nameServerAddrs) {
return new DnsAddressResolveContext(parent, hostname, additionals, nameServerAddrs, resolveCache,
authoritativeDnsServerCache);
authoritativeDnsServerCache, completeEarlyIfPossible);
}
@Override
@ -60,6 +63,11 @@ final class DnsAddressResolveContext extends DnsResolveContext<InetAddress> {
return unfiltered;
}
@Override
boolean isCompleteEarly(InetAddress resolved) {
return completeEarlyIfPossible && parent.preferredAddressType().addressType() == resolved.getClass();
}
@Override
void cache(String hostname, DnsRecord[] additionals,
DnsRecord result, InetAddress convertedResult) {

26
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
View File

@ -851,7 +851,7 @@ public class DnsNameResolver extends InetNameResolver {
}
if (!doResolveCached(hostname, additionals, promise, resolveCache)) {
doResolveUncached(hostname, additionals, promise, resolveCache);
doResolveUncached(hostname, additionals, promise, resolveCache, true);
}
}
@ -905,9 +905,9 @@ public class DnsNameResolver extends InetNameResolver {
private void doResolveUncached(String hostname,
DnsRecord[] additionals,
final Promise<InetAddress> promise,
DnsCache resolveCache) {
DnsCache resolveCache, boolean completeEarlyIfPossible) {
final Promise<List<InetAddress>> allPromise = executor().newPromise();
doResolveAllUncached(hostname, additionals, allPromise, resolveCache);
doResolveAllUncached(hostname, additionals, allPromise, resolveCache, true);
allPromise.addListener(new FutureListener<List<InetAddress>>() {
@Override
public void operationComplete(Future<List<InetAddress>> future) {
@ -954,7 +954,7 @@ public class DnsNameResolver extends InetNameResolver {
}
if (!doResolveAllCached(hostname, additionals, promise, resolveCache, resolvedInternetProtocolFamilies)) {
doResolveAllUncached(hostname, additionals, promise, resolveCache);
doResolveAllUncached(hostname, additionals, promise, resolveCache, false);
}
}
@ -997,33 +997,35 @@ public class DnsNameResolver extends InetNameResolver {
private void doResolveAllUncached(final String hostname,
final DnsRecord[] additionals,
final Promise<List<InetAddress>> promise,
final DnsCache resolveCache) {
final DnsCache resolveCache,
final boolean completeEarlyIfPossible) {
// Call doResolveUncached0(...) in the EventLoop as we may need to submit multiple queries which would need
// to submit multiple Runnable at the end if we are not already on the EventLoop.
EventExecutor executor = executor();
if (executor.inEventLoop()) {
doResolveAllUncached0(hostname, additionals, promise, resolveCache);
doResolveAllUncached0(hostname, additionals, promise, resolveCache, completeEarlyIfPossible);
} else {
executor.execute(new Runnable() {
@Override
public void run() {
doResolveAllUncached0(hostname, additionals, promise, resolveCache);
doResolveAllUncached0(hostname, additionals, promise, resolveCache, completeEarlyIfPossible);
}
});
}
}
private void doResolveAllUncached0(String hostname,
DnsRecord[] additionals,
Promise<List<InetAddress>> promise,
DnsCache resolveCache) {
DnsRecord[] additionals,
Promise<List<InetAddress>> promise,
DnsCache resolveCache,
boolean completeEarlyIfPossible) {
assert executor().inEventLoop();
final DnsServerAddressStream nameServerAddrs =
dnsServerAddressStreamProvider.nameServerAddressStream(hostname);
new DnsAddressResolveContext(this, hostname, additionals, nameServerAddrs,
resolveCache, authoritativeDnsServerCache).resolve(promise);
new DnsAddressResolveContext(this, hostname, additionals, nameServerAddrs, resolveCache,
authoritativeDnsServerCache, completeEarlyIfPossible).resolve(promise);
}
private static String hostname(String inetHost) {

5
resolver-dns/src/main/java/io/netty/resolver/dns/DnsRecordResolveContext.java
View File

@ -58,6 +58,11 @@ final class DnsRecordResolveContext extends DnsResolveContext<DnsRecord> {
return unfiltered;
}
@Override
boolean isCompleteEarly(DnsRecord resolved) {
return false;
}
@Override
void cache(String hostname, DnsRecord[] additionals, DnsRecord result, DnsRecord convertedResult) {
// Do not cache.

73
resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
View File

@ -62,15 +62,6 @@ import static java.lang.Math.min;
abstract class DnsResolveContext<T> {
private static final FutureListener<AddressedEnvelope<DnsResponse, InetSocketAddress>> RELEASE_RESPONSE =
new FutureListener<AddressedEnvelope<DnsResponse, InetSocketAddress>>() {
@Override
public void operationComplete(Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> future) {
if (future.isSuccess()) {
future.getNow().release();
}
}
};
private static final RuntimeException NXDOMAIN_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
new RuntimeException("No answer found and NXDOMAIN response code returned"),
DnsResolveContext.class,
@ -107,6 +98,7 @@ abstract class DnsResolveContext<T> {
private List<T> finalResult;
private int allowedQueries;
private boolean triedCNAME;
private boolean completeEarly;
DnsResolveContext(DnsNameResolver parent,
String hostname, int dnsClass, DnsRecordType[] expectedTypes,
@ -165,6 +157,8 @@ abstract class DnsResolveContext<T> {
*/
abstract List<T> filterResults(List<T> unfiltered);
abstract boolean isCompleteEarly(T resolved);
/**
* Caches a successful resolution.
*/
@ -329,7 +323,8 @@ abstract class DnsResolveContext<T> {
final boolean flush,
final Promise<List<T>> promise,
final Throwable cause) {
if (nameServerAddrStreamIndex >= nameServerAddrStream.size() || allowedQueries == 0 || promise.isCancelled()) {
if (completeEarly || nameServerAddrStreamIndex >= nameServerAddrStream.size() ||
allowedQueries == 0 || promise.isCancelled()) {
tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question, queryLifecycleObserver,
promise, cause);
return;
@ -456,7 +451,7 @@ abstract class DnsResolveContext<T> {
public boolean clear(String hostname) {
return authoritativeDnsServerCache.clear(hostname);
}
}).resolve(resolverPromise);
}, false).resolve(resolverPromise);
}
}
@ -695,21 +690,41 @@ abstract class DnsResolveContext<T> {
continue;
}
// We want to ensure we do not have duplicates in finalResult as this may be unexpected.
//
// While using a LinkedHashSet or HashSet may sound like the perfect fit for this we will use an
// ArrayList here as duplicates should be found quite unfrequently in the wild and we dont want to pay
// for the extra memory copy and allocations in this cases later on.
if (finalResult == null) {
finalResult = new ArrayList<T>(8);
finalResult.add(converted);
} else if (!finalResult.contains(converted)) {
finalResult.add(converted);
boolean shouldRelease = false;
// Check if we did determine we wanted to complete early before. If this is the case we want to not
// include the result
if (!completeEarly) {
boolean completeEarly = isCompleteEarly(converted);
if (completeEarly) {
this.completeEarly = true;
}
// We want to ensure we do not have duplicates in finalResult as this may be unexpected.
//
// While using a LinkedHashSet or HashSet may sound like the perfect fit for this we will use an
// ArrayList here as duplicates should be found quite unfrequently in the wild and we dont want to pay
// for the extra memory copy and allocations in this cases later on.
if (finalResult == null) {
if (completeEarly) {
finalResult = Collections.singletonList(converted);
} else {
finalResult = new ArrayList<T>(8);
finalResult.add(converted);
}
} else if (!finalResult.contains(converted)) {
finalResult.add(converted);
} else {
shouldRelease = true;
}
} else {
shouldRelease = true;
}
cache(hostname, additionals, r, converted);
found = true;
if (shouldRelease) {
ReferenceCountUtil.release(converted);
}
// Note that we do not break from the loop here, so we decode/cache all A/AAAA records.
}
@ -800,7 +815,7 @@ abstract class DnsResolveContext<T> {
final Throwable cause) {
// There are no queries left to try.
if (!queriesInProgress.isEmpty()) {
if (!completeEarly && !queriesInProgress.isEmpty()) {
queryLifecycleObserver.queryCancelled(allowedQueries);
// There are still some queries in process, we will try to notify once the next one finishes until
@ -846,22 +861,24 @@ abstract class DnsResolveContext<T> {
}
private void finishResolve(Promise<List<T>> promise, Throwable cause) {
if (!queriesInProgress.isEmpty()) {
// If completeEarly was true we still want to continue processing the queries to ensure we still put everything
// in the cache eventually.
if (!completeEarly && !queriesInProgress.isEmpty()) {
// If there are queries in progress, we should cancel it because we already finished the resolution.
for (Iterator<Future<AddressedEnvelope<DnsResponse, InetSocketAddress>>> i = queriesInProgress.iterator();
i.hasNext();) {
Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> f = i.next();
i.remove();
if (!f.cancel(false)) {
f.addListener(RELEASE_RESPONSE);
}
f.cancel(false);
}
}
if (finalResult != null) {
// Found at least one resolved record.
DnsNameResolver.trySuccess(promise, filterResults(finalResult));
if (!promise.isDone()) {
// Found at least one resolved record.
DnsNameResolver.trySuccess(promise, filterResults(finalResult));
}
return;
}

25
resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java
View File

@ -2620,4 +2620,29 @@ public class DnsNameResolverTest {
}
}
}
@Test(timeout = 2000)
public void testDropAAAAResolveFastFast() throws IOException {
String host = "somehost.netty.io";
TestDnsServer dnsServer2 = new TestDnsServer(Collections.singleton(host));
dnsServer2.start(true);
DnsNameResolver resolver = null;
try {
DnsNameResolverBuilder builder = newResolver()
.recursionDesired(false)
.queryTimeoutMillis(10000)
.resolvedAddressTypes(ResolvedAddressTypes.IPV4_PREFERRED)
.maxQueriesPerResolve(16)
.nameServerProvider(new SingletonDnsServerAddressStreamProvider(dnsServer2.localAddress()));
resolver = builder.build();
InetAddress address = resolver.resolve(host).syncUninterruptibly().getNow();
assertEquals(host, address.getHostName());
} finally {
dnsServer2.stop();
if (resolver != null) {
resolver.close();
}
}
}
}