This is an automated email from the ASF dual-hosted git repository. kerneltime pushed a commit to branch HDDS-15599-followup-cleanup in repository https://gitbox.apache.org/repos/asf/ozone.git
commit 762e23003bdddd7a5595024a2411ceb88aeddb77 Author: Ritesh H Shukla <[email protected]> AuthorDate: Wed Jun 17 22:15:27 2026 -0700 HDDS-15599. Follow up clean up from HDDS-15531 PR 10486 Addresses the post-merge review comments from @szetszwo on #10486. All three are minor, behavior-preserving changes he asked to be handled in a follow-up PR. - ConnectionFailureUtils: ConnectException and NoRouteToHostException both extend SocketException, so their explicit instanceof checks were redundant. Removed them (the SocketException branch still matches both) and added a comment. They remain listed in the class Javadoc as connection-failure shapes. - OzoneConfigKeys: moved ozone.client.failover.resolve-needed (key, default, and Javadoc) next to ozone.client.failover.max.attempts. - OMFailoverProxyProviderBase: replaced the synchronized null-guard in maybeRefreshCurrentOmAddress with Objects.requireNonNull for nodeId and info. getCurrentProxyOMNodeId() is already synchronized and omProxies is an unmodifiable map built once at construction, so neither read needs the provider monitor and both values are always present. --- .../hadoop/hdds/utils/ConnectionFailureUtils.java | 8 ++++--- .../org/apache/hadoop/ozone/OzoneConfigKeys.java | 25 +++++++++++----------- .../ozone/om/ha/OMFailoverProxyProviderBase.java | 20 +++++++---------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/ConnectionFailureUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/ConnectionFailureUtils.java index 5c68d4c9fad..62cd868c763 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/ConnectionFailureUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/ConnectionFailureUtils.java @@ -82,9 +82,11 @@ private ConnectionFailureUtils() { public static boolean isConnectionFailure(Throwable t) { Throwable cause = t; for (int depth = 0; cause != null && depth < MAX_CAUSE_DEPTH; depth++) { - if (cause instanceof ConnectException - || cause instanceof SocketTimeoutException - || cause instanceof NoRouteToHostException + // ConnectException and NoRouteToHostException both extend + // SocketException, so the SocketException check below already matches + // them. They remain listed in this class's Javadoc as connection- + // failure shapes for documentation. + if (cause instanceof SocketTimeoutException || cause instanceof UnknownHostException || cause instanceof EOFException || cause instanceof SocketException) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index df0865862cc..4a808c2ee64 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -491,6 +491,18 @@ public final class OzoneConfigKeys { "ozone.client.failover.max.attempts"; public static final int OZONE_CLIENT_FAILOVER_MAX_ATTEMPTS_DEFAULT = 500; + /** + * When true, RPC clients (DN heartbeat, OM client, SCM client) re-resolve + * cached hostnames on connection failure and rebuild the proxy if the + * resolved IP has changed. Set to true in environments where server pod + * IPs may change while DNS names remain stable, such as Kubernetes. + * Default false preserves pre-fix behavior. Mirrors the design intent of + * HADOOP-17068 / HDFS-14118. + */ + public static final String OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_KEY = + "ozone.client.failover.resolve-needed"; + public static final boolean OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_DEFAULT = + false; public static final String OZONE_CLIENT_WAIT_BETWEEN_RETRIES_MILLIS_KEY = "ozone.client.wait.between.retries.millis"; public static final long OZONE_CLIENT_WAIT_BETWEEN_RETRIES_MILLIS_DEFAULT = @@ -604,19 +616,6 @@ public final class OzoneConfigKeys { public static final boolean OZONE_JVM_NETWORK_ADDRESS_CACHE_ENABLED_DEFAULT = true; - /** - * When true, RPC clients (DN heartbeat, OM client, SCM client) re-resolve - * cached hostnames on connection failure and rebuild the proxy if the - * resolved IP has changed. Set to true in environments where server pod - * IPs may change while DNS names remain stable, such as Kubernetes. - * Default false preserves pre-fix behavior. Mirrors the design intent of - * HADOOP-17068 / HDFS-14118. - */ - public static final String OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_KEY = - "ozone.client.failover.resolve-needed"; - public static final boolean OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_DEFAULT = - false; - public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY = "ozone.client.required.om.version.min"; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java index 769f3f8d496..a34709a3a54 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java @@ -24,6 +24,7 @@ import java.net.InetSocketAddress; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdds.HddsUtils; @@ -533,18 +534,13 @@ protected ConfigurationSource getConf() { */ @VisibleForTesting boolean maybeRefreshCurrentOmAddress() { - final String nodeId; - final OMProxyInfo<T> info; - synchronized (this) { - nodeId = getCurrentProxyOMNodeId(); - if (nodeId == null) { - return false; - } - info = omProxies.get(nodeId); - if (info == null) { - return false; - } - } + // getCurrentProxyOMNodeId() is synchronized and omProxies is an + // unmodifiable map populated once at construction, so neither read + // needs the provider monitor; both values are always present. + final String nodeId = Objects.requireNonNull(getCurrentProxyOMNodeId(), + "Current proxy node id is null"); + final OMProxyInfo<T> info = Objects.requireNonNull(omProxies.get(nodeId), + "Current proxy info is null"); // refreshAddressIfChanged handles its own locking and performs the // DNS lookup outside its entry monitor. boolean swapped = info.refreshAddressIfChanged(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
