kerneltime commented on code in PR #10487:
URL: https://github.com/apache/ozone/pull/10487#discussion_r3425194586
##########
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();
+ }
Review Comment:
Done in a0fc4ed -- the synchronized block now does only the map lookup;
SCMProxyInfo is immutable, so hostAndPort, cachedAddress and serviceId are read
lock-free afterwards.
##########
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);
+ 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();
Review Comment:
Done in a0fc4ed -- added import java.net.InetAddress.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java:
##########
@@ -342,8 +450,24 @@ public RetryAction shouldRetry(Exception e, int retry,
printRetryMessage(e, failover, retryAction.delayMillis);
}
+ // Before advancing the failover index, give the cached SCM
+ // address a chance to be re-resolved -- the same nodeId may
+ // have moved to a new IP under a stable hostname (Kubernetes
+ // pod restart). Limited to connection-class exceptions to
+ // avoid extra DNS load on application-level errors.
+ boolean refreshed = false;
+ if (resolveOnFailureEnabled
+ && ConnectionFailureUtils.isConnectionFailure(e)) {
+ refreshed = refreshProxyAddressIfChanged(getCurrentProxySCMNodeId());
+ }
+
if (SCMHAUtils.checkRetriableWithNoFailoverException(e)) {
setUpdatedLeaderNodeID();
+ } else if (refreshed) {
Review Comment:
Done in a0fc4ed -- swapped so the cheap already-computed refreshed boolean
is tested first; both branches pin via setUpdatedLeaderNodeID, so behavior is
unchanged.
--
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]
