From 6613bd588a4396c37f5d8f5e6705b75518d73e29 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Fri, 4 Oct 2024 02:11:06 -0700 Subject: [PATCH] Add experimental system properties for Netty `DnsNameResolver` (#3073) 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. --- .../dns/discovery/netty/DefaultDnsClient.java | 14 +- .../DefaultDnsServiceDiscovererBuilder.java | 27 +++- .../netty/DnsNameResolverBuilderUtils.java | 125 +++++++++++++++++- 3 files changed, 152 insertions(+), 14 deletions(-) diff --git a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java index b909dc4bf0..01be9371e9 100644 --- a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java +++ b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java @@ -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. @@ -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 @@ -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) @@ -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()); } diff --git a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java index 816a3050f1..34c87b5ce8 100644 --- a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java +++ b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java @@ -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. @@ -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 = @@ -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); } } @@ -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); } diff --git a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsNameResolverBuilderUtils.java b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsNameResolverBuilderUtils.java index 7c8c465aad..695d370e48 100644 --- a/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsNameResolverBuilderUtils.java +++ b/servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsNameResolverBuilderUtils.java @@ -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. @@ -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; @@ -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: @@ -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) { @@ -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 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) { @@ -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 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; + } + } + } }