lokeshj1703 commented on a change in pull request #3153:
URL: https://github.com/apache/ozone/pull/3153#discussion_r833017057



##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -792,21 +757,154 @@ private void resetState() {
   }
 
   /**
-   * Stops ContainerBalancer.
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops ContainerBalancer if it's running and the current SCM becomes a
+   * follower or enters safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    if (!checkLeaderAndSafeMode()) {
+      if (isBalancerRunning()) {
+        stopBalancingThread();
+      }
+    }
+  }
+
+  /**
+   * Checks if ContainerBalancer should run.
+   * @return false
    */
+  @Override
+  public boolean shouldRun() {
+    return false;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return ContainerBalancer.class.getSimpleName();
+  }
+
+  /**
+   * Starts ContainerBalancer as an SCMService.
+   */
+  @Override
+  public void start() {
+    if (shouldRun()) {
+      startBalancingThread();
+    }
+  }
+
+  /**
+   * Starts Container Balancer after checking its state and validating
+   * configuration.
+   *
+   * @throws IllegalContainerBalancerStateException if ContainerBalancer is
+   * not in a start-appropriate state
+   * @throws InvalidContainerBalancerConfigurationException if
+   * {@link ContainerBalancerConfiguration} config file is incorrectly
+   * configured
+   */
+  public void startBalancer() throws IllegalContainerBalancerStateException,
+      InvalidContainerBalancerConfigurationException {
+    lock.lock();
+    try {
+      if (!canRun()) {
+        throw new IllegalContainerBalancerStateException("Cannot start " +
+            "ContainerBalancer in the current state.");
+      }
+      if (!validateConfiguration(this.config)) {
+        throw new InvalidContainerBalancerConfigurationException("Cannot " +
+            "start ContainerBalancer because the configuration is invalid.");
+      }
+      startBalancingThread();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  /**
+   * Starts a new balancing thread asynchronously.
+   */
+  private void startBalancingThread() {
+    lock.lock();
+    try {
+      balancerRunning = true;
+      currentBalancingThread = new Thread(this::balance);
+      currentBalancingThread.setName("ContainerBalancer");
+      currentBalancingThread.setDaemon(true);
+      currentBalancingThread.start();
+    } finally {
+      lock.unlock();
+    }
+    LOG.info("Starting Container Balancer... {}", this);
+  }
+
+  private boolean canRun() {
+    if (!checkLeaderAndSafeMode()) {
+      return false;
+    }
+    lock.lock();
+    try {
+      if (isBalancerRunning() || currentBalancingThread != null) {
+        LOG.warn("Cannot run ContainerBalancer because it's already running");
+        return false;
+      }
+    } finally {
+      lock.unlock();
+    }
+    return true;
+  }
+
+  /**
+   * Used to check if SCM is leader ready and not in safe mode.
+   * @return true if SCM is leader ready and not in safe mode, false otherwise
+   */
+  private boolean checkLeaderAndSafeMode() {
+    if (!scmContext.isLeaderReady()) {
+      LOG.warn("SCM is not leader ready");
+      return false;
+    }
+    if (scmContext.isInSafeMode()) {
+      LOG.warn("SCM is in safe mode");
+      return false;
+    }
+    return true;
+  }
+
+  /**
+   * Stops the SCM service.
+   */
+  @Override
   public void stop() {
+    stopBalancer();
+  }
+
+  /**
+   * Stops ContainerBalancer gracefully.
+   */
+  public void stopBalancer() {
     lock.lock();
     try {
-      // we should stop the balancer thread gracefully
-      if (!balancerRunning) {
+      if (!isBalancerRunning()) {
         LOG.info("Container Balancer is not running.");
         return;
       }
-      balancerRunning = false;
+      stopBalancingThread();
     } finally {
       lock.unlock();
     }
+  }
 
+  private void stopBalancingThread() {
+    lock.lock();
+    try {
+      balancerRunning = false;

Review comment:
       currentBalancingThread should be set within the lock.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to