From fa86ec99a1ff0bef4fb322a82a68667ebd733926 Mon Sep 17 00:00:00 2001 From: Arpit Agarwal Date: Fri, 24 Feb 2017 15:44:11 -0800 Subject: [PATCH] HDFS-11295. Check storage remaining instead of node remaining in BlockPlacementPolicyDefault.chooseReplicaToDelete(). Contributed by Marton Elek. --- .../BlockPlacementPolicyDefault.java | 2 +- .../blockmanagement/DatanodeStorageInfo.java | 5 +++ .../TestReplicationPolicy.java | 35 +++++++++++++------ .../TestReplicationPolicyWithNodeGroup.java | 23 +++++++++--- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java index ec2e4ba9271..6fab722cf41 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java @@ -966,7 +966,7 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy { } final DatanodeDescriptor node = storage.getDatanodeDescriptor(); - long free = node.getRemaining(); + long free = storage.getRemaining(); long lastHeartbeat = node.getLastUpdateMonotonic(); if (lastHeartbeat < oldestHeartbeat) { oldestHeartbeat = lastHeartbeat; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java index 862b1bfedfb..6474e3fd5be 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java @@ -382,6 +382,11 @@ public class DatanodeStorageInfo { return null; } + @VisibleForTesting + void setRemainingForTests(int remaining) { + this.remaining = remaining; + } + static enum AddBlockResult { ADDED, REPLACED, ALREADY_EXIST } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java index 21839c67fa8..d2cd91930d0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java @@ -950,24 +950,31 @@ public class TestReplicationPolicy extends BaseReplicationPolicyTest { List replicaList = new ArrayList<>(); final Map> rackMap = new HashMap>(); - - dataNodes[0].setRemaining(4*1024*1024); + + storages[0].setRemainingForTests(4*1024*1024); + dataNodes[0].setRemaining(calculateRemaining(dataNodes[0])); replicaList.add(storages[0]); - - dataNodes[1].setRemaining(3*1024*1024); + + storages[1].setRemainingForTests(3*1024*1024); + dataNodes[1].setRemaining(calculateRemaining(dataNodes[1])); replicaList.add(storages[1]); - - dataNodes[2].setRemaining(2*1024*1024); + + storages[2].setRemainingForTests(2*1024*1024); + dataNodes[2].setRemaining(calculateRemaining(dataNodes[2])); replicaList.add(storages[2]); - - dataNodes[5].setRemaining(1*1024*1024); + + //Even if this node has the most space, because the storage[5] has + //the lowest it should be chosen in case of block delete. + storages[4].setRemainingForTests(100 * 1024 * 1024); + storages[5].setRemainingForTests(512 * 1024); + dataNodes[5].setRemaining(calculateRemaining(dataNodes[5])); replicaList.add(storages[5]); - + // Refresh the last update time for all the datanodes for (int i = 0; i < dataNodes.length; i++) { DFSTestUtil.resetLastUpdatesWithOffset(dataNodes[i], 0); } - + List first = new ArrayList<>(); List second = new ArrayList<>(); replicator.splitNodesWithRack(replicaList, rackMap, first, second); @@ -998,6 +1005,14 @@ public class TestReplicationPolicy extends BaseReplicationPolicyTest { assertEquals(chosen, storages[1]); } + private long calculateRemaining(DatanodeDescriptor dataNode) { + long sum = 0; + for (DatanodeStorageInfo storageInfo: dataNode.getStorageInfos()){ + sum += storageInfo.getRemaining(); + } + return sum; + } + @Test public void testChooseReplicasToDelete() throws Exception { Collection nonExcess = new ArrayList<>(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyWithNodeGroup.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyWithNodeGroup.java index 1fb46f9a6bf..e5ffd56aefb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyWithNodeGroup.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyWithNodeGroup.java @@ -625,16 +625,21 @@ public class TestReplicationPolicyWithNodeGroup extends BaseReplicationPolicyTes public void testChooseReplicaToDelete() throws Exception { List replicaList = new ArrayList<>(); final Map> rackMap = new HashMap<>(); - dataNodes[0].setRemaining(4*1024*1024); + storages[0].setRemainingForTests(4*1024*1024); + dataNodes[0].setRemaining(calculateRemaining(dataNodes[0])); replicaList.add(storages[0]); - dataNodes[1].setRemaining(3*1024*1024); + storages[1].setRemainingForTests(3*1024*1024); + dataNodes[1].setRemaining(calculateRemaining(dataNodes[1])); replicaList.add(storages[1]); - dataNodes[2].setRemaining(2*1024*1024); + storages[2].setRemainingForTests(2*1024*1024); + dataNodes[2].setRemaining(calculateRemaining(dataNodes[2])); replicaList.add(storages[2]); - dataNodes[5].setRemaining(1*1024*1024); + storages[4].setRemainingForTests(100 * 1024 * 1024); + storages[5].setRemainingForTests(512 * 1024); + dataNodes[5].setRemaining(calculateRemaining(dataNodes[5])); replicaList.add(storages[5]); List first = new ArrayList<>(); @@ -671,7 +676,15 @@ public class TestReplicationPolicyWithNodeGroup extends BaseReplicationPolicyTes first, second, excessTypes, rackMap); assertEquals(chosen, storages[5]); } - + + private long calculateRemaining(DatanodeDescriptor dataNode) { + long sum = 0; + for (DatanodeStorageInfo storageInfo: dataNode.getStorageInfos()){ + sum += storageInfo.getRemaining(); + } + return sum; + } + /** * Test replica placement policy in case of boundary topology. * Rack 2 has only 1 node group & can't be placed with two replicas