Repository: netty/netty (Java)

Issue:
DNSResolve: DNS Cache Not Working in Certain Scenarios
1. In k8s environment, the concatenation of the hostname and searchDomain, can cause the DNS cache to become ineffective. I think if searchDomains are concatenated during DNS resolution, we should iterate through and concatenate all searchDomains to obtain the cached results.

   eg. 
   hostname: k8s service name
![image-20240510234145365](https://github.com/netty/netty/assets/65326092/9294e011-80d2-412e-933e-21d2daa65718)


2. Each EpollEventLoop has its own DNS resolution cache, but when DNS resolution is executed in an EpollEventLoop that does not have a cached result, it can cause the DNS resolution cache to become ineffective.

This looks like a potential bug. Can it be assigned to me? I would like to try fixing it.

### Netty version
4.1.73.Final
### JVM version (e.g. `java -version`)
jdk8
### OS version (e.g. `uname -a`)
k8s 5.15.0-101-generic #111-Ubuntu SMP Tue Mar 5 20:16:58 UTC 2024 x86_64 GNU/Linux


Hints:
You are welcome to try fixing it. I recommend first making sure it's still a bug in the latest version. Then see if you can capture it in an automated test. Also feel free to discuss the ideas you have for how to fix it. 
> You are welcome to try fixing it. I recommend first making sure it's still a bug in the latest version. Then see if you can capture it in an automated test. Also feel free to discuss the ideas you have for how to fix it.

This bug still exists in the latest version. If each type of DNSCache implementation has its own global cache, it should be able to resolve the issue mentioned above, and the cost of fixing it would be relatively low. In the ResolveWithDotSearchDomain scenario, I have made some small modifications by iterating through all searchDomains and matching 'hostname+ '.' +searchDomain' to search for results in the DNSCache. Would this be an appropriate solution?
please review: https://github.com/netty/netty/compare/4.1...qnnn:netty:4.1

Best wishes!
@chrisvest Or, we could initialize the DNSCache by default in the DnsAddressResolverGroup during initialization. If the DnsNameResolverBuilder does not already have a DNSCache, we can use the same DNSCache when creating the NameResolver. WDYT?

Patch:
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java
@@ -45,23 +45,31 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
     private final ConcurrentMap<String, Promise<List<InetAddress>>> resolveAllsInProgress = newConcurrentHashMap();
 
     public DnsAddressResolverGroup(DnsNameResolverBuilder dnsResolverBuilder) {
-        this.dnsResolverBuilder = dnsResolverBuilder.copy();
+        this.dnsResolverBuilder = withSharedCaches(dnsResolverBuilder.copy());
     }
 
     public DnsAddressResolverGroup(
             Class<? extends DatagramChannel> channelType,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this.dnsResolverBuilder = new DnsNameResolverBuilder();
+        this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
         dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider);
     }
 
     public DnsAddressResolverGroup(
             ChannelFactory<? extends DatagramChannel> channelFactory,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this.dnsResolverBuilder = new DnsNameResolverBuilder();
+        this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
         dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider);
     }
 
+    private static DnsNameResolverBuilder withSharedCaches(DnsNameResolverBuilder dnsResolverBuilder) {
+        /// To avoid each member of the group having its own cache we either use the configured cache
+        // or create a new one to share among the entire group.
+        return dnsResolverBuilder.resolveCache(dnsResolverBuilder.getOrNewCache())
+                .cnameCache(dnsResolverBuilder.getOrNewCnameCache())
+                .authoritativeDnsServerCache(dnsResolverBuilder.getOrNewAuthoritativeDnsServerCache());
+    }
+
     @SuppressWarnings("deprecation")
     @Override
     protected final AddressResolver<InetSocketAddress> newResolver(EventExecutor executor) throws Exception {
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
@@ -513,11 +513,17 @@ public DnsNameResolverBuilder ndots(int ndots) {
         return this;
     }
 
-    private DnsCache newCache() {
+   DnsCache getOrNewCache() {
+        if (this.resolveCache != null) {
+            return this.resolveCache;
+        }
         return new DefaultDnsCache(intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE), intValue(negativeTtl, 0));
     }
 
-    private AuthoritativeDnsServerCache newAuthoritativeDnsServerCache() {
+   AuthoritativeDnsServerCache getOrNewAuthoritativeDnsServerCache() {
+        if (this.authoritativeDnsServerCache != null) {
+            return this.authoritativeDnsServerCache;
+        }
         return new DefaultAuthoritativeDnsServerCache(
                 intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE),
                 // Let us use the sane ordering as DnsNameResolver will be used when returning
@@ -530,7 +536,10 @@ private DnsServerAddressStream newQueryServerAddressStream(
         return new ThreadLocalNameServerAddressStream(dnsServerAddressStreamProvider);
     }
 
-    private DnsCnameCache newCnameCache() {
+   DnsCnameCache getOrNewCnameCache() {
+        if (this.cnameCache != null) {
+            return this.cnameCache;
+        }
         return new DefaultDnsCnameCache(
                 intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE));
     }
@@ -583,10 +592,9 @@ public DnsNameResolver build() {
             logger.debug("authoritativeDnsServerCache and TTLs are mutually exclusive. TTLs are ignored.");
         }
 
-        DnsCache resolveCache = this.resolveCache != null ? this.resolveCache : newCache();
-        DnsCnameCache cnameCache = this.cnameCache != null ? this.cnameCache : newCnameCache();
-        AuthoritativeDnsServerCache authoritativeDnsServerCache = this.authoritativeDnsServerCache != null ?
-                this.authoritativeDnsServerCache : newAuthoritativeDnsServerCache();
+        DnsCache resolveCache = getOrNewCache();
+        DnsCnameCache cnameCache = getOrNewCnameCache();
+        AuthoritativeDnsServerCache authoritativeDnsServerCache = getOrNewAuthoritativeDnsServerCache();
 
         DnsServerAddressStream queryDnsServerAddressStream = this.queryDnsServerAddressStream != null ?
                 this.queryDnsServerAddressStream : newQueryServerAddressStream(dnsServerAddressStreamProvider);


Test patch:
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java
--- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java
+++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java
@@ -20,16 +20,21 @@
 import io.netty.channel.nio.NioEventLoopGroup;
 import io.netty.channel.socket.nio.NioDatagramChannel;
 import io.netty.resolver.AddressResolver;
+import io.netty.resolver.InetSocketAddressResolver;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.FutureListener;
 import io.netty.util.concurrent.Promise;
 import org.junit.jupiter.api.Test;
 
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.channels.UnsupportedAddressTypeException;
+import java.util.concurrent.ExecutionException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class DnsAddressResolverGroupTest {
@@ -66,4 +71,50 @@ public void operationComplete(Future<Object> future) {
             defaultEventLoopGroup.shutdownGracefully();
         }
     }
+
+    @Test
+    public void testSharedDNSCacheAcrossEventLoops() throws InterruptedException, ExecutionException {
+        NioEventLoopGroup group = new NioEventLoopGroup(1);
+        final EventLoop loop = group.next();
+        DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
+                .eventLoop(loop).channelType(NioDatagramChannel.class);
+        DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(builder);
+        DefaultEventLoopGroup defaultEventLoopGroup = new DefaultEventLoopGroup(2);
+        EventLoop eventLoop1 = defaultEventLoopGroup.next();
+        EventLoop eventLoop2 = defaultEventLoopGroup.next();
+        try {
+            final Promise<InetSocketAddress> promise1 = loop.newPromise();
+            InetSocketAddressResolver resolver1 = (InetSocketAddressResolver) resolverGroup.getResolver(eventLoop1);
+            InetAddress address1 =
+                    resolve(resolver1, InetSocketAddress.createUnresolved("netty.io", 80), promise1);
+            final Promise<InetSocketAddress> promise2 = loop.newPromise();
+            InetSocketAddressResolver resolver2 = (InetSocketAddressResolver) resolverGroup.getResolver(eventLoop2);
+            InetAddress address2 =
+                    resolve(resolver2, InetSocketAddress.createUnresolved("netty.io", 80), promise2);
+            assertSame(address1, address2);
+        } finally {
+            resolverGroup.close();
+            group.shutdownGracefully();
+            defaultEventLoopGroup.shutdownGracefully();
+        }
+    }
+
+    private InetAddress resolve(InetSocketAddressResolver resolver, SocketAddress socketAddress,
+                                final Promise<InetSocketAddress> promise)
+            throws InterruptedException, ExecutionException {
+        resolver.resolve(socketAddress)
+                .addListener(new FutureListener<InetSocketAddress>() {
+                    @Override
+                    public void operationComplete(Future<InetSocketAddress> future) {
+                        try {
+                            promise.setSuccess(future.get());
+                        } catch (Throwable cause) {
+                            promise.setFailure(cause);
+                        }
+                    }
+                }).await();
+        promise.sync();
+        InetSocketAddress inetSocketAddress = promise.get();
+        return inetSocketAddress.getAddress();
+    }
 }