From ca2438e34f6749ab2dd3298db2850570d56d5427 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Fri, 20 Sep 2024 18:12:58 +0800 Subject: [PATCH 1/4] HDDS-11388. Fix unnecessary call to the DB for ContainerBalancer#getBalancerStatusInfo --- .../container/balancer/ContainerBalancer.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java index 3dddd67bd8ad..9809ee0df4d4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java @@ -183,17 +183,19 @@ public ContainerBalancerTask.Status getBalancerStatus() { * @return balancer status info if balancer started */ public ContainerBalancerStatusInfo getBalancerStatusInfo() throws IOException { - if (isBalancerRunning()) { - ContainerBalancerConfigurationProto configProto = readConfiguration(ContainerBalancerConfigurationProto.class); - return new ContainerBalancerStatusInfo( - this.startedAt, - configProto, - task.getCurrentIterationsStatistic() - ); - } else { + lock.lock(); + try { + if (isBalancerRunning()) { + return new ContainerBalancerStatusInfo( + this.startedAt, + config.toProtobufBuilder().build(), + task.getCurrentIterationsStatistic() + ); + } return null; + } finally { + lock.unlock(); } - } /** * Checks if ContainerBalancer is in valid state to call stop. From 510a189cb507ab647c7f65638da9f16ce6ff51c2 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Fri, 27 Sep 2024 23:09:06 +0800 Subject: [PATCH 2/4] Try removing the balancer running assertion when waiting for finish --- hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot b/hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot index 4299afe5f2d1..8ba482da43bf 100644 --- a/hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot +++ b/hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot @@ -66,7 +66,6 @@ Run Container Balancer Wait Finish Of Balancing ${result} = Execute ozone admin containerbalancer status - Should Contain ${result} ContainerBalancer is Running. Wait Until Keyword Succeeds 3min 10sec ContainerBalancer is Not Running Sleep 60000ms From 4146a39aba05d362a85ef5a08fc8c04dd51b48e8 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 1 Oct 2024 15:31:37 +0800 Subject: [PATCH 3/4] Set shouldRun --- .../hadoop/hdds/scm/container/balancer/ContainerBalancer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java index 9809ee0df4d4..2f6b8a7f814f 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java @@ -188,7 +188,7 @@ public ContainerBalancerStatusInfo getBalancerStatusInfo() throws IOException { if (isBalancerRunning()) { return new ContainerBalancerStatusInfo( this.startedAt, - config.toProtobufBuilder().build(), + config.toProtobufBuilder().setShouldRun(true).build(), task.getCurrentIterationsStatistic() ); } From 806dc9b97f625bcfdabae6e52bb1297d3a0bdf8c Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 1 Oct 2024 15:48:51 +0800 Subject: [PATCH 4/4] Add unit test for getBalancerStatusInfo --- .../balancer/TestContainerBalancer.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java index 662322b42f55..c77928874715 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java @@ -44,6 +44,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_NODE_REPORT_INTERVAL; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import static org.junit.jupiter.api.Assertions.assertSame; @@ -257,6 +258,22 @@ public void testDelayedStartOnSCMStatusChange() stopBalancer(); } + @Test + public void testGetBalancerStatusInfo() throws Exception { + startBalancer(balancerConfiguration); + assertSame(ContainerBalancerTask.Status.RUNNING, containerBalancer.getBalancerStatus()); + + // Assert the configuration fields that were explicitly set + ContainerBalancerStatusInfo status = containerBalancer.getBalancerStatusInfo(); + assertEquals(balancerConfiguration.getThreshold(), + Double.parseDouble(status.getConfiguration().getUtilizationThreshold())); + assertEquals(balancerConfiguration.getIterations(), status.getConfiguration().getIterations()); + assertEquals(balancerConfiguration.getTriggerDuEnable(), status.getConfiguration().getTriggerDuBeforeMoveEnable()); + + stopBalancer(); + assertSame(ContainerBalancerTask.Status.STOPPED, containerBalancer.getBalancerStatus()); + } + private void startBalancer(ContainerBalancerConfiguration config) throws IllegalContainerBalancerStateException, IOException, InvalidContainerBalancerConfigurationException, TimeoutException {