Skip to content

Commit

Permalink
Add experimental system properties for Netty DnsNameResolver (#3073)
Browse files Browse the repository at this point in the history 
Motivation:

Netty recently introduced 2 more DNS config options that help to
workaround known infrastructure issues:
- TCP fallback on timeout;
- `DnsNameResolverChannelStrategy`;

Modifications:

Add 2 new experimental system properties:
1. `io.servicetalk.dns.discovery.netty.experimental.tcpFallbackOnTimeout`
2. `io.servicetalk.dns.discovery.netty.experimental.datagramChannelStrategy`

Result:

We can test these new features without commiting to them in our public
API.
  • Loading branch information
@idelpivnitskiy
idelpivnitskiy authored Oct 4, 2024
1 parent 94c1bbb commit 6613bd5
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 14 deletions.
14 changes: 9 additions & 5 deletions ...ns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2018, 2021-2023 Apple Inc. and the ServiceTalk project authors
* Copyright © 2018, 2021-2024 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -158,7 +158,9 @@ final class DefaultDnsClient implements DnsClient {
@Nullable final DnsServerAddressStreamProvider dnsServerAddressStreamProvider,
@Nullable final DnsServiceDiscovererObserver observer,
final ServiceDiscovererEvent.Status missingRecordStatus,
final boolean nxInvalidation) {
final boolean nxInvalidation,
final boolean tcpFallbackOnTimeout,
final String datagramChannelStrategy) {
this.srvConcurrency = srvConcurrency;
this.srvFilterDuplicateEvents = srvFilterDuplicateEvents;
// Implementation of this class expects to use only single EventLoop from IoExecutor
Expand Down Expand Up @@ -193,9 +195,6 @@ final class DefaultDnsClient implements DnsClient {
.localAddress(localAddress)
.channelType(datagramChannel(eventLoop))
.resolvedAddressTypes(resolvedAddressTypes)
// Enable TCP fallback to be able to handle truncated responses.
// https://tools.ietf.org/html/rfc7766
.socketChannelType(socketChannelClass)
// We should complete once the preferred address types could be resolved to ensure we always
// respond as fast as possible.
.completeOncePreferredResolved(completeOncePreferredResolved)
Expand All @@ -209,7 +208,12 @@ final class DefaultDnsClient implements DnsClient {
// Use the same comparator as Netty uses by default.
new NameServerComparator(preferredAddressType(resolvedAddressTypes).addressType())));

// Enable TCP fallback to be able to handle truncated responses.
// https://tools.ietf.org/html/rfc7766
DnsNameResolverBuilderUtils.enableTcpFallback(id, builder, socketChannelClass, tcpFallbackOnTimeout);
DnsNameResolverBuilderUtils.consolidateCacheSize(id, builder, consolidateCacheSize);
DnsNameResolverBuilderUtils.datagramChannelStrategy(id, builder, datagramChannelStrategy);

if (queryTimeout != null) {
builder.queryTimeoutMillis(queryTimeout.toMillis());
}
Expand Down
27 changes: 20 additions & 7 deletions .../src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2018, 2021-2023 Apple Inc. and the ServiceTalk project authors
* Copyright © 2018, 2021-2024 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -58,13 +58,23 @@ public final class DefaultDnsServiceDiscovererBuilder implements DnsServiceDisco

private static final Logger LOGGER = LoggerFactory.getLogger(DefaultDnsServiceDiscovererBuilder.class);

/**
* @deprecated This system property is introduced temporarily as a way for users to skip binding and revert the
* preexisting behavior.
*/
@Deprecated // FIXME: 0.43 - consider removing this system property
// FIXME: 0.43 - consider removing deprecated system properties.
// Those were introduced temporarily as a way for us to experiment with new Netty features.
// In the next major release, we should promote required features to builder API.
@Deprecated
private static final String DATAGRAM_CHANNEL_STRATEGY_PROPERTY =
"io.servicetalk.dns.discovery.netty.experimental.datagramChannelStrategy";
@Deprecated
private static final String TCP_FALLBACK_ON_TIMEOUT_PROPERTY =
"io.servicetalk.dns.discovery.netty.experimental.tcpFallbackOnTimeout";
@Deprecated
private static final String SKIP_BINDING_PROPERTY = "io.servicetalk.dns.discovery.netty.skipBinding";
@Deprecated
private static final String NX_DOMAIN_INVALIDATES_PROPERTY = "io.servicetalk.dns.discovery.nxdomain.invalidation";

private static final String DEFAULT_DATAGRAM_CHANNEL_STRATEGY =
getProperty(DATAGRAM_CHANNEL_STRATEGY_PROPERTY, "ChannelPerResolver");
private static final boolean DEFAULT_TCP_FALLBACK_ON_TIMEOUT = getBoolean(TCP_FALLBACK_ON_TIMEOUT_PROPERTY);
private static final boolean DEFAULT_NX_DOMAIN_INVALIDATES = getBoolean(NX_DOMAIN_INVALIDATES_PROPERTY);
@Nullable
private static final SocketAddress DEFAULT_LOCAL_ADDRESS =
Expand Down Expand Up @@ -106,6 +116,8 @@ public final class DefaultDnsServiceDiscovererBuilder implements DnsServiceDisco
LOGGER.debug("Default negative TTL cache in seconds: {}", DEFAULT_NEGATIVE_TTL_CACHE_SECONDS);
LOGGER.debug("Default missing records status: {}", DEFAULT_MISSING_RECOREDS_STATUS);
LOGGER.debug("-D{}: {}", NX_DOMAIN_INVALIDATES_PROPERTY, DEFAULT_NX_DOMAIN_INVALIDATES);
LOGGER.debug("-D{}: {}", TCP_FALLBACK_ON_TIMEOUT_PROPERTY, DEFAULT_TCP_FALLBACK_ON_TIMEOUT);
LOGGER.debug("-D{}: {}", DATAGRAM_CHANNEL_STRATEGY_PROPERTY, DEFAULT_DATAGRAM_CHANNEL_STRATEGY);
}
}

Expand Down Expand Up @@ -396,7 +408,8 @@ DnsClient build() {
srvConcurrency, completeOncePreferredResolved, srvFilterDuplicateEvents,
srvHostNameRepeatInitialDelay, srvHostNameRepeatJitter, maxUdpPayloadSize, ndots, optResourceEnabled,
queryTimeout, resolutionTimeout, dnsResolverAddressTypes, localAddress, dnsServerAddressStreamProvider,
observer, missingRecordStatus, nxInvalidation);
observer, missingRecordStatus, nxInvalidation,
DEFAULT_TCP_FALLBACK_ON_TIMEOUT, DEFAULT_DATAGRAM_CHANNEL_STRATEGY);
return filterFactory == null ? rawClient : filterFactory.create(rawClient);
}

Expand Down
125 changes: 123 additions & 2 deletions ...y-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsNameResolverBuilderUtils.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2023 Apple Inc. and the ServiceTalk project authors
* Copyright © 2023-2024 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,7 +15,10 @@
*/
package io.servicetalk.dns.discovery.netty;

import io.netty.channel.socket.SocketChannel;
import io.netty.channel.socket.nio.NioSocketChannel;
import io.netty.resolver.dns.DnsNameResolverBuilder;
import io.netty.resolver.dns.DnsNameResolverChannelStrategy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -31,11 +34,18 @@ final class DnsNameResolverBuilderUtils {

private static final Logger LOGGER = LoggerFactory.getLogger(DnsNameResolverBuilderUtils.class);
private static final String NETTY_VERSION = DnsNameResolverBuilder.class.getPackage().getImplementationVersion();
private static final String DEFAULT_DATAGRAM_CHANNEL_STRATEGY = "ChannelPerResolver";

@Nullable
private static final MethodHandle CONSOLIDATE_CACHE_SIZE;
@Nullable
private static final MethodHandle TCP_FALLBACK_ON_TIMEOUT;
@Nullable
private static final MethodHandle DATAGRAM_CHANNEL_STRATEGY;

static {
final DnsNameResolverBuilder builder = new DnsNameResolverBuilder();

MethodHandle consolidateCacheSize;
try {
// Find a new method that exists only in Netty starting from 4.1.88.Final:
Expand All @@ -44,19 +54,54 @@ final class DnsNameResolverBuilderUtils {
.findVirtual(DnsNameResolverBuilder.class, "consolidateCacheSize",
methodType(DnsNameResolverBuilder.class, int.class));
// Verify the method is working as expected:
consolidateCacheSize(consolidateCacheSize, new DnsNameResolverBuilder(), 1);
consolidateCacheSize(consolidateCacheSize, builder, 1);
} catch (Throwable cause) {
LOGGER.debug("DnsNameResolverBuilder#consolidateCacheSize(int) is available only starting from " +
"Netty 4.1.88.Final. Detected Netty version: {}", NETTY_VERSION, cause);
consolidateCacheSize = null;
}
CONSOLIDATE_CACHE_SIZE = consolidateCacheSize;

MethodHandle tcpFallbackOnTimeout;
try {
// Find a new method that exists only in Netty starting from 4.1.105.Final:
// https://github.com/netty/netty/commit/684dfd88e319bb7870d88977bd6a63d5fea765c0
tcpFallbackOnTimeout = MethodHandles.publicLookup()
.findVirtual(DnsNameResolverBuilder.class, "socketChannelType",
methodType(DnsNameResolverBuilder.class, Class.class, boolean.class));
// Verify the method is working as expected:
enableTcpFallback(tcpFallbackOnTimeout, builder, NioSocketChannel.class, true);
} catch (Throwable cause) {
LOGGER.debug("DnsNameResolverBuilder#socketChannelType(Class, boolean) is available only starting from " +
"Netty 4.1.105.Final. Detected Netty version: {}", NETTY_VERSION, cause);
tcpFallbackOnTimeout = null;
}
TCP_FALLBACK_ON_TIMEOUT = tcpFallbackOnTimeout;

MethodHandle datagramChannelStrategy;
try {
// Find a new method that exists only in Netty starting from 4.1.114.Final:
// https://github.com/netty/netty/commit/d5f4bfb6c9ca14bd5820fa61a9ce3352492de872
datagramChannelStrategy = MethodHandles.publicLookup()
.findVirtual(DnsNameResolverBuilder.class, "datagramChannelStrategy",
methodType(DnsNameResolverBuilder.class,
Class.forName("io.netty.resolver.dns.DnsNameResolverChannelStrategy")));
// Verify the method is working as expected:
datagramChannelStrategy(datagramChannelStrategy, builder, DEFAULT_DATAGRAM_CHANNEL_STRATEGY);
} catch (Throwable cause) {
LOGGER.debug("DnsNameResolverBuilder#datagramChannelStrategy(DnsNameResolverChannelStrategy) is " +
"available only starting from Netty 4.1.114.Final. Detected Netty version: {}",
NETTY_VERSION, cause);
datagramChannelStrategy = null;
}
DATAGRAM_CHANNEL_STRATEGY = datagramChannelStrategy;
}

private DnsNameResolverBuilderUtils() {
// No instances
}

@SuppressWarnings("UnusedReturnValue")
private static DnsNameResolverBuilder consolidateCacheSize(final MethodHandle consolidateCacheSize,
final DnsNameResolverBuilder builder,
final int maxNumConsolidation) {
Expand All @@ -69,6 +114,35 @@ private static DnsNameResolverBuilder consolidateCacheSize(final MethodHandle co
}
}

@SuppressWarnings("UnusedReturnValue")
private static DnsNameResolverBuilder enableTcpFallback(final MethodHandle tcpFallbackOnTimeout,
final DnsNameResolverBuilder builder,
final Class<? extends SocketChannel> socketChannelClass,
final boolean retryOnTimeout) {
try {
// invokeExact requires return type cast to match the type signature
return (DnsNameResolverBuilder) tcpFallbackOnTimeout
.invokeExact(builder, socketChannelClass, retryOnTimeout);
} catch (Throwable t) {
throwException(t);
return builder;
}
}

@SuppressWarnings("UnusedReturnValue")
private static DnsNameResolverBuilder datagramChannelStrategy(final MethodHandle datagramChannelStrategy,
final DnsNameResolverBuilder builder,
final String strategy) {
try {
// invokeExact requires return type cast to match the type signature
return (DnsNameResolverBuilder) datagramChannelStrategy.invokeExact(builder,
LazyHolder.toNettyStrategy(strategy));
} catch (Throwable t) {
throwException(t);
return builder;
}
}

static void consolidateCacheSize(final String id,
final DnsNameResolverBuilder builder,
final int maxNumConsolidation) {
Expand All @@ -83,4 +157,51 @@ static void consolidateCacheSize(final String id,
}
consolidateCacheSize(CONSOLIDATE_CACHE_SIZE, builder, maxNumConsolidation);
}

static void enableTcpFallback(final String id,
final DnsNameResolverBuilder builder,
final Class<? extends SocketChannel> socketChannelClass,
final boolean retryOnTimeout) {
if (TCP_FALLBACK_ON_TIMEOUT == null) {
if (retryOnTimeout) {
LOGGER.warn("tcpFallbackOnTimeout({}) can not be applied for a new DNS ServiceDiscoverer '{}' " +
"because io.netty.resolver.dns.DnsNameResolverBuilder#socketChannelType(Class, boolean) " +
"method is not available in Netty {}, expected Netty version is 4.1.105.Final or later.",
retryOnTimeout, id, NETTY_VERSION);
}
// Still configure TCP fallback for truncated responses.
builder.socketChannelType(socketChannelClass);
return;
}
enableTcpFallback(TCP_FALLBACK_ON_TIMEOUT, builder, socketChannelClass, retryOnTimeout);
}

static void datagramChannelStrategy(final String id,
final DnsNameResolverBuilder builder,
final String datagramChannelStrategy) {
if (DATAGRAM_CHANNEL_STRATEGY == null) {
if (!DEFAULT_DATAGRAM_CHANNEL_STRATEGY.equals(datagramChannelStrategy)) {
LOGGER.warn("datagramChannelStrategy({}) can not be applied for a new DNS ServiceDiscoverer '{}' " +
"because io.netty.resolver.dns.DnsNameResolverBuilder#datagramChannelStrategy(...) " +
"method is not available in Netty {}, expected Netty version is 4.1.114.Final or later.",
datagramChannelStrategy, id, NETTY_VERSION);
}
return;
}
datagramChannelStrategy(DATAGRAM_CHANNEL_STRATEGY, builder, datagramChannelStrategy);
}

// Defer loading of the class that is available only in Netty 4.1.114.Final or later versions.
private static final class LazyHolder {
static DnsNameResolverChannelStrategy toNettyStrategy(final String strategy) {
try {
return DnsNameResolverChannelStrategy.valueOf(strategy);
} catch (IllegalArgumentException e) {
DnsNameResolverChannelStrategy fallback = DnsNameResolverChannelStrategy.ChannelPerResolver;
LOGGER.warn("Unknown {} value: {}. Fallback to {}",
DnsNameResolverChannelStrategy.class.getName(), strategy, fallback);
return fallback;
}
}
}
}

0 comments on commit 6613bd5

Please sign in to comment.