Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDFS-17210. Optimize AvailableSpaceBlockPlacementPolicy. #6113

Merged
merged 13 commits into from
Oct 16, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,12 @@ public class DFSConfigKeys extends CommonConfigurationKeys {
public static final int
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_DEFAULT =
5;
public static final String
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_KEY =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align as other configuration item.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @Hexiaoqiao ,i kept the Align as other configuration item and fixed the checkstyle items. but DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_DEFAULT is still a bit long(over 100 per line), do you have better ideas?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is OK for me now. The root cause is name of this constant is too long, but I also have no other choice to be better. Maybe we could care that when name next time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, thanks @Hexiaoqiao for your review~

"dfs.namenode.available-space-block-placement-policy.balanced-space-tolerance-limit";
public static final int
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_DEFAULT =
100;
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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_DEFAULT;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_DEFAULT;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_KEY;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_KEY;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_DEFAULT;

import java.util.Collection;
import java.util.EnumMap;
Expand Down Expand Up @@ -51,6 +53,9 @@ public class AvailableSpaceBlockPlacementPolicy extends
(int) (100 * DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_DEFAULT);
private int balancedSpaceTolerance =
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_DEFAULT;

private int balancedSpaceToleranceLimit =
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_DEFAULT;
private boolean optimizeLocal;

@Override
Expand All @@ -71,6 +76,11 @@ public void initialize(Configuration conf, FSClusterStats stats,
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_KEY,
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_DEFAULT);

balancedSpaceToleranceLimit =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to check if this value is between 0 and 100. Otherwise, it could work abnormally.

conf.getInt(
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_KEY,
DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_TOLERANCE_LIMIT_DEFAULT);

optimizeLocal = conf.getBoolean(
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCE_LOCAL_NODE_KEY,
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCE_LOCAL_NODE_DEFAULT);
Expand Down Expand Up @@ -201,8 +211,12 @@ private DatanodeDescriptor select(DatanodeDescriptor a, DatanodeDescriptor b,
*/
protected int compareDataNode(final DatanodeDescriptor a,
final DatanodeDescriptor b, boolean isBalanceLocal) {

boolean toleranceLimit = Math.max(a.getDfsUsedPercent(), b.getDfsUsedPercent())
< balancedSpaceToleranceLimit;
if (a.equals(b)
|| Math.abs(a.getDfsUsedPercent() - b.getDfsUsedPercent()) < balancedSpaceTolerance || ((
|| (toleranceLimit && Math.abs(a.getDfsUsedPercent() - b.getDfsUsedPercent())
< balancedSpaceTolerance) || ((
isBalanceLocal && a.getDfsUsedPercent() < 50))) {
return 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5164,6 +5164,18 @@
</description>
</property>

<property>
<name>dfs.namenode.available-space-block-placement-policy.balanced-space-tolerance-limit</name>
<value>100</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 100, inclusive. if the comparable datanodes usage both beyond
${dfs.namenode.available-space-block-placement-policy.balanced-space-tolerance-limit},
dfs.namenode.available-space-block-placement-policy.balanced-space-tolerance will be disable.
</description>
</property>

<property>
<name>
dfs.namenode.available-space-block-placement-policy.balance-local-node
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ public class TestAvailableSpaceBlockPlacementPolicy {
@BeforeClass
public static void setupCluster() throws Exception {
conf = new HdfsConfiguration();
conf.setFloat(
DFSConfigKeys.DFS_NAMENODE_AVAILABLE_SPACE_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY,
0.6f);
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_LIMIT_KEY,
93);
String[] racks = new String[numRacks];
for (int i = 0; i < numRacks; i++) {
racks[i] = "/rack" + i;
Expand Down Expand Up @@ -219,6 +222,77 @@ public void testChooseSimilarDataNode() {
tolerateDataNodes[0], false) == 1);
}


@Test
public void testCompareDataNode() {
DatanodeDescriptor[] tolerateDataNodes;
DatanodeStorageInfo[] tolerateStorages;
int capacity = 5;
Collection<Node> allTolerateNodes = new ArrayList<>(capacity);
String[] ownerRackOfTolerateNodes = new String[capacity];
for (int i = 0; i < capacity; i++) {
ownerRackOfTolerateNodes[i] = "rack"+i;
}
tolerateStorages = DFSTestUtil.createDatanodeStorageInfos(ownerRackOfTolerateNodes);
tolerateDataNodes = DFSTestUtil.toDatanodeDescriptor(tolerateStorages);

Collections.addAll(allTolerateNodes, tolerateDataNodes);
final BlockManager bm = namenode.getNamesystem().getBlockManager();
AvailableSpaceBlockPlacementPolicy toleratePlacementPolicy =
(AvailableSpaceBlockPlacementPolicy)bm.getBlockPlacementPolicy();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to keep the alignment as usual. You should add the 'checkstyle.xml' to your IDE or follow this guide docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Hexiaoqiao , will fix the checkstyle soon~


//96.6%
updateHeartbeatWithUsage(tolerateDataNodes[0],
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
29 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
HdfsServerConstants.MIN_BLOCKS_FOR_WRITE
* blockSize, 0L, 0L, 0L, 0, 0);

//93.3%
updateHeartbeatWithUsage(tolerateDataNodes[1],
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
28 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
HdfsServerConstants.MIN_BLOCKS_FOR_WRITE
* blockSize, 0L, 0L, 0L, 0, 0);

//90.0%
updateHeartbeatWithUsage(tolerateDataNodes[2],
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
27 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
HdfsServerConstants.MIN_BLOCKS_FOR_WRITE
* blockSize, 0L, 0L, 0L, 0, 0);

//86.6%
updateHeartbeatWithUsage(tolerateDataNodes[3],
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
26 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
HdfsServerConstants.MIN_BLOCKS_FOR_WRITE
* blockSize, 0L, 0L, 0L, 0, 0);
//83.3%
updateHeartbeatWithUsage(tolerateDataNodes[4],
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
25 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize,
HdfsServerConstants.MIN_BLOCKS_FOR_WRITE
* blockSize, 0L, 0L, 0L, 0, 0);

assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[0],
tolerateDataNodes[1], false) == 1);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[1],
tolerateDataNodes[0], false) == -1);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[1],
tolerateDataNodes[2], false) == 1);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[2],
tolerateDataNodes[1], false) == -1);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[2],
tolerateDataNodes[3], false) == 0);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[3],
tolerateDataNodes[2], false) == 0);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[2],
tolerateDataNodes[4], false) == 1);
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[4],
tolerateDataNodes[2], false) == -1);
}

@AfterClass
public static void teardownCluster() {
if (namenode != null) {
Expand Down