ayushtkn commented on a change in pull request #3559:
URL: https://github.com/apache/hadoop/pull/3559#discussion_r733299381
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/AvailableSpaceBlockPlacementPolicy.java
##########
@@ -37,6 +39,7 @@
import org.apache.hadoop.net.NetworkTopology;
import org.apache.hadoop.net.Node;
+
Review comment:
nit:
Avoid this
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestAvailableSpaceRackFaultTolerantBPP.java
##########
@@ -66,6 +68,9 @@ public static void setupCluster() throws Exception {
conf.setFloat(
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY,
0.6f);
+ conf.setInt(
+
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_RACK_FAULT_TOLERANT_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_KEY,
+ 5);
Review comment:
It is by default 5. You need not to set it explicitly.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestAvailableSpaceRackFaultTolerantBPP.java
##########
@@ -206,6 +210,48 @@ public void testMaxRackAllocation() {
assertEquals(REPLICA, racks.size());
}
+ @Test
+ public void testChooseSimilarDataNode() {
+ DatanodeDescriptor[] tolerateDataNodes;
+ DatanodeStorageInfo[] tolerateStorages;
+ AvailableSpaceRackFaultTolerantBlockPlacementPolicy
toleratePlacementPolicy;
Review comment:
Why are you declaring it separately. Could have done this below:
```
AvailableSpaceRackFaultTolerantBlockPlacementPolicy toleratePlacementPolicy
=
(AvailableSpaceRackFaultTolerantBlockPlacementPolicy)bm.getBlockPlacementPolicy();
```
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestAvailableSpaceBlockPlacementPolicy.java
##########
@@ -61,6 +62,9 @@ public static void setupCluster() throws Exception {
conf.setFloat(
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY,
0.6f);
+ conf.setInt(
+
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_KEY,
+ 5);
Review comment:
It is by default 5. You need not to set it explicitly.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/AvailableSpaceRackFaultTolerantBlockPlacementPolicy.java
##########
@@ -70,6 +77,19 @@ public void initialize(Configuration conf, FSClusterStats
stats,
+ " is less than 0.5 so datanodes with more used percent will"
+ " receive more block allocations.");
}
+
+
+ if (balancedSpaceTolerance >= 20 || balancedSpaceTolerance <= 0) {
+ LOG.warn("The value of "
+ +
DFS_NAMENODE_AVAILABLE_SPACE_RACK_FAULT_TOLERANT_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_KEY
+ + " is invalid, Default value " +
+
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_RACK_FAULT_TOLERANT_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_DEFAULT
+ + " will be used instead. Increases tolerance of"
+ + " placing blocks on Datanodes with similar disk space used ");
Review comment:
No need of Increases to...
You can just stay `till will be used`
Remove the line after that
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
##########
@@ -5067,6 +5067,18 @@
</description>
</property>
+ <property>
+
<name>dfs.namenode.available-space-block-placement-policy.balanced-space-tolerance</name>
+ <value>5</value>
+ <description>
+ Only used when the dfs.block.replicator.classname is set to
+
org.apache.hadoop.hdfs.server.blockmanagement.AvailableSpaceBlockPlacementPolicy.
+ Special value between 0 and 20 , noninclusive. if the value is set
beyond the scope,
Review comment:
Change it to include 0 & 20
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/AvailableSpaceBlockPlacementPolicy.java
##########
@@ -77,6 +87,17 @@ public void initialize(Configuration conf, FSClusterStats
stats,
+ " is less than 0.5 so datanodes with more used percent will"
+ " receive more block allocations.");
}
+
+ if (balancedSpaceTolerance >= 20 || balancedSpaceTolerance <= 0) {
Review comment:
I think 0 we can allow, May be just remove the = sign for both.
--
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]
