kerneltime commented on code in PR #10487:
URL: https://github.com/apache/ozone/pull/10487#discussion_r3418804676


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java:
##########
@@ -260,6 +289,85 @@ public synchronized void close() throws IOException {
     }
   }
 
+  /**
+   * Re-resolve the configured hostname for the given SCM nodeId. If DNS
+   * now returns a different IP, swap in a fresh {@link SCMProxyInfo}
+   * (with the new resolved address) and discard any cached proxy so the
+   * next {@link #getProxy()} call dials the new IP.
+   *
+   * @return true when a swap occurred; false when the hostname was not
+   *         preserved, the IP is unchanged, the lookup failed, or the
+   *         nodeId is unknown.
+   */
+  boolean refreshProxyAddressIfChanged(String nodeId) {
+    // Read the cached info first so we can do the DNS lookup outside
+    // any monitor. A slow / dead resolver while holding the provider
+    // monitor would freeze every concurrent getProxy() / shouldRetry()
+    // caller.
+    String hostAndPort;
+    InetSocketAddress cachedAddress;
+    String serviceId;
+    synchronized (this) {
+      SCMProxyInfo cached = scmProxyInfoMap.get(nodeId);
+      if (cached == null) {
+        return false;
+      }
+      hostAndPort = cached.getHostAndPort();
+      if (hostAndPort == null) {
+        return false;
+      }
+      cachedAddress = cached.getAddress();
+      serviceId = cached.getServiceId();
+    }
+    InetSocketAddress refreshed;
+    try {
+      refreshed = NetUtils.createSocketAddr(hostAndPort);
+    } catch (IllegalArgumentException ex) {
+      getLogger().warn("Failed to re-resolve SCM address {}: {}",
+          hostAndPort, ex.getMessage());
+      return false;
+    }

Review Comment:
   Done in 207c253 -- the createSocketAddr catch now passes the throwable to 
SLF4J (message plus ex as the trailing arg), so the full stack trace is 
preserved for parse/DNS failures instead of just the message.



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java:
##########
@@ -260,6 +289,85 @@ public synchronized void close() throws IOException {
     }
   }
 
+  /**
+   * Re-resolve the configured hostname for the given SCM nodeId. If DNS
+   * now returns a different IP, swap in a fresh {@link SCMProxyInfo}
+   * (with the new resolved address) and discard any cached proxy so the
+   * next {@link #getProxy()} call dials the new IP.
+   *
+   * @return true when a swap occurred; false when the hostname was not
+   *         preserved, the IP is unchanged, the lookup failed, or the
+   *         nodeId is unknown.
+   */
+  boolean refreshProxyAddressIfChanged(String nodeId) {
+    // Read the cached info first so we can do the DNS lookup outside
+    // any monitor. A slow / dead resolver while holding the provider
+    // monitor would freeze every concurrent getProxy() / shouldRetry()
+    // caller.
+    String hostAndPort;
+    InetSocketAddress cachedAddress;
+    String serviceId;
+    synchronized (this) {
+      SCMProxyInfo cached = scmProxyInfoMap.get(nodeId);
+      if (cached == null) {
+        return false;
+      }
+      hostAndPort = cached.getHostAndPort();
+      if (hostAndPort == null) {
+        return false;
+      }
+      cachedAddress = cached.getAddress();
+      serviceId = cached.getServiceId();
+    }
+    InetSocketAddress refreshed;
+    try {
+      refreshed = NetUtils.createSocketAddr(hostAndPort);
+    } catch (IllegalArgumentException ex) {
+      getLogger().warn("Failed to re-resolve SCM address {}: {}",
+          hostAndPort, ex.getMessage());
+      return false;
+    }
+    if (refreshed.isUnresolved()) {
+      getLogger().warn("SCM hostname {} re-resolved to an unresolved "
+          + "address; leaving cached entry in place.", hostAndPort);
+      return false;
+    }
+    // Null-safe IP comparison. SCMProxyInfo's constructor allows
+    // an unresolved cached address (warns but stores). In that case
+    // cachedAddress.getAddress() is null and a successful
+    // re-resolution is genuinely a change -- proceed to swap rather
+    // than NPE on .equals().
+    java.net.InetAddress cachedIp = cachedAddress.getAddress();
+    if (cachedIp != null
+        && refreshed.getAddress().equals(cachedIp)) {
+      return false;
+    }
+    SCMProxyInfo updated = new SCMProxyInfo(serviceId, nodeId,
+        refreshed, hostAndPort);
+    ProxyInfo<T> staleProxy;
+    synchronized (this) {
+      // Re-check under the lock to avoid a lost update if another
+      // refresher beat us to the swap.
+      SCMProxyInfo current = scmProxyInfoMap.get(nodeId);
+      if (current == null || !cachedAddress.equals(current.getAddress())) {
+        return false;
+      }
+      scmProxyInfoMap.put(nodeId, updated);
+      staleProxy = scmProxies.remove(nodeId);
+    }
+    if (staleProxy != null && staleProxy.proxy != null) {
+      try {
+        RPC.stopProxy(staleProxy.proxy);
+      } catch (RuntimeException stopEx) {
+        getLogger().warn("Failed to stop stale proxy for SCM nodeId {}: {}",
+            nodeId, stopEx.getMessage());
+      }
+    }

Review Comment:
   Done in 207c253 -- the RPC.stopProxy catch now passes stopEx as the trailing 
throwable arg, so the full stop-proxy failure stack trace is logged.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to