From 0b3589a2b43f230df4f9c8f248c6b60699e1eadf Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:56:20 -0700 Subject: [PATCH 1/3] HDDS-17302. OM client retry wait time grows without bound with a single OM Co-Authored-By: Claude Opus 5.5 --- .../om/ha/OMFailoverProxyProviderBase.java | 25 +++++++++---------- .../om/ha/TestOMFailoverProxyProvider.java | 18 +++++++++++++ 2 files changed, 30 insertions(+), 13 deletions(-) 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 7486c76e6fb7..a1da7e8f5d6f 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 @@ -399,6 +399,17 @@ private synchronized boolean updateLeaderOMNodeId(String newLeaderOMNodeId) { * @return delay in milliseconds */ public synchronized long getWaitTime() { + // OMs are being contacted in Round Robin way. If all the OMs have been + // contacted once in this attempt, clear the list. The OMs will be retried + // in a Round Robin fashion again after a fixed delay. This is checked first + // because with a single OM, Round Robin selects the same OM again, which is + // not a retry on the same OM based on response from OM. + if (attemptedOMs.containsAll(omProxies.getNodeIds())) { + attemptedOMs.clear(); + numAttemptsOnSameOM = 0; + return waitBetweenRetries; + } + if (omProxies.getNodeId(nextProxyIndex).equals(lastAttemptedOM)) { // Clear attemptedOMs list as the same OM has been selected again. attemptedOMs.clear(); @@ -409,19 +420,7 @@ public synchronized long getWaitTime() { } // Reset numAttemptsOnSameOM as we failed over to a different OM. numAttemptsOnSameOM = 0; - - // OMs are being contacted in Round Robin way. Check if all the OMs have - // been contacted in this attempt. - for (String omNodeID : omProxies.getNodeIds()) { - if (!attemptedOMs.contains(omNodeID)) { - return 0; - } - } - // This implies all the OMs have been contacted once. Return true and - // clear the list. The OMs will be retried in a Round Robin fashion again - // after a delay. - attemptedOMs.clear(); - return waitBetweenRetries; + return 0; } public List> getOMProxies() { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java index 08085743270b..8124561d3bd9 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java @@ -145,6 +145,24 @@ public void testWaitTimeResetWhenAllNodeFailoverAndSameNode() { failoverToNextNode(1, waitBetweenRetries); } + /** + * Tests waitTime with a single OM: Round Robin failover (e.g. on connection + * failure) selects the same OM again, and should not increase the wait time. + */ + @Test + public void testWaitTimeWithSingleNode() throws IOException { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_OM_ADDRESS_KEY, DUMMY_NODE_ADDR); + provider = new HadoopRpcOMFailoverProxyProvider<>(conf, UserGroupInformation.getCurrentUser(), null, + OzoneManagerProtocolPB.class); + + failoverToNextNode(3, waitBetweenRetries); + // Retry on the same OM based on response from OM, waitTime should increase. + failoverToSameNode(2); + // Round Robin failover again should reset waitTime. + failoverToNextNode(1, waitBetweenRetries); + } + /** * Ensure listener nodes are excluded from provider's proxy list. */ From a163770385179facb522b8b41cf879cb959ca517 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Thu, 22 Oct 2026 03:51:31 -0700 Subject: [PATCH 2/3] HDDS-17302. Test that the same-OM wait restarts after a full round Co-Authored-By: Claude Opus 5.5 --- .../hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java | 6 +++--- .../hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) 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 a1da7e8f5d6f..5646ad119142 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 @@ -392,10 +392,10 @@ private synchronized boolean updateLeaderOMNodeId(String newLeaderOMNodeId) { /** * Get the wait time based on - * 1. Is the same OM being retried based on response from OM. - * 2. Is this a new OM that is being used. - * 3. Were all the OMs visited once and retries need to be resumed after a + * 1. Were all the OMs visited once and retries need to be resumed after a * delay. + * 2. Is the same OM being retried based on response from OM. + * 3. Is this a new OM that is being used. * @return delay in milliseconds */ public synchronized long getWaitTime() { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java index 8124561d3bd9..6f74ab936cd2 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java @@ -161,6 +161,8 @@ public void testWaitTimeWithSingleNode() throws IOException { failoverToSameNode(2); // Round Robin failover again should reset waitTime. failoverToNextNode(1, waitBetweenRetries); + // The next retry on the same OM should start from the base waitTime again. + failoverToSameNode(1); } /** From 30a9add62fa6d28943dbbcbea03502ab0b97e96d Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:20:42 -0700 Subject: [PATCH 3/3] HDDS-17302. Wait for OM leader after restart in TestLeaseRecovery#testOMConnectionFailure Co-Authored-By: Claude Opus 5.5 --- .../test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java index 632470ba51dd..63a86bd44ac4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java @@ -483,6 +483,8 @@ public void testOMConnectionFailure() throws Exception { om.restart(); cluster.waitForClusterToBeReady(); + // the client retries only a few times, so wait until OM can serve requests + GenericTestUtils.waitFor(om::isLeaderReady, 100, 10000); assertTrue(fs.recoverLease(file)); } }