siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r685739597



##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
       ContainerManagerV2 containerManager,
       ReplicationManager replicationManager,
       OzoneConfiguration ozoneConfiguration,
-      final SCMContext scmContext) {
+      final SCMContext scmContext,
+      PlacementPolicy placementPolicy) {
     this.nodeManager = nodeManager;
     this.containerManager = containerManager;
     this.replicationManager = replicationManager;
     this.ozoneConfiguration = ozoneConfiguration;
-    this.config = new ContainerBalancerConfiguration();
+    this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
     this.metrics = new ContainerBalancerMetrics();
     this.scmContext = scmContext;
+    this.placementPolicy = placementPolicy;
 
-    this.clusterCapacity = 0L;
-    this.clusterUsed = 0L;
-    this.clusterRemaining = 0L;
-
-    this.overUtilizedNodes = new ArrayList<>();
-    this.underUtilizedNodes = new ArrayList<>();
-    this.unBalancedNodes = new ArrayList<>();
-    this.withinThresholdUtilizedNodes = new ArrayList<>();
     this.lock = new ReentrantLock();
+    findTargetStrategy =
+        new FindTargetGreedy(containerManager, placementPolicy);
   }
+
   /**
    * Starts ContainerBalancer. Current implementation is incomplete.
    *
    * @param balancerConfiguration Configuration values.
    */
-  public boolean start(
-      ContainerBalancerConfiguration balancerConfiguration) {
-    lock.lock();
-    try {
-      if (!balancerRunning.compareAndSet(false, true)) {
-        LOG.info("Container Balancer is already running.");
-        return false;
-      }
-      
-      this.config = balancerConfiguration;
-      this.idleIteration = config.getIdleIteration();
-      this.threshold = config.getThreshold();
-      this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
-      this.maxSizeToMoveInGB = config.getMaxSizeToMove();
-      this.unBalancedNodes = new ArrayList<>();
-      LOG.info("Starting Container Balancer...{}", this);
-      //we should start a new balancer thread async
-      //and response to cli as soon as possible
+  public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+    if (!balancerRunning.compareAndSet(false, true)) {
+      LOG.error("Container Balancer is already running.");
+      return false;
+    }
 
+    ozoneConfiguration = new OzoneConfiguration();
+    this.config = balancerConfiguration;
+    LOG.info("Starting Container Balancer...{}", this);
 
-      //TODO: this is a temporary implementation
-      //modify this later
-      currentBalancingThread = new Thread(() -> balance());
-      currentBalancingThread.start();
-      ////////////////////////
-    } finally {
-      lock.unlock();
-    }
+    //we should start a new balancer thread async
+    //and response to cli as soon as possible
 
 
+    //TODO: this is a temporary implementation
+    //modify this later
+    currentBalancingThread = new Thread(this::balance);
+    currentBalancingThread.start();
+    ////////////////////////
     return true;
   }
 
   /**
    * Balances the cluster.
    */
   private void balance() {
+    this.idleIteration = config.getIdleIteration();
     for (int i = 0; i < idleIteration; i++) {
+      countDatanodesInvolvedPerIteration = 0;
+      sizeMovedPerIteration = 0;
+      this.threshold = config.getThreshold();
+      this.maxDatanodesToInvolvePerIteration =
+          config.getMaxDatanodesToInvolvePerIteration();
+      this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
+
+      // stop balancing if iteration is not initialized
       if (!initializeIteration()) {
-        //balancer should be stopped immediately
-        break;
+        stop();
+        return;
+      }
+      doIteration();
+      if (!isBalancerRunning()) {
+        return;
+      }
+
+      // wait for configured time before starting next iteration, unless
+      // this was the final iteration
+      if (i != idleIteration - 1) {
+        synchronized (this) {

Review comment:
       The `wait` method requires that the current thread owns this object's 
monitor




-- 
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