Merge branch 'master' into HDDS-13223

Author: YutaLin

…/volume/StorageVolumeScannerMetrics.java …lume/BackgroundVolumeScannerMetrics.javahadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolumeScannerMetrics.java renamed to hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/BackgroundVolumeScannerMetrics.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolumeScannerMetrics.java renamed to hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/BackgroundVolumeScannerMetrics.java

@@ -26,12 +26,12 @@
 import org.apache.hadoop.metrics2.lib.MutableGaugeLong;
 
 /**
- * This class captures the Storage Volume Scanner Metrics.
+ * This class captures the Background Storage Volume Scanner Metrics.
  **/
 @InterfaceAudience.Private
-@Metrics(about = "Storage Volume Scanner Metrics", context = "dfs")
-public class StorageVolumeScannerMetrics {
-  public static final String SOURCE_NAME = StorageVolumeScannerMetrics.class.getSimpleName();
+@Metrics(about = "Background Volume Scanner Metrics", context = "dfs")
+public class BackgroundVolumeScannerMetrics {
+  public static final String SOURCE_NAME = BackgroundVolumeScannerMetrics.class.getSimpleName();
 
   @Metric("number of volumes scanned in the last iteration")
   private MutableGaugeLong numVolumesScannedInLastIteration;
@@ -49,12 +49,12 @@ public class StorageVolumeScannerMetrics {
       "since the last iteration had not elapsed")
   private MutableCounterLong numIterationsSkipped;
 
-  public StorageVolumeScannerMetrics() {
+  public BackgroundVolumeScannerMetrics() {
   }
 
-  public static StorageVolumeScannerMetrics create() {
+  public static BackgroundVolumeScannerMetrics create() {
     MetricsSystem ms = DefaultMetricsSystem.instance();
-    return ms.register(SOURCE_NAME, "Storage Volume Scanner Metrics", new StorageVolumeScannerMetrics());
+    return ms.register(SOURCE_NAME, "Background Volume Scanner Metrics", new BackgroundVolumeScannerMetrics());
   }
 
   /**

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolumeChecker.java

@@ -65,7 +65,7 @@ public class StorageVolumeChecker {
 
   private AsyncChecker<Boolean, VolumeCheckResult> delegateChecker;
 
-  private final StorageVolumeScannerMetrics metrics;
+  private final BackgroundVolumeScannerMetrics metrics;
 
   /**
    * Max allowed time for a disk check in milliseconds. If the check
@@ -105,7 +105,7 @@ public class StorageVolumeChecker {
   public StorageVolumeChecker(ConfigurationSource conf, Timer timer,
       String threadNamePrefix) {
 
-    metrics = StorageVolumeScannerMetrics.create();
+    metrics = BackgroundVolumeScannerMetrics.create();
 
     this.timer = timer;
 
@@ -441,7 +441,7 @@ void setDelegateChecker(
   }
 
   @VisibleForTesting
-  public StorageVolumeScannerMetrics getMetrics() {
+  public BackgroundVolumeScannerMetrics getMetrics() {
     return metrics;
   }
 }

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestPeriodicVolumeChecker.java

@@ -79,7 +79,7 @@ public void testPeriodicVolumeChecker(TestInfo testInfo) throws Exception {
     FakeTimer timer = new FakeTimer();
 
     StorageVolumeChecker volumeChecker = new StorageVolumeChecker(conf, timer, "");
-    StorageVolumeScannerMetrics metrics = volumeChecker.getMetrics();
+    BackgroundVolumeScannerMetrics metrics = volumeChecker.getMetrics();
 
     try {
       volumeChecker.registerVolumeSet(new ImmutableVolumeSet(makeVolumes(2, HEALTHY)));

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

@@ -308,16 +308,13 @@ private void startBalancingThread(int nextIterationIndex,
   }
 
   /**
-   * Validates balancer's state based on the specified expectedRunning.
+   * Validates balancer's eligibility based on SCM state.
    * Confirms SCM is leader-ready and out of safe mode.
    *
-   * @param expectedRunning true if ContainerBalancer is expected to be
-   *                        running, else false
    * @throws IllegalContainerBalancerStateException if SCM is not
-   * leader-ready, is in safe mode, or state does not match the specified
-   * expected state
+   * leader-ready or is in safe mode
    */
-  private void validateState(boolean expectedRunning)
+  private void validateEligibility()
       throws IllegalContainerBalancerStateException {
     if (!scmContext.isLeaderReady()) {
       LOG.warn("SCM is not leader ready");
@@ -328,6 +325,19 @@ private void validateState(boolean expectedRunning)
       LOG.warn("SCM is in safe mode");
       throw new IllegalContainerBalancerStateException("SCM is in safe mode");
     }
+  }
+
+  /**
+   * Validates balancer's state based on the specified expectedRunning.
+   *
+   * @param expectedRunning true if ContainerBalancer is expected to be
+   *                        running, else false
+   * @throws IllegalContainerBalancerStateException if state does not
+   * match the specified expected state
+   */
+  private void validateState(boolean expectedRunning)
+      throws IllegalContainerBalancerStateException {
+    validateEligibility();
     if (!expectedRunning && !canBalancerStart()) {
       throw new IllegalContainerBalancerStateException(
           "Expect ContainerBalancer as not running state" +
@@ -387,18 +397,22 @@ private static void blockTillTaskStop(Thread balancingThread) {
    */
   public void stopBalancer()
       throws IOException, IllegalContainerBalancerStateException {
-    Thread balancingThread;
+    Thread balancingThread = null;
     lock.lock();
     try {
-      validateState(true);
+      validateEligibility();
       saveConfiguration(config, false, 0);
-      LOG.info("Trying to stop ContainerBalancer service.");
-      task.stop();
-      balancingThread = currentBalancingThread;
+      if (isBalancerRunning()) {
+        LOG.info("Trying to stop ContainerBalancer service.");
+        task.stop();
+        balancingThread = currentBalancingThread;
+      }
     } finally {
       lock.unlock();
     }
-    blockTillTaskStop(balancingThread);
+    if (balancingThread != null) {
+      blockTillTaskStop(balancingThread);
+    }
   }
 
   public void saveConfiguration(ContainerBalancerConfiguration configuration,

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java

@@ -20,6 +20,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.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertSame;
@@ -128,6 +129,8 @@ public void testShouldRun() throws Exception {
 
   @Test
   public void testStartBalancerStop() throws Exception {
+    //stop should not throw an exception as it is idempotent
+    assertDoesNotThrow(() -> containerBalancer.stopBalancer());
     startBalancer(balancerConfiguration);
     assertThrows(IllegalContainerBalancerStateException.class,
         () -> containerBalancer.startBalancer(balancerConfiguration),
@@ -142,9 +145,9 @@ public void testStartBalancerStop() throws Exception {
     stopBalancer();
     assertSame(ContainerBalancerTask.Status.STOPPED, containerBalancer.getBalancerStatus());
 
-    assertThrows(Exception.class,
-        () -> containerBalancer.stopBalancer(),
-        "Exception should be thrown when stop again");
+    // If the balancer is already stopped, the stop command should do nothing
+    // and return successfully as stopBalancer is idempotent
+    assertDoesNotThrow(() -> containerBalancer.stopBalancer());
   }
 
   @Test

hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java

@@ -26,8 +26,6 @@
 import io.opentelemetry.api.trace.StatusCode;
 import java.io.IOException;
 import java.io.InputStream;
-import java.time.Duration;
-import java.time.Instant;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
@@ -327,8 +325,7 @@ public void init() {
     LongSupplier supplier;
     if (duration != null) {
       maxValue = durationInSecond;
-      supplier = () -> Duration.between(
-          Instant.ofEpochMilli(startTime), Instant.now()).getSeconds();
+      supplier = () -> (Time.monotonicNow() - startTime) / 1000;
     } else {
       maxValue = testNo;
       supplier = () -> successCounter.get() + failureCounter.get();

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java

@@ -19,10 +19,12 @@
 
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_NODE_REPORT_INTERVAL;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.io.IOException;
 import java.util.Optional;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.PlacementPolicy;
@@ -178,4 +180,44 @@ public void testIfCBCLIOverridesConfigs() throws Exception {
     running = containerBalancerClient.getContainerBalancerStatus();
     assertFalse(running);
   }
+
+  /**
+   * Tests that stopBalancer is idempotent - once the balancer is in STOPPED state,
+   * invoking stop again should be a no-op and return successfully with exit code 0.
+   */
+  @Test
+  public void testStopBalancerIdempotent() throws IOException {
+    boolean running = containerBalancerClient.getContainerBalancerStatus();
+    assertFalse(running);
+    assertDoesNotThrow(() -> containerBalancerClient.stopContainerBalancer());
+
+    Optional<Double> threshold = Optional.of(0.1);
+    Optional<Integer> iterations = Optional.of(10000);
+    Optional<Integer> maxDatanodesPercentageToInvolvePerIteration =
+        Optional.of(100);
+    Optional<Long> maxSizeToMovePerIterationInGB = Optional.of(1L);
+    Optional<Long> maxSizeEnteringTargetInGB = Optional.of(6L);
+    Optional<Long> maxSizeLeavingSourceInGB = Optional.of(6L);
+    Optional<Integer> balancingInterval = Optional.of(70);
+    Optional<Integer> moveTimeout = Optional.of(65);
+    Optional<Integer> moveReplicationTimeout = Optional.of(50);
+    Optional<Boolean> networkTopologyEnable = Optional.of(false);
+    Optional<String> includeNodes = Optional.of("");
+    Optional<String> excludeNodes = Optional.of("");
+    containerBalancerClient.startContainerBalancer(threshold, iterations,
+        maxDatanodesPercentageToInvolvePerIteration,
+        maxSizeToMovePerIterationInGB, maxSizeEnteringTargetInGB,
+        maxSizeLeavingSourceInGB, balancingInterval, moveTimeout,
+        moveReplicationTimeout, networkTopologyEnable, includeNodes,
+        excludeNodes);
+    running = containerBalancerClient.getContainerBalancerStatus();
+    assertTrue(running);
+
+    containerBalancerClient.stopContainerBalancer();
+    running = containerBalancerClient.getContainerBalancerStatus();
+    assertFalse(running);
+
+    // Calling stop balancer again should not throw an exception
+    assertDoesNotThrow(() -> containerBalancerClient.stopContainerBalancer());
+  }
 }