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..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,13 +392,24 @@ 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() { + // 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..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 @@ -145,6 +145,26 @@ 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); + // The next retry on the same OM should start from the base waitTime again. + failoverToSameNode(1); + } + /** * Ensure listener nodes are excluded from provider's proxy list. */ 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)); } }