-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
Changes from 6 commits
5a45a6c
bccc4c4
c1caab9
86b9f3d
d857387
5dcd0ec
7ee4231
2b47e25
74f2f68
2c9517c
b8af4bb
04245a7
ec8977e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 | ||
|
@@ -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 = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
@@ -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.min(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; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -219,6 +222,61 @@ public void testChooseSimilarDataNode() { | |
tolerateDataNodes[0], false) == 1); | ||
} | ||
|
||
|
||
@Test | ||
public void testCompareDataNode() { | ||
DatanodeDescriptor[] tolerateDataNodes; | ||
DatanodeStorageInfo[] tolerateStorages; | ||
int capacity = 4; | ||
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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @Hexiaoqiao , will fix the checkstyle soon~ |
||
|
||
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); | ||
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); | ||
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); | ||
|
||
updateHeartbeatWithUsage(tolerateDataNodes[3], | ||
30 * HdfsServerConstants.MIN_BLOCKS_FOR_WRITE * blockSize, | ||
20 * 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) == 0); | ||
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[2], | ||
tolerateDataNodes[1], false) == 0); | ||
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[2], | ||
tolerateDataNodes[3], false) == 1); | ||
assertTrue(toleratePlacementPolicy.compareDataNode(tolerateDataNodes[3], | ||
tolerateDataNodes[2], false) == -1); | ||
} | ||
|
||
@AfterClass | ||
public static void teardownCluster() { | ||
if (namenode != null) { | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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~