siddhantsangwan commented on a change in pull request #2893:
URL: https://github.com/apache/ozone/pull/2893#discussion_r772108469
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -752,6 +755,33 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}
+ private void validateConfiguration(ContainerBalancerConfiguration conf) {
+ // maxSizeEnteringTarget and maxSizeLeavingSource should by default be
+ // greater than container size
+ long size = (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES) +
+ OzoneConsts.GB;
+
+ if (conf.getMaxSizeEnteringTarget() < size) {
Review comment:
This should be the check if another GB isn't added to `size` (as
commented above)
```suggestion
if (conf.getMaxSizeEnteringTarget() <= size) {
```
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -752,6 +755,33 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}
+ private void validateConfiguration(ContainerBalancerConfiguration conf) {
+ // maxSizeEnteringTarget and maxSizeLeavingSource should by default be
+ // greater than container size
+ long size = (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES) +
+ OzoneConsts.GB;
Review comment:
We don't need to add another GB here. The configs just need to be
greater than `size`.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -752,6 +755,33 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}
+ private void validateConfiguration(ContainerBalancerConfiguration conf) {
+ // maxSizeEnteringTarget and maxSizeLeavingSource should by default be
+ // greater than container size
+ long size = (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES) +
+ OzoneConsts.GB;
+
+ if (conf.getMaxSizeEnteringTarget() < size) {
+ LOG.info("MaxSizeEnteringTarget should be larger than " +
+ "ozone.scm.container.size");
+ }
+ if (conf.getMaxSizeLeavingSource() < size) {
Review comment:
Same as above
```suggestion
if (conf.getMaxSizeLeavingSource() <= size) {
```
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -752,6 +755,33 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}
+ private void validateConfiguration(ContainerBalancerConfiguration conf) {
+ // maxSizeEnteringTarget and maxSizeLeavingSource should by default be
+ // greater than container size
+ long size = (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES) +
+ OzoneConsts.GB;
+
+ if (conf.getMaxSizeEnteringTarget() < size) {
+ LOG.info("MaxSizeEnteringTarget should be larger than " +
+ "ozone.scm.container.size");
+ }
+ if (conf.getMaxSizeLeavingSource() < size) {
+ LOG.info("MaxSizeLeavingSource should be larger than " +
+ "ozone.scm.container.size");
+ }
+
+ // balancing interval should be greater than DUFactory refresh period
+ DUFactory.Conf duConf = ozoneConfiguration.getObject(DUFactory.Conf.class);
+ long balancingInterval = duConf.getRefreshPeriod().toMillis() +
+ Duration.ofMinutes(10).toMillis();
Review comment:
Again, don't need to add extra 10 minutes since the config only needs to
be greater than DU refresh period.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -126,31 +120,6 @@
private DUFactory.Conf duConf;
Review comment:
Since this isn't getting initialised now, we can remove this and its
usages in the `setBalancingInterval` method.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -752,6 +755,33 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}
+ private void validateConfiguration(ContainerBalancerConfiguration conf) {
+ // maxSizeEnteringTarget and maxSizeLeavingSource should by default be
+ // greater than container size
+ long size = (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES) +
+ OzoneConsts.GB;
+
+ if (conf.getMaxSizeEnteringTarget() < size) {
+ LOG.info("MaxSizeEnteringTarget should be larger than " +
+ "ozone.scm.container.size");
+ }
+ if (conf.getMaxSizeLeavingSource() < size) {
+ LOG.info("MaxSizeLeavingSource should be larger than " +
+ "ozone.scm.container.size");
+ }
+
+ // balancing interval should be greater than DUFactory refresh period
+ DUFactory.Conf duConf = ozoneConfiguration.getObject(DUFactory.Conf.class);
+ long balancingInterval = duConf.getRefreshPeriod().toMillis() +
+ Duration.ofMinutes(10).toMillis();
+ if (conf.getBalancingInterval().toMillis() < balancingInterval) {
+ LOG.info("balancing.iteration.interval should be at lease 10 minutes " +
+ "larger than hdds.datanode.du.refresh.period.");
Review comment:
```suggestion
LOG.info("balancing.iteration.interval should be " +
"larger than hdds.datanode.du.refresh.period.");
```
--
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]
