ayushtkn commented on a change in pull request #3559:
URL: https://github.com/apache/hadoop/pull/3559#discussion_r731901083
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
##########
@@ -1195,13 +1195,24 @@
"dfs.namenode.available-space-block-placement-policy.balanced-space-preference-fraction";
public static final float
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_DEFAULT
=
0.6f;
+ public static final String
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERATE_KEY
=
+
"dfs.namenode.available-space-block-placement-policy.balanced-space-tolerate";
+ public static final int
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERATE_DEFAULT
=
+ 5;
public static final String
DFS_NAMENODE_AVAILABLE_SPACE_RACK_FAULT_TOLERANT_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY
=
"dfs.namenode.available-space-rack-fault-tolerant-block-placement-policy"
+ ".balanced-space-preference-fraction";
public static final float
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_RACK_FAULT_TOLERANT_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_DEFAULT
=
0.6f;
+ public static final String
+
DFS_NAMENODE_AVAILABLE_SPACE_RACK_FAULT_TOLERANT_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERATE_KEY
=
+
"dfs.namenode.available-space-rack-fault-tolerant-block-placement-policy"
+ + ".balanced-space-tolerate";
Review comment:
Can you change tolerate to tolerance everywhere in the config names
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/AvailableSpaceBlockPlacementPolicy.java
##########
@@ -37,6 +34,8 @@
import org.apache.hadoop.net.NetworkTopology;
import org.apache.hadoop.net.Node;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.*;
+
Review comment:
Don't club imports. Keep all imports separate and the existing ones
untouched
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HdfsBlockPlacementPolicies.md
##########
@@ -119,6 +119,24 @@ The AvailableSpaceBlockPlacementPolicy is a space balanced
block placement polic
</description>
</property>
+<property>
+<name>dfs.namenode.available-space-block-placement-policy.balanced-space-tolerate</name>
+<value>5</value>
+<description>
+ Special value between 0 and 100, noninclusive. Increases tolerance of
+ placing blocks on Datanodes with similar disk space used.
+</description>
+</property>
+
+<property>
+
<name>dfs.namenode.available-space-block-placement-policy.datanode-usgae-tolerace-fraction</name>
+ <value>0.6</value>
+ <description>
+ Special value between 1 and 100, noninclusive. Increases chance of
+ placing blocks on Datanodes with less disk space used.
+</description>
+</property>
Review comment:
Didn't catch what is this? Some copy-paste error?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/AvailableSpaceBlockPlacementPolicy.java
##########
@@ -62,6 +63,11 @@ public void initialize(Configuration conf, FSClusterStats
stats,
+
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY
+ " = " + balancedPreferencePercent);
+ balancedTolerate =
+ conf.getInt(
+
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERATE_KEY,
+
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERATE_DEFAULT);
+
Review comment:
Rename this variable, current name doesn't make much sense.
I think we should have some validation on this value, and an upper limit as
well, May be at max 20,(document that as well)
If the value is not in the valid range(0-20) add a warn log and use the
default value.
Same for the other class as well
--
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]