lokeshj1703 commented on a change in pull request #2230:
URL: https://github.com/apache/ozone/pull/2230#discussion_r635049406
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -205,11 +202,10 @@ private boolean initializeIteration() {
numDatanodesBalanced += 1;
}
}
-
- // calculate total number of nodes that have been balanced
+ // calculate total number of nodes that have been balanced so far
numDatanodesBalanced =
- numDatanodesBalanced + metrics.getNumDatanodesBalanced().get();
- metrics.setNumDatanodesBalanced(new LongMetric(numDatanodesBalanced));
+ metrics.addToNumDatanodesBalanced(numDatanodesBalanced);
Review comment:
NIT: Change fn name to `incroNumDatanodesBalanced`
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -40,96 +44,341 @@
private ContainerManagerV2 containerManager;
private ReplicationManager replicationManager;
private OzoneConfiguration ozoneConfiguration;
+ private final SCMContext scmContext;
private double threshold;
private int maxDatanodesToBalance;
private long maxSizeToMove;
- private boolean balancerRunning;
- private List<DatanodeUsageInfo> sourceNodes;
- private List<DatanodeUsageInfo> targetNodes;
+ private List<DatanodeUsageInfo> unBalancedNodes;
+ private List<DatanodeUsageInfo> overUtilizedNodes;
+ private List<DatanodeUsageInfo> underUtilizedNodes;
+ private List<DatanodeUsageInfo> withinThresholdUtilizedNodes;
private ContainerBalancerConfiguration config;
+ private ContainerBalancerMetrics metrics;
+ private long clusterCapacity;
+ private long clusterUsed;
+ private long clusterRemaining;
+ private double clusterAvgUtilisation;
+ private final AtomicBoolean balancerRunning = new AtomicBoolean(false);
+ /**
+ * Constructs ContainerBalancer with the specified arguments. Initializes
+ * new ContainerBalancerConfiguration and ContainerBalancerMetrics.
+ * Container Balancer does not start on construction.
+ *
+ * @param nodeManager NodeManager
+ * @param containerManager ContainerManager
+ * @param replicationManager ReplicationManager
+ * @param ozoneConfiguration OzoneConfiguration
+ */
public ContainerBalancer(
NodeManager nodeManager,
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
- OzoneConfiguration ozoneConfiguration) {
+ OzoneConfiguration ozoneConfiguration,
+ final SCMContext scmContext) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.balancerRunning = false;
this.config = new ContainerBalancerConfiguration();
+ this.metrics = new ContainerBalancerMetrics();
+ this.scmContext = scmContext;
}
/**
- * Start ContainerBalancer. Current implementation is incomplete.
+ * Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public void start(ContainerBalancerConfiguration balancerConfiguration) {
- this.balancerRunning = true;
-
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
+ LOG.error("Container Balancer is already running.");
+ return false;
+ }
+ if (scmContext.isInSafeMode()) {
+ LOG.error("Container Balancer cannot operate while SCM is in Safe
Mode.");
+ return false;
+ }
Review comment:
This should be moved in initialise function. So the balancer skips
balancing in case SCM is in safe mode.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -23,80 +23,95 @@
import org.apache.hadoop.hdds.conf.ConfigGroup;
import org.apache.hadoop.hdds.conf.ConfigTag;
import org.apache.hadoop.hdds.conf.ConfigType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* This class contains configuration values for the ContainerBalancer.
*/
@ConfigGroup(prefix = "hdds.container.balancer.")
public final class ContainerBalancerConfiguration {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerBalancerConfiguration.class);
+
@Config(key = "utilization.threshold", type = ConfigType.AUTO, defaultValue =
"0.1", tags = {ConfigTag.BALANCER},
description = "Threshold is a fraction in the range of 0 to 1. A " +
"cluster is considered balanced if for each datanode, the " +
"utilization of the datanode (used space to capacity ratio) differs"
+
" from the utilization of the cluster (used space to capacity ratio"
+
" of the entire cluster) no more than the threshold value.")
- private double threshold = 0.1;
+ private String threshold = "0.1";
@Config(key = "datanodes.balanced.max", type = ConfigType.INT,
defaultValue = "5", tags = {ConfigTag.BALANCER}, description = "The " +
"maximum number of datanodes that should be balanced. Container " +
"Balancer will not balance more number of datanodes than this limit.")
private int maxDatanodesToBalance = 5;
- @Config(key = "size.moved.max", type = ConfigType.LONG,
- defaultValue = "10737418240L", tags = {ConfigTag.BALANCER},
- description = "The maximum size of data in Bytes that will be moved " +
- "by the Container Balancer.")
- private long maxSizeToMove = 10737418240L;
+ @Config(key = "size.moved.max", type = ConfigType.SIZE,
+ defaultValue = "10", tags = {ConfigTag.BALANCER},
Review comment:
Please see how ConfigType.SIZE is used in other places. I think the
default value should be `10GB`. It is more flexible config type and it will not
be limited to GB alone. The description and javadoc for corresponding fns can
be updated to remove specificity to GB.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
