ayushtkn commented on code in PR #5823:
URL: https://github.com/apache/hadoop/pull/5823#discussion_r1263492781
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -951,6 +960,45 @@ private String reconfDfsUsageParameters(String property,
String newVal)
}
}
+ private String reconfDiskBalancerParameters(String property, String newVal)
+ throws ReconfigurationException {
+ String result = null;
+ try {
+ LOG.info("Reconfiguring {} to {}", property, newVal);
+ if (property.equals(DFS_DISK_BALANCER_ENABLED)) {
+ if (newVal != null && !newVal.equalsIgnoreCase("true")
+ && !newVal.equalsIgnoreCase("false")) {
+ throw new IllegalArgumentException("Not a valid Boolean value for "
+ property +
+ " in reconfDiskBalancerParameters");
Review Comment:
this isn't required
```
" in reconfDiskBalancerParameters"
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -4201,7 +4249,7 @@ public List<DatanodeVolumeInfo> getVolumeReport() throws
IOException {
return volumeInfoList;
}
- private DiskBalancer getDiskBalancer() throws IOException {
+ public DiskBalancer getDiskBalancer() throws IOException {
Review Comment:
Add ```@VisibleForTesting```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -951,6 +960,45 @@ private String reconfDfsUsageParameters(String property,
String newVal)
}
}
+ private String reconfDiskBalancerParameters(String property, String newVal)
+ throws ReconfigurationException {
+ String result = null;
+ try {
+ LOG.info("Reconfiguring {} to {}", property, newVal);
+ if (property.equals(DFS_DISK_BALANCER_ENABLED)) {
+ if (newVal != null && !newVal.equalsIgnoreCase("true")
+ && !newVal.equalsIgnoreCase("false")) {
+ throw new IllegalArgumentException("Not a valid Boolean value for "
+ property +
+ " in reconfDiskBalancerParameters");
+ }
+ boolean enable = (newVal == null ? DFS_DISK_BALANCER_ENABLED_DEFAULT :
+ Boolean.parseBoolean(newVal));
Review Comment:
this is a redundant check, earlier above we already did checks to figure out
whether it is true/false, should have done check once and saved the value above
only
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancer.java:
##########
@@ -341,6 +341,59 @@ private void checkDiskBalancerEnabled()
}
}
+ /**
+ * Sets Disk balancer is to enable or not to enable.
+ *
+ * @param diskBalancerEnabled
+ * true, enable diskBalancer, otherwise false to disable it.
+ */
+ public void setDiskBalancerEnabled(boolean diskBalancerEnabled) {
+ isDiskBalancerEnabled = diskBalancerEnabled;
+ }
+
+ /**
+ * Returns the value indicating if diskBalancer is enabled.
+ *
+ * @return boolean.
+ */
+ @VisibleForTesting
+ public boolean isDiskBalancerEnabled() {
+ return isDiskBalancerEnabled;
+ }
+
+ /**
+ * Sets maximum amount of time disk balancer plan is valid.
+ *
+ * @param planValidityInterval
Review Comment:
add description for the param
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancer.java:
##########
@@ -341,6 +341,59 @@ private void checkDiskBalancerEnabled()
}
}
+ /**
+ * Sets Disk balancer is to enable or not to enable.
+ *
+ * @param diskBalancerEnabled
+ * true, enable diskBalancer, otherwise false to disable it.
+ */
+ public void setDiskBalancerEnabled(boolean diskBalancerEnabled) {
+ isDiskBalancerEnabled = diskBalancerEnabled;
+ }
+
+ /**
+ * Returns the value indicating if diskBalancer is enabled.
+ *
+ * @return boolean.
+ */
+ @VisibleForTesting
+ public boolean isDiskBalancerEnabled() {
+ return isDiskBalancerEnabled;
+ }
+
+ /**
+ * Sets maximum amount of time disk balancer plan is valid.
+ *
+ * @param planValidityInterval
+ */
+ public void setPlanValidityInterval(long planValidityInterval) {
+
this.config.setTimeDuration(DFSConfigKeys.DFS_DISK_BALANCER_PLAN_VALID_INTERVAL,
+ planValidityInterval, TimeUnit.MILLISECONDS);
+ this.planValidityInterval = planValidityInterval;
+ }
+
+ /**
+ * Gets maximum amount of time disk balancer plan is valid, then
milliseconds is assumed.
+ *
+ * @return long
Review Comment:
it returns ``plan validity interval``, rather than putting the return type
add description
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancer.java:
##########
@@ -341,6 +341,59 @@ private void checkDiskBalancerEnabled()
}
}
+ /**
+ * Sets Disk balancer is to enable or not to enable.
+ *
+ * @param diskBalancerEnabled
+ * true, enable diskBalancer, otherwise false to disable it.
+ */
+ public void setDiskBalancerEnabled(boolean diskBalancerEnabled) {
+ isDiskBalancerEnabled = diskBalancerEnabled;
+ }
+
+ /**
+ * Returns the value indicating if diskBalancer is enabled.
+ *
+ * @return boolean.
+ */
+ @VisibleForTesting
+ public boolean isDiskBalancerEnabled() {
+ return isDiskBalancerEnabled;
+ }
+
+ /**
+ * Sets maximum amount of time disk balancer plan is valid.
+ *
+ * @param planValidityInterval
+ */
+ public void setPlanValidityInterval(long planValidityInterval) {
+
this.config.setTimeDuration(DFSConfigKeys.DFS_DISK_BALANCER_PLAN_VALID_INTERVAL,
+ planValidityInterval, TimeUnit.MILLISECONDS);
+ this.planValidityInterval = planValidityInterval;
+ }
+
+ /**
+ * Gets maximum amount of time disk balancer plan is valid, then
milliseconds is assumed.
+ *
+ * @return long
+ */
+ @VisibleForTesting
+ public long getPlanValidityInterval() {
+ return planValidityInterval;
+ }
+
+ /**
+ * Gets maximum amount of time disk balancer plan is valid in config,
+ * then milliseconds is assumed.
+ *
+ * @return long
Review Comment:
return description not data type
--
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]
