Copilot commented on code in PR #10487:
URL: https://github.com/apache/ozone/pull/10487#discussion_r3418639114
##########
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:
When stopping the stale proxy fails, the log drops the stack trace by
logging only stopEx.getMessage(). Include the exception itself so failures in
RPC.stopProxy are diagnosable.
##########
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:
The warning when NetUtils.createSocketAddr(hostAndPort) fails logs only
ex.getMessage(), which drops the stack trace and makes DNS/parse failures hard
to debug in production. Log the exception as a throwable parameter so the full
cause is preserved.
--
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]
