Skip to content

Commit 684dfd8

Browse files
normanmaurernormanmaurer
authored
Be able to retry the query via TCP if a query failed because of a timeout when u… (#13757)
…sing UDP Motivation: We should allow people to retry the query via TCP if the query failed because of a timeout when using UDP. Modifications: - Move all the retry code for TCP into DnsQueryContext so we can reuse the same code for handling truncation and retry. - Retry on timeout if configured by user - Add unit tests Result: More robust resolver
1 parent 47e1f43 commit 684dfd8

7 files changed

Lines changed: 482 additions & 290 deletions

File tree

‎resolver-dns/src/main/java/io/netty/resolver/dns/DatagramDnsQueryContext.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.netty.resolver.dns;
1717

18+
import io.netty.bootstrap.Bootstrap;
1819
import io.netty.channel.AddressedEnvelope;
1920
import io.netty.channel.Channel;
2021
import io.netty.handler.codec.dns.DatagramDnsQuery;
@@ -33,10 +34,12 @@ final class DatagramDnsQueryContext extends DnsQueryContext {
3334
InetSocketAddress nameServerAddr,
3435
DnsQueryContextManager queryContextManager,
3536
int maxPayLoadSize, boolean recursionDesired,
37+
long queryTimeoutMillis,
3638
DnsQuestion question, DnsRecord[] additionals,
37-
Promise<AddressedEnvelope<DnsResponse, InetSocketAddress>> promise) {
39+
Promise<AddressedEnvelope<DnsResponse, InetSocketAddress>> promise,
40+
Bootstrap socketBootstrap, boolean retryWithTcpOnTimeout) {
3841
super(channel, channelReadyFuture, nameServerAddr, queryContextManager, maxPayLoadSize, recursionDesired,
39-
question, additionals, promise);
42+
queryTimeoutMillis, question, additionals, promise, socketBootstrap, retryWithTcpOnTimeout);
4043
}
4144

4245
@Override

‎resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java‎

Lines changed: 21 additions & 233 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import io.netty.channel.ChannelFactory;
2424
import io.netty.channel.ChannelFuture;
2525
import io.netty.channel.ChannelFutureListener;
26+
import io.netty.channel.ChannelHandler;
27+
import io.netty.channel.ChannelHandlerAdapter;
2628
import io.netty.channel.ChannelHandlerContext;
2729
import io.netty.channel.ChannelInboundHandlerAdapter;
2830
import io.netty.channel.ChannelInitializer;
@@ -43,8 +45,6 @@
4345
import io.netty.handler.codec.dns.DnsRecord;
4446
import io.netty.handler.codec.dns.DnsRecordType;
4547
import io.netty.handler.codec.dns.DnsResponse;
46-
import io.netty.handler.codec.dns.TcpDnsQueryEncoder;
47-
import io.netty.handler.codec.dns.TcpDnsResponseDecoder;
4848
import io.netty.resolver.DefaultHostsFileEntriesResolver;
4949
import io.netty.resolver.HostsFileEntries;
5050
import io.netty.resolver.HostsFileEntriesResolver;
@@ -121,6 +121,13 @@ public class DnsNameResolver extends InetNameResolver {
121121
private static final InternetProtocolFamily[] IPV6_PREFERRED_RESOLVED_PROTOCOL_FAMILIES =
122122
{InternetProtocolFamily.IPv6, InternetProtocolFamily.IPv4};
123123

124+
private static final ChannelHandler NOOP_HANDLER = new ChannelHandlerAdapter() {
125+
@Override
126+
public boolean isSharable() {
127+
return true;
128+
}
129+
};
130+
124131
static final ResolvedAddressTypes DEFAULT_RESOLVE_ADDRESS_TYPES;
125132
static final String[] DEFAULT_SEARCH_DOMAINS;
126133
private static final UnixResolverOptions DEFAULT_OPTIONS;
@@ -227,7 +234,6 @@ protected DnsResponse decodeResponse(ChannelHandlerContext ctx, DatagramPacket p
227234
}
228235
};
229236
private static final DatagramDnsQueryEncoder DATAGRAM_ENCODER = new DatagramDnsQueryEncoder();
230-
private static final TcpDnsQueryEncoder TCP_ENCODER = new TcpDnsQueryEncoder();
231237

232238
private final Promise<Channel> channelReadyPromise;
233239
private final Channel ch;
@@ -273,6 +279,7 @@ protected DnsServerAddressStream initialValue() {
273279
private final DnsQueryLifecycleObserverFactory dnsQueryLifecycleObserverFactory;
274280
private final boolean completeOncePreferredResolved;
275281
private final Bootstrap socketBootstrap;
282+
private final boolean retryWithTcpOnTimeout;
276283

277284
private final int maxNumConsolidation;
278285
private final Map<String, Future<List<InetAddress>>> inflightLookups;
@@ -376,44 +383,18 @@ public DnsNameResolver(
376383
String[] searchDomains,
377384
int ndots,
378385
boolean decodeIdn) {
379-
this(eventLoop, channelFactory, null, resolveCache, NoopDnsCnameCache.INSTANCE, authoritativeDnsServerCache,
386+
this(eventLoop, channelFactory, null, false, resolveCache,
387+
NoopDnsCnameCache.INSTANCE, authoritativeDnsServerCache, null,
380388
dnsQueryLifecycleObserverFactory, queryTimeoutMillis, resolvedAddressTypes, recursionDesired,
381389
maxQueriesPerResolve, traceEnabled, maxPayloadSize, optResourceEnabled, hostsFileEntriesResolver,
382-
dnsServerAddressStreamProvider, searchDomains, ndots, decodeIdn, false);
383-
}
384-
385-
DnsNameResolver(
386-
EventLoop eventLoop,
387-
ChannelFactory<? extends DatagramChannel> channelFactory,
388-
ChannelFactory<? extends SocketChannel> socketChannelFactory,
389-
final DnsCache resolveCache,
390-
final DnsCnameCache cnameCache,
391-
final AuthoritativeDnsServerCache authoritativeDnsServerCache,
392-
DnsQueryLifecycleObserverFactory dnsQueryLifecycleObserverFactory,
393-
long queryTimeoutMillis,
394-
ResolvedAddressTypes resolvedAddressTypes,
395-
boolean recursionDesired,
396-
int maxQueriesPerResolve,
397-
boolean traceEnabled,
398-
int maxPayloadSize,
399-
boolean optResourceEnabled,
400-
HostsFileEntriesResolver hostsFileEntriesResolver,
401-
DnsServerAddressStreamProvider dnsServerAddressStreamProvider,
402-
String[] searchDomains,
403-
int ndots,
404-
boolean decodeIdn,
405-
boolean completeOncePreferredResolved) {
406-
this(eventLoop, channelFactory, socketChannelFactory, resolveCache, cnameCache, authoritativeDnsServerCache,
407-
null, dnsQueryLifecycleObserverFactory, queryTimeoutMillis, resolvedAddressTypes,
408-
recursionDesired, maxQueriesPerResolve, traceEnabled, maxPayloadSize, optResourceEnabled,
409-
hostsFileEntriesResolver, dnsServerAddressStreamProvider, searchDomains, ndots, decodeIdn,
410-
completeOncePreferredResolved, 0);
390+
dnsServerAddressStreamProvider, searchDomains, ndots, decodeIdn, false, 0);
411391
}
412392

413393
DnsNameResolver(
414394
EventLoop eventLoop,
415395
ChannelFactory<? extends DatagramChannel> channelFactory,
416396
ChannelFactory<? extends SocketChannel> socketChannelFactory,
397+
boolean retryWithTcpOnTimeout,
417398
final DnsCache resolveCache,
418399
final DnsCnameCache cnameCache,
419400
final AuthoritativeDnsServerCache authoritativeDnsServerCache,
@@ -457,6 +438,7 @@ public DnsNameResolver(
457438
this.ndots = ndots >= 0 ? ndots : DEFAULT_OPTIONS.ndots();
458439
this.decodeIdn = decodeIdn;
459440
this.completeOncePreferredResolved = completeOncePreferredResolved;
441+
this.retryWithTcpOnTimeout = retryWithTcpOnTimeout;
460442
if (socketChannelFactory == null) {
461443
socketBootstrap = null;
462444
} else {
@@ -465,7 +447,7 @@ public DnsNameResolver(
465447
.group(executor())
466448
.channelFactory(socketChannelFactory)
467449
.attr(DNS_PIPELINE_ATTRIBUTE, Boolean.TRUE)
468-
.handler(TCP_ENCODER);
450+
.handler(NOOP_HANDLER);
469451
if (queryTimeoutMillis > 0 && queryTimeoutMillis <= Integer.MAX_VALUE) {
470452
// Set the connect timeout to the same as queryTimeout as otherwise it might take a long
471453
// time for the query to fail in case of a connection timeout.
@@ -1354,8 +1336,9 @@ final Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> query0(
13541336
final int payloadSize = isOptResourceEnabled() ? maxPayloadSize() : 0;
13551337
try {
13561338
DnsQueryContext queryContext = new DatagramDnsQueryContext(ch, channelReadyPromise, nameServerAddr,
1357-
queryContextManager, payloadSize, isRecursionDesired(), question, additionals, castPromise);
1358-
ChannelFuture future = queryContext.writeQuery(queryTimeoutMillis(), flush);
1339+
queryContextManager, payloadSize, isRecursionDesired(), queryTimeoutMillis(), question, additionals,
1340+
castPromise, socketBootstrap, retryWithTcpOnTimeout);
1341+
ChannelFuture future = queryContext.writeQuery(flush);
13591342
queryLifecycleObserver.queryWritten(nameServerAddr, future);
13601343
return castPromise;
13611344
} catch (Exception e) {
@@ -1400,94 +1383,8 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
14001383
return;
14011384
}
14021385

1403-
// Check if the response was truncated and if we can fallback to TCP to retry.
1404-
if (!res.isTruncated() || socketBootstrap == null) {
1405-
qCtx.finishSuccess(res);
1406-
return;
1407-
}
1408-
1409-
socketBootstrap.connect(res.sender()).addListener(new ChannelFutureListener() {
1410-
@Override
1411-
public void operationComplete(ChannelFuture future) {
1412-
if (!future.isSuccess()) {
1413-
logger.debug("{} Unable to fallback to TCP [{}: {}]",
1414-
ch, queryId, res.sender(), future.cause());
1415-
1416-
// TCP fallback failed, just use the truncated response.
1417-
qCtx.finishSuccess(res);
1418-
return;
1419-
}
1420-
final Channel tcpCh = future.channel();
1421-
1422-
Promise<AddressedEnvelope<DnsResponse, InetSocketAddress>> promise =
1423-
tcpCh.eventLoop().newPromise();
1424-
final int payloadSize = isOptResourceEnabled() ? maxPayloadSize() : 0;
1425-
final TcpDnsQueryContext tcpCtx = new TcpDnsQueryContext(tcpCh, channelReadyPromise,
1426-
(InetSocketAddress) tcpCh.remoteAddress(), queryContextManager, payloadSize,
1427-
isRecursionDesired(), qCtx.question(), EMPTY_ADDITIONALS, promise);
1428-
1429-
tcpCh.pipeline().addLast(new TcpDnsResponseDecoder());
1430-
tcpCh.pipeline().addLast(new ChannelInboundHandlerAdapter() {
1431-
@Override
1432-
public void channelRead(ChannelHandlerContext ctx, Object msg) {
1433-
Channel tcpCh = ctx.channel();
1434-
DnsResponse response = (DnsResponse) msg;
1435-
int queryId = response.id();
1436-
1437-
if (logger.isDebugEnabled()) {
1438-
logger.debug("{} RECEIVED: TCP [{}: {}], {}", tcpCh, queryId,
1439-
tcpCh.remoteAddress(), response);
1440-
}
1441-
1442-
DnsQueryContext foundCtx = queryContextManager.get(res.sender(), queryId);
1443-
if (foundCtx != null && foundCtx.isDone()) {
1444-
logger.debug("{} Received a DNS response for a query that was timed out or cancelled " +
1445-
": TCP [{}: {}]", tcpCh, queryId, res.sender());
1446-
response.release();
1447-
} else if (foundCtx == tcpCtx) {
1448-
tcpCtx.finishSuccess(new AddressedEnvelopeAdapter(
1449-
(InetSocketAddress) ctx.channel().remoteAddress(),
1450-
(InetSocketAddress) ctx.channel().localAddress(),
1451-
response));
1452-
} else {
1453-
response.release();
1454-
tcpCtx.finishFailure("Received TCP DNS response with unexpected ID", null, false);
1455-
if (logger.isDebugEnabled()) {
1456-
logger.debug("{} Received a DNS response with an unexpected ID: TCP [{}: {}]",
1457-
tcpCh, queryId, tcpCh.remoteAddress());
1458-
}
1459-
}
1460-
}
1461-
1462-
@Override
1463-
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
1464-
if (tcpCtx.finishFailure(
1465-
"TCP fallback error", cause, false) && logger.isDebugEnabled()) {
1466-
logger.debug("{} Error during processing response: TCP [{}: {}]",
1467-
ctx.channel(), queryId,
1468-
ctx.channel().remoteAddress(), cause);
1469-
}
1470-
}
1471-
});
1472-
1473-
promise.addListener(
1474-
new FutureListener<AddressedEnvelope<DnsResponse, InetSocketAddress>>() {
1475-
@Override
1476-
public void operationComplete(
1477-
Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> future) {
1478-
if (future.isSuccess()) {
1479-
qCtx.finishSuccess(future.getNow());
1480-
res.release();
1481-
} else {
1482-
// TCP fallback failed, just use the truncated response.
1483-
qCtx.finishSuccess(res);
1484-
}
1485-
tcpCh.close();
1486-
}
1487-
});
1488-
tcpCtx.writeQuery(queryTimeoutMillis(), true);
1489-
}
1490-
});
1386+
// The context will handle truncation itself.
1387+
qCtx.finishSuccess(res, res.isTruncated());
14911388
}
14921389

14931390
@Override
@@ -1505,113 +1402,4 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
15051402
}
15061403
}
15071404
}
1508-
1509-
private static final class AddressedEnvelopeAdapter implements AddressedEnvelope<DnsResponse, InetSocketAddress> {
1510-
private final InetSocketAddress sender;
1511-
private final InetSocketAddress recipient;
1512-
private final DnsResponse response;
1513-
1514-
AddressedEnvelopeAdapter(InetSocketAddress sender, InetSocketAddress recipient, DnsResponse response) {
1515-
this.sender = sender;
1516-
this.recipient = recipient;
1517-
this.response = response;
1518-
}
1519-
1520-
@Override
1521-
public DnsResponse content() {
1522-
return response;
1523-
}
1524-
1525-
@Override
1526-
public InetSocketAddress sender() {
1527-
return sender;
1528-
}
1529-
1530-
@Override
1531-
public InetSocketAddress recipient() {
1532-
return recipient;
1533-
}
1534-
1535-
@Override
1536-
public AddressedEnvelope<DnsResponse, InetSocketAddress> retain() {
1537-
response.retain();
1538-
return this;
1539-
}
1540-
1541-
@Override
1542-
public AddressedEnvelope<DnsResponse, InetSocketAddress> retain(int increment) {
1543-
response.retain(increment);
1544-
return this;
1545-
}
1546-
1547-
@Override
1548-
public AddressedEnvelope<DnsResponse, InetSocketAddress> touch() {
1549-
response.touch();
1550-
return this;
1551-
}
1552-
1553-
@Override
1554-
public AddressedEnvelope<DnsResponse, InetSocketAddress> touch(Object hint) {
1555-
response.touch(hint);
1556-
return this;
1557-
}
1558-
1559-
@Override
1560-
public int refCnt() {
1561-
return response.refCnt();
1562-
}
1563-
1564-
@Override
1565-
public boolean release() {
1566-
return response.release();
1567-
}
1568-
1569-
@Override
1570-
public boolean release(int decrement) {
1571-
return response.release(decrement);
1572-
}
1573-
1574-
@Override
1575-
public boolean equals(Object obj) {
1576-
if (this == obj) {
1577-
return true;
1578-
}
1579-
1580-
if (!(obj instanceof AddressedEnvelope)) {
1581-
return false;
1582-
}
1583-
1584-
@SuppressWarnings("unchecked")
1585-
final AddressedEnvelope<?, SocketAddress> that = (AddressedEnvelope<?, SocketAddress>) obj;
1586-
if (sender() == null) {
1587-
if (that.sender() != null) {
1588-
return false;
1589-
}
1590-
} else if (!sender().equals(that.sender())) {
1591-
return false;
1592-
}
1593-
1594-
if (recipient() == null) {
1595-
if (that.recipient() != null) {
1596-
return false;
1597-
}
1598-
} else if (!recipient().equals(that.recipient())) {
1599-
return false;
1600-
}
1601-
1602-
return response.equals(obj);
1603-
}
1604-
1605-
@Override
1606-
public int hashCode() {
1607-
int hashCode = response.hashCode();
1608-
if (sender() != null) {
1609-
hashCode = hashCode * 31 + sender().hashCode();
1610-
}
1611-
if (recipient() != null) {
1612-
hashCode = hashCode * 31 + recipient().hashCode();
1613-
}
1614-
return hashCode;
1615-
}
1616-
}
16171405
}

0 commit comments

Comments
 (0)