This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new b64c7fc0d76 HDDS-16387. ContainerBalancer status --verbose throws
IllegalStateException after a rejected start (#11275)
b64c7fc0d76 is described below
commit b64c7fc0d76ecc9e4583d6778d10d9729029a7a8
Author: Navink <[email protected]>
AuthorDate: Fri Sep 25 15:52:54 2026 +0530
HDDS-16387. ContainerBalancer status --verbose throws IllegalStateException
after a rejected start (#11275)
---
.../scm/container/balancer/ContainerBalancer.java | 8 +++-
.../container/balancer/TestContainerBalancer.java | 50 ++++++++++++++++++++++
2 files changed, 56 insertions(+), 2 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 14aa7efdc38..7515d93a747 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
@@ -250,7 +250,6 @@ private boolean canBalancerStop() {
@Override
public void start() throws IllegalContainerBalancerStateException,
InvalidContainerBalancerConfigurationException {
- startedAt = OffsetDateTime.now();
lock.lock();
try {
// should be leader-ready, out of safe mode, and not running already
@@ -277,6 +276,9 @@ public void start() throws
IllegalContainerBalancerStateException,
ozoneConfiguration);
validateConfiguration(configuration);
this.config = configuration;
+ // Set startedAt only after validation has passed, so a rejected start
+ // does not overwrite the recorded start time of the previous run.
+ startedAt = OffsetDateTime.now();
startBalancingThread(proto.getNextIterationIndex(), true);
} finally {
lock.unlock();
@@ -298,7 +300,6 @@ public void start() throws
IllegalContainerBalancerStateException,
public void startBalancer(ContainerBalancerConfiguration configuration)
throws IllegalContainerBalancerStateException,
InvalidContainerBalancerConfigurationException, IOException {
- startedAt = OffsetDateTime.now();
lock.lock();
try {
// validates state, config, and then saves config
@@ -307,6 +308,9 @@ public void startBalancer(ContainerBalancerConfiguration
configuration)
saveConfiguration(configuration, true, 0);
this.config = configuration;
+ // Set startedAt only after validation has passed, so a rejected start
+ // does not overwrite the recorded start time of the previous run.
+ startedAt = OffsetDateTime.now();
//start balancing task
startBalancingThread(0, false);
} finally {
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 8931c9d6719..440cf590f02 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
@@ -38,6 +38,7 @@
import com.google.protobuf.ByteString;
import java.io.IOException;
+import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -459,6 +460,55 @@ public void testRejectInvalidStartupConfiguration() throws
Exception {
assertSame(ContainerBalancerTask.Status.STOPPED,
containerBalancer.getBalancerStatus());
}
+ /**
+ * Verifies that a start request rejected during validation does not update
startedAt of last run.
+ * Prevents startedAt from advancing past stoppedAt, which previously caused
a negative duration and CLI status crash.
+ */
+ @Test
+ public void testRejectedStartDoesNotModifyStartedAt() throws Exception {
+ // The test first starts and immediately stops the balancer normally. This
creates a realistic "previous state"
+ // in system memory and later trigger a bad request and verify that
startedAt remains identical to startedAtBefore,
+ // rather than being wrongly overwritten with "now", which would make
startedAt come after stoppedAt which resulted
+ // in a negative balancing duration.
+ startBalancer(balancerConfiguration);
+ assertSame(ContainerBalancerTask.Status.RUNNING,
containerBalancer.getBalancerStatus());
+ stopBalancer();
+ assertSame(ContainerBalancerTask.Status.STOPPED,
containerBalancer.getBalancerStatus());
+
+ ContainerBalancerStatusInfo before =
containerBalancer.getBalancerStatusInfo();
+ assertNotNull(before);
+ OffsetDateTime startedAtBefore = before.getStartedAt();
+ OffsetDateTime stoppedAt = before.getStoppedAt();
+ assertNotNull(startedAtBefore);
+ assertNotNull(stoppedAt);
+
+ // Pause so that system time advances before the second start attempt so a
timestamp overwrite becomes measurable.
+ Thread.sleep(20);
+
+ // A start that is rejected during validation. Here we used an invalid
config, this is just one of many rejection
+ // paths (safe mode, non-leader SCM, already running, etc.).
+
+
balancerConfiguration.setMoveReplicationTimeout(TimeUnit.MINUTES.toMillis(60));
+ balancerConfiguration.setMoveTimeout(TimeUnit.MINUTES.toMillis(59));
+ assertThrows(InvalidContainerBalancerConfigurationException.class,
+ () -> containerBalancer.startBalancer(balancerConfiguration));
+
+ // The rejected start must leave the balancer STOPPED.
+ assertSame(ContainerBalancerTask.Status.STOPPED,
containerBalancer.getBalancerStatus());
+
+ ContainerBalancerStatusInfo after =
containerBalancer.getBalancerStatusInfo();
+ assertNotNull(after);
+
+ // startedAt must still equal the previous run's start time, a rejected
start must not reset it.
+ assertEquals(startedAtBefore, after.getStartedAt(),
+ "Rejected start reset startedAt to 'now' (set before validation),
replacing the previous run's start time");
+
+ // startedAt must never be after stoppedAt. Violating this is
+ // what produced the negative duration and the status --verbose crash.
+ assertFalse(after.getStartedAt().isAfter(stoppedAt),
+ "startedAt ended up after stoppedAt -> negative balancing duration
that crashes 'status --verbose'");
+ }
+
private static List<DatanodeInfo> createEligibleDatanodes(int count) {
List<DatanodeInfo> datanodes = new ArrayList<>(count);
for (int i = 0; i < count; i++) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
