Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]

["ptupitsyn (via GitHub)" <gi...@apache.org>]

ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2570836122


##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
      * @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
      *         in {@code addrs} parameter.
      */
-    private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+    private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
         if (addrs == null || addrs.length == 0) {
             throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
         }
 
-        Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+        Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
 
         for (String a : addrs) {
-            ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+            parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
         }
 
-        return ranges.stream()
-                .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
-                .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+        var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+        for (HostAndPort addr : parsedAddrs) {
+            try {
+                // Special handling for "localhost" to avoid unnecessary DNS resolution.
+                if ("localhost".equalsIgnoreCase(addr.host())) {
+                    map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+                    continue;

Review Comment:
   Does `getAllByName` really return multiple IPs for localhost?



##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
      * @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
      *         in {@code addrs} parameter.
      */
-    private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+    private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
         if (addrs == null || addrs.length == 0) {
             throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
         }
 
-        Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+        Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
 
         for (String a : addrs) {
-            ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+            parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
         }
 
-        return ranges.stream()
-                .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
-                .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+        var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+        for (HostAndPort addr : parsedAddrs) {
+            try {
+                // Special handling for "localhost" to avoid unnecessary DNS resolution.
+                if ("localhost".equalsIgnoreCase(addr.host())) {
+                    map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+                    continue;
+                }
+
+                for (InetAddress inetAddr : addressResolver.getAllByName(addr.host())) {
+                    // Preserve unresolved address if the resolved address equals to the original host string.

Review Comment:
   Why does the order change?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org