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]

Reply via email to