You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/08 15:25:56 UTC

[GitHub] [ozone] symious opened a new pull request, #3663: HDDS-7106. Client-SCM interface

symious opened a new pull request, #3663:
URL: https://github.com/apache/ozone/pull/3663

   ## What changes were proposed in this pull request?
   
   This ticket is for creating the interface between Client and SCM. The new interfaces are as follows:
   
   * getDiskBalancerReport
     * Return datanodes' volume data density
   * getDiskBalancerStatus
     * Return diskbalancer's information of datanode in balancing
   * startDiskBalancer
   * stopDiskBalancer
   * updateDiskBalancerConfiguration
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7106
   
   ## How was this patch tested?
   
   unit test.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941048753


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java:
##########
@@ -391,4 +391,35 @@ StatusAndMessages finalizeScmUpgrade(String upgradeClientID)
   StatusAndMessages queryUpgradeFinalizationProgress(
       String upgradeClientID, boolean force, boolean readonly)
       throws IOException;
+
+  List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(

Review Comment:
   Maybe we can call these two API both getDiskBalancerInfo?  Please also add comments that this count means top N DNs, not random DNs. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1216015909

   @errose28 Thank your for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ferhui commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ferhui commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1216078474

   It seems no new comments for several days. @ChenSammi @siddhantsangwan @errose28 Thanks for your careful reviews! I will merge it so that @symious can go ahead.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r946680067


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   Please use uint64 instead. diskBandwidth is a number without fraction.  We can change it in next PR. 



##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   @symious ,  Please use uint64 instead. diskBandwidth is a number without fraction.  We can change it in next PR. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1209112746

   @symious  thanks for quickly providing the first patch.  I left a few comments. 


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r943073098


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},

Review Comment:
   @siddhantsangwan Thanks for the review, added a new tag.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941029248


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   node, diskBalanderRunning and currentVolumeDensitySum should be required instead of optional.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r946681533


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java:
##########
@@ -391,4 +391,35 @@ StatusAndMessages finalizeScmUpgrade(String upgradeClientID)
   StatusAndMessages queryUpgradeFinalizationProgress(
       String upgradeClientID, boolean force, boolean readonly)
       throws IOException;
+
+  List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(

Review Comment:
   @symious Could you explain a bit about the major difference between Report and Status? 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r943072973


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "Threshold is a percentage in the range of 0 to 100. A " +
+          "datanode is considered balanced if for each volume, the " +
+          "utilization of the volume(used space to capacity ratio) differs" +
+          " from the utilization of the datanode(used space to capacity ratio" +
+          " of the entire datanode) no more than the threshold.")
+  private double threshold = 10d;
+
+  @Config(key = "max.disk.throughputInMBPerSec", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "The max balance speed.")
+  private double diskBandwidth = 10;
+
+  @Config(key = "parallel.thread", type = ConfigType.AUTO,
+      defaultValue = "5", tags = {ConfigTag.BALANCER},
+      description = "The max parallel balance thread count.")
+  private int parallelThread = 5;
+
+  /**
+   * Gets the threshold value for Container Balancer.

Review Comment:
   Fixed.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ferhui commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ferhui commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1212715131

   @ChenSammi @siddhantsangwan Thanks for your reviews! Do you have any other comments?


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] siddhantsangwan commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r942103114


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "Threshold is a percentage in the range of 0 to 100. A " +
+          "datanode is considered balanced if for each volume, the " +
+          "utilization of the volume(used space to capacity ratio) differs" +
+          " from the utilization of the datanode(used space to capacity ratio" +
+          " of the entire datanode) no more than the threshold.")
+  private double threshold = 10d;
+
+  @Config(key = "max.disk.throughputInMBPerSec", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "The max balance speed.")
+  private double diskBandwidth = 10;
+
+  @Config(key = "parallel.thread", type = ConfigType.AUTO,
+      defaultValue = "5", tags = {ConfigTag.BALANCER},
+      description = "The max parallel balance thread count.")
+  private int parallelThread = 5;
+
+  /**
+   * Gets the threshold value for Container Balancer.

Review Comment:
   Change "Container Balancer" to "Disk Balancer"



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},

Review Comment:
   Do we want to create a new ConfigTag? Something like DiskBalancer?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ferhui commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ferhui commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r940887525


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "Threshold is a percentage in the range of 0 to 100. A " +
+          "datanode is considered balanced if for each volume, the " +
+          "utilization of the volume(used space to capacity ratio) differs" +
+          " from the utilization of the datanode(used space to capacity ratio" +
+          " of the entire datanode) no more than the threshold.")
+  private String threshold = "10";
+
+  @Config(key = "max.disk.throughputInMBPerSec", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "The max balance speed")
+  private String diskBandwidth = "10";
+
+  @Config(key = "parallel.thread", type = ConfigType.AUTO,
+      defaultValue = "5", tags = {ConfigTag.BALANCER},
+      description = "The max parallel balance thread count")
+  private int parallelThread = 5;
+
+  /**
+   * Gets the threshold value for Container Balancer.
+   *
+   * @return percentage value in the range 0 to 100
+   */
+  public double getThreshold() {
+    return Double.parseDouble(threshold);
+  }
+
+  public double getThresholdAsRatio() {
+    return Double.parseDouble(threshold) / 100;
+  }
+
+  /**
+   * Sets the threshold value for Disk Balancer.
+   *
+   * @param threshold a percentage value in the range 0 to 100
+   */
+  public void setThreshold(double threshold) {
+    if (threshold < 0d || threshold >= 100d) {
+      throw new IllegalArgumentException(
+          "Threshold must be a percentage(double) in the range 0 to 100.");
+    }
+    this.threshold = String.valueOf(threshold);
+  }
+
+  /**
+   * Gets the disk bandwidth value for Disk Balancer.
+   *
+   * @return max disk bandwidth per second
+   */
+  public double getDiskBandwidth() {
+    return Double.parseDouble(diskBandwidth);
+  }
+
+  /**
+   * Sets the disk bandwidth value for Disk Balancer.
+   *
+   * @param

Review Comment:
   Miss anything here?



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/DiskBalancerConfiguration.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.storage;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigTag;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class contains configuration values for the DiskBalancer.
+ */
+@ConfigGroup(prefix = "hdds.datanode.disk.balancer")
+public final class DiskBalancerConfiguration {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerConfiguration.class);
+
+  @Config(key = "volume.density.threshold", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "Threshold is a percentage in the range of 0 to 100. A " +
+          "datanode is considered balanced if for each volume, the " +
+          "utilization of the volume(used space to capacity ratio) differs" +
+          " from the utilization of the datanode(used space to capacity ratio" +
+          " of the entire datanode) no more than the threshold.")
+  private String threshold = "10";
+
+  @Config(key = "max.disk.throughputInMBPerSec", type = ConfigType.AUTO,
+      defaultValue = "10", tags = {ConfigTag.BALANCER},
+      description = "The max balance speed")
+  private String diskBandwidth = "10";
+
+  @Config(key = "parallel.thread", type = ConfigType.AUTO,
+      defaultValue = "5", tags = {ConfigTag.BALANCER},
+      description = "The max parallel balance thread count")
+  private int parallelThread = 5;
+
+  /**
+   * Gets the threshold value for Container Balancer.
+   *
+   * @return percentage value in the range 0 to 100
+   */
+  public double getThreshold() {
+    return Double.parseDouble(threshold);
+  }
+
+  public double getThresholdAsRatio() {
+    return Double.parseDouble(threshold) / 100;
+  }
+
+  /**
+   * Sets the threshold value for Disk Balancer.
+   *
+   * @param threshold a percentage value in the range 0 to 100
+   */
+  public void setThreshold(double threshold) {
+    if (threshold < 0d || threshold >= 100d) {
+      throw new IllegalArgumentException(
+          "Threshold must be a percentage(double) in the range 0 to 100.");
+    }
+    this.threshold = String.valueOf(threshold);
+  }
+
+  /**
+   * Gets the disk bandwidth value for Disk Balancer.
+   *
+   * @return max disk bandwidth per second
+   */
+  public double getDiskBandwidth() {
+    return Double.parseDouble(diskBandwidth);
+  }
+
+  /**
+   * Sets the disk bandwidth value for Disk Balancer.
+   *
+   * @param
+   */
+  public void setDiskBandwidth(double diskBandwidth) {
+    if (diskBandwidth <= 0d) {
+      throw new IllegalArgumentException(
+          "diskBandwidth must be a value larger than 0.");
+    }
+    this.diskBandwidth = String.valueOf(diskBandwidth);
+  }
+
+  /**
+   * Gets the parallel thread for Disk Balancer.
+   *
+   * @return parallel thread
+   */
+  public int getParallelThread() {
+    return parallelThread;
+  }
+
+  /**
+   * Sets the parallel thread for Disk Balancer.
+   *
+   * @param

Review Comment:
   and here



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941029248


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   This should be required instead of optional.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ferhui merged pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ferhui merged PR #3663:
URL: https://github.com/apache/ozone/pull/3663


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941029248


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   1. node, diskBalanderRunning and currentVolumeDensitySum should be required instead of optional.
   2. use double  for threashold, and uint64 for diskBandwidth. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941083487


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {

Review Comment:
   Why isRunning should be checked? I think if it's not running, we still need return it's disk balancer info, right? 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941090080


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {

Review Comment:
   DiskBalancerStatus is suppose to only return datanodes in balancing, or return the status of the datanode that user want to check, that's also a reason "DiskBalancerStatus" and "DiskBalancerInfo" are separated.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941087764


##########
hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto:
##########
@@ -322,6 +328,28 @@ message DatanodeUsageInfoResponseProto {
   repeated DatanodeUsageInfoProto info = 1;
 }
 
+/*
+  Datanode disk balancer report request message.
+*/
+message DatanodeDiskBalancerReportRequestProto {
+  optional uint32 count = 1;

Review Comment:
   Please use one request and one response to wrap the two getDiskBalancerInfo API. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ferhui commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ferhui commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1209002885

   @JacksonYao287 @ChenSammi could you please have a look?


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941496179


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   > node, diskBalanderRunning and currentVolumeDensitySum should be required instead of optional.
   
   Changed "node" and "currentVolumeDensitySum" to required, but kept "diskBalancerRunning" as optional since "Report" command won't return the running status.
   
   > use double for threashold, and uint64 for diskBandwidth, same for DiskBalancerConfigurationProto.
   
   Used double for threshold and diskBandwidth (seems double is better?) 



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {
+        double volumeDensitySum =
+            getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+        statusList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+            .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+            .setDiskBalancerRunning(isRunning(datanodeDetails))
+            .setDiskBalancerConf(statusMap.get(datanodeDetails)
+                .getDiskBalancerConfiguration().toProtobufBuilder())
+            .setNode(datanodeDetails.toProto(clientVersion))
+            .build());
+      }
+    }
+
+    return statusList;
+  }
+
+  /**
+   * Get volume density for a specific DatanodeDetails node.
+   *
+   * @param datanodeDetails DatanodeDetails
+   * @return DiskBalancer report.
+   */
+  private double getVolumeDataDensitySumForDatanodeDetails(
+      DatanodeDetails datanodeDetails) {
+    Preconditions.checkArgument(datanodeDetails instanceof DatanodeInfo);
+
+    DatanodeInfo datanodeInfo = (DatanodeInfo) datanodeDetails;
+
+    double totalCapacity = 0d, totalUsed = 0d;
+    for (StorageContainerDatanodeProtocolProtos.StorageReportProto reportProto :
+        datanodeInfo.getStorageReports()) {
+      totalCapacity += reportProto.getCapacity();
+      totalUsed += reportProto.getScmUsed();
+    }
+
+    Preconditions.checkArgument(totalCapacity != 0);
+    double idealUsage = totalUsed / totalCapacity;
+
+    double volumeDensitySum = datanodeInfo.getStorageReports().stream()
+        .map(report ->
+            Math.abs((double)report.getScmUsed() / report.getCapacity()
+                - idealUsage))
+        .mapToDouble(Double::valueOf).sum();
+
+    return volumeDensitySum;
+  }
+
+  private List<DatanodeDetails> mapHostnamesToDatanodes(List<String> hosts)

Review Comment:
   Updated.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941501249


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {

Review Comment:
   Updated the code to filter datanodes.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941502797


##########
hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto:
##########
@@ -322,6 +328,28 @@ message DatanodeUsageInfoResponseProto {
   repeated DatanodeUsageInfoProto info = 1;
 }
 
+/*
+  Datanode disk balancer report request message.
+*/
+message DatanodeDiskBalancerReportRequestProto {
+  optional uint32 count = 1;

Review Comment:
   Updated to use DatanodeDiskBalancerInfoRequest and DatanodeDiskBalancerInfoResponse.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1216081324

   @ferhui Thank you for the merge. @ChenSammi @ferhui @siddhantsangwan @errose28 Thank you for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1208278062

   @ChenSammi @ferhui @lokeshj1703 @siddhantsangwan @sodonnel @neils-dev @JacksonYao287 Could you help to review this PR?


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
symious commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1208925350

   @ferhui Thanks for the review. Updated the PR, please have a check.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941041197


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {
+        double volumeDensitySum =
+            getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+        statusList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+            .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+            .setDiskBalancerRunning(isRunning(datanodeDetails))
+            .setDiskBalancerConf(statusMap.get(datanodeDetails)
+                .getDiskBalancerConfiguration().toProtobufBuilder())
+            .setNode(datanodeDetails.toProto(clientVersion))
+            .build());
+      }
+    }
+
+    return statusList;
+  }
+
+  /**
+   * Get volume density for a specific DatanodeDetails node.
+   *
+   * @param datanodeDetails DatanodeDetails
+   * @return DiskBalancer report.
+   */
+  private double getVolumeDataDensitySumForDatanodeDetails(
+      DatanodeDetails datanodeDetails) {
+    Preconditions.checkArgument(datanodeDetails instanceof DatanodeInfo);
+
+    DatanodeInfo datanodeInfo = (DatanodeInfo) datanodeDetails;
+
+    double totalCapacity = 0d, totalUsed = 0d;
+    for (StorageContainerDatanodeProtocolProtos.StorageReportProto reportProto :
+        datanodeInfo.getStorageReports()) {
+      totalCapacity += reportProto.getCapacity();
+      totalUsed += reportProto.getScmUsed();
+    }
+
+    Preconditions.checkArgument(totalCapacity != 0);
+    double idealUsage = totalUsed / totalCapacity;
+
+    double volumeDensitySum = datanodeInfo.getStorageReports().stream()
+        .map(report ->
+            Math.abs((double)report.getScmUsed() / report.getCapacity()
+                - idealUsage))
+        .mapToDouble(Double::valueOf).sum();
+
+    return volumeDensitySum;
+  }
+
+  private List<DatanodeDetails> mapHostnamesToDatanodes(List<String> hosts)

Review Comment:
   Please move this function to some utility class, so that disk balancer and node decommission can both use it.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941083487


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DiskBalancerManager.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.storage.DiskBalancerConfiguration;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+
+/**
+ * Maintains information about the DiskBalancer on SCM side.
+ */
+public class DiskBalancerManager {
+  public static final Logger LOG =
+      LoggerFactory.getLogger(DiskBalancerManager.class);
+
+  private final EventPublisher scmNodeEventPublisher;
+  private final SCMContext scmContext;
+  private final NodeManager nodeManager;
+  private Map<DatanodeDetails, DiskBalancerStatus> statusMap;
+  private boolean useHostnames;
+
+  /**
+   * Constructs DiskBalancer Manager.
+   */
+  public DiskBalancerManager(OzoneConfiguration conf,
+                        EventPublisher eventPublisher,
+                        SCMContext scmContext,
+                        NodeManager nodeManager) {
+    this.scmNodeEventPublisher = eventPublisher;
+    this.scmContext = scmContext;
+    this.nodeManager = nodeManager;
+    this.useHostnames = conf.getBoolean(
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME,
+        DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+    this.statusMap = new ConcurrentHashMap<>();
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerReport(
+      int count, int clientVersion) throws IOException {
+
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> reportList =
+        new ArrayList<>();
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      double volumeDensitySum =
+          getVolumeDataDensitySumForDatanodeDetails(datanodeDetails);
+      reportList.add(HddsProtos.DatanodeDiskBalancerInfoProto.newBuilder()
+          .setCurrentVolumeDensitySum(String.valueOf(volumeDensitySum))
+          .setNode(datanodeDetails.toProto(clientVersion))
+          .build());
+    }
+
+    reportList.sort((t1, t2) -> t2.getCurrentVolumeDensitySum().
+        compareTo(t1.getCurrentVolumeDensitySum()));
+    return reportList.stream().limit(count).collect(Collectors.toList());
+  }
+
+  public List<HddsProtos.DatanodeDiskBalancerInfoProto> getDiskBalancerStatus(
+      Optional<List<String>> hosts, int clientVersion) throws IOException {
+    List<HddsProtos.DatanodeDiskBalancerInfoProto> statusList =
+        new ArrayList<>();
+    List<DatanodeDetails> filterDns = null;
+
+    if (hosts.isPresent() && !hosts.get().isEmpty()) {
+      filterDns = mapHostnamesToDatanodes(hosts.get());
+    }
+
+    for (DatanodeDetails datanodeDetails: nodeManager.getNodes(IN_SERVICE,
+        HddsProtos.NodeState.HEALTHY)) {
+      if ((filterDns != null && !filterDns.isEmpty() &&
+          filterDns.contains(datanodeDetails) &&
+          statusMap.containsKey(datanodeDetails)) ||
+          isRunning(datanodeDetails)) {

Review Comment:
   Why isRunning should be checked? I think if it's not running, we still need return it's disk balancer info, right? 
   
    Also I don't fully get the point of this statusMap, could you explain it a bit more?  



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3663:
URL: https://github.com/apache/ozone/pull/3663#discussion_r941029248


##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -451,3 +451,16 @@ message ContainerBalancerConfigurationProto {
     required bool shouldRun = 18;
     optional int32 nextIterationIndex = 19;
 }
+
+message DiskBalancerConfigurationProto {
+    optional string threshold = 1;
+    optional string diskBandwidth = 2;
+    optional int32 parallelThread = 3;
+}
+
+message DatanodeDiskBalancerInfoProto {
+    optional DatanodeDetailsProto node = 1;

Review Comment:
   1. node, diskBalanderRunning and currentVolumeDensitySum should be required instead of optional.
   2. use double  for threashold, and uint64 for diskBandwidth, same for DiskBalancerConfigurationProto.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] errose28 commented on pull request #3663: HDDS-7106. [DiskBalancer] Client-SCM interface

Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3663:
URL: https://github.com/apache/ozone/pull/3663#issuecomment-1215771924

   We should hold off on proto changes or incomplete command line options until the 1.3.0 branch is cut. If the branch is cut with these changes the protos will be locked and unable to change even though the feature is not ready to use. @captainzmc is currently working on getting a release branch AFAIK.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org