diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 9bc7974431c..2234da306c5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -172,6 +172,9 @@ Release 2.6.1 - UNRELEASED HDFS-7470. SecondaryNameNode need twice memory when calling reloadFromImageFile. (zhaoyunjiong via cnauroth) + HDFS-8863. The remaining space check in BlockPlacementPolicyDefault is + flawed. (Kihwal Lee via yliu) + Release 2.6.0 - 2014-11-18 INCOMPATIBLE CHANGES 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 5b02384737b..19cb70156a2 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 @@ -780,7 +780,8 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy { final long requiredSize = blockSize * HdfsConstants.MIN_BLOCKS_FOR_WRITE; final long scheduledSize = blockSize * node.getBlocksScheduled(storage.getStorageType()); - final long remaining = node.getRemaining(storage.getStorageType()); + final long remaining = + node.getRemaining(storage.getStorageType(), requiredSize); if (requiredSize > remaining - scheduledSize) { logNodeIsNotChosen(storage, "the node does not have enough " + storage.getStorageType() + " space" diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java index 1bef8050d67..da85a1c6ea0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hdfs.protocol.DatanodeInfo; import org.apache.hadoop.hdfs.server.namenode.CachedBlock; import org.apache.hadoop.hdfs.server.protocol.BlockReportContext; import org.apache.hadoop.hdfs.server.protocol.DatanodeStorage; +import org.apache.hadoop.hdfs.server.protocol.DatanodeStorage.State; import org.apache.hadoop.hdfs.server.protocol.StorageReport; import org.apache.hadoop.hdfs.util.EnumCounters; import org.apache.hadoop.hdfs.util.LightWeightHashSet; @@ -642,16 +643,26 @@ public class DatanodeDescriptor extends DatanodeInfo { } /** - * @return Approximate number of blocks currently scheduled to be written + * Return the sum of remaining spaces of the specified type. If the remaining + * space of a storage is less than minSize, it won't be counted toward the + * sum. + * + * @param t The storage type. If null, the type is ignored. + * @param minSize The minimum free space required. + * @return the sum of remaining spaces that are bigger than minSize. */ - public long getRemaining(StorageType t) { + public long getRemaining(StorageType t, long minSize) { long remaining = 0; - for(DatanodeStorageInfo s : getStorageInfos()) { - if (s.getStorageType() == t) { - remaining += s.getRemaining(); + for (DatanodeStorageInfo s : getStorageInfos()) { + if (s.getState() == State.NORMAL && + (t == null || s.getStorageType() == t)) { + long r = s.getRemaining(); + if (r >= minSize) { + remaining += r; + } } } - return remaining; + return remaining; } /** 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 1e514af7e16..c85ecb336aa 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 @@ -101,6 +101,16 @@ public class TestReplicationPolicy { dnCacheCapacity, dnCacheUsed, xceiverCount, volFailures); } + private static void updateHeartbeatForExtraStorage(long capacity, + long dfsUsed, long remaining, long blockPoolUsed) { + DatanodeDescriptor dn = dataNodes[5]; + dn.getStorageInfos()[1].setUtilizationForTesting( + capacity, dfsUsed, remaining, blockPoolUsed); + dn.updateHeartbeat( + BlockManagerTestUtil.getStorageReportsForDatanode(dn), + 0L, 0L, 0, 0); + } + @BeforeClass public static void setupCluster() throws Exception { Configuration conf = new HdfsConfiguration(); @@ -114,6 +124,16 @@ public class TestReplicationPolicy { storages = DFSTestUtil.createDatanodeStorageInfos(racks); dataNodes = DFSTestUtil.toDatanodeDescriptor(storages); + // create an extra storage for dn5. + DatanodeStorage extraStorage = new DatanodeStorage( + storages[5].getStorageID() + "-extra", DatanodeStorage.State.NORMAL, + StorageType.DEFAULT); +/* DatanodeStorageInfo si = new DatanodeStorageInfo( + storages[5].getDatanodeDescriptor(), extraStorage); +*/ + BlockManagerTestUtil.updateStorage(storages[5].getDatanodeDescriptor(), + extraStorage); + FileSystem.setDefaultUri(conf, "hdfs://localhost:0"); conf.set(DFSConfigKeys.DFS_NAMENODE_HTTP_ADDRESS_KEY, "0.0.0.0:0"); File baseDir = PathUtils.getTestDir(TestReplicationPolicy.class); @@ -136,11 +156,17 @@ public class TestReplicationPolicy { bm.getDatanodeManager().getHeartbeatManager().addDatanode( dataNodes[i]); } + resetHeartbeatForStorages(); + } + + private static void resetHeartbeatForStorages() { for (int i=0; i < NUM_OF_DATANODES; i++) { updateHeartbeatWithUsage(dataNodes[i], 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, 0); } + // No available space in the extra storage of dn0 + updateHeartbeatForExtraStorage(0L, 0L, 0L, 0L); } private static boolean isOnSameRack(DatanodeStorageInfo left, DatanodeStorageInfo right) { @@ -150,6 +176,31 @@ public class TestReplicationPolicy { private static boolean isOnSameRack(DatanodeStorageInfo left, DatanodeDescriptor right) { return cluster.isOnSameRack(left.getDatanodeDescriptor(), right); } + + /** + * Test whether the remaining space per storage is individually + * considered. + */ + @Test + public void testChooseNodeWithMultipleStorages() throws Exception { + updateHeartbeatWithUsage(dataNodes[5], + 2* HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, + (2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE)/3, 0L, + 0L, 0L, 0, 0); + + updateHeartbeatForExtraStorage( + 2* HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, + (2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE)/3, 0L); + + DatanodeStorageInfo[] targets; + targets = chooseTarget (1, dataNodes[5], + new ArrayList(), null); + assertEquals(1, targets.length); + assertEquals(storages[4], targets[0]); + + resetHeartbeatForStorages(); + } + /** * In this testcase, client is dataNodes[0]. So the 1st replica should be * placed on dataNodes[0], the 2nd replica should be placed on @@ -191,10 +242,8 @@ public class TestReplicationPolicy { assertTrue(isOnSameRack(targets[1], targets[2]) || isOnSameRack(targets[2], targets[3])); assertFalse(isOnSameRack(targets[0], targets[2])); - - updateHeartbeatWithUsage(dataNodes[0], - 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, - HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, 0); + + resetHeartbeatForStorages(); } private static DatanodeStorageInfo[] chooseTarget(int numOfReplicas) { @@ -349,9 +398,7 @@ public class TestReplicationPolicy { isOnSameRack(targets[2], targets[3])); assertFalse(isOnSameRack(targets[1], targets[3])); - updateHeartbeatWithUsage(dataNodes[0], - 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, - HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, 0); + resetHeartbeatForStorages(); } /** @@ -392,12 +439,8 @@ public class TestReplicationPolicy { assertTrue(isOnSameRack(targets[0], targets[1]) || isOnSameRack(targets[1], targets[2])); assertFalse(isOnSameRack(targets[0], targets[2])); - - for(int i=0; i<2; i++) { - updateHeartbeatWithUsage(dataNodes[i], - 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, - HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, 0); - } + + resetHeartbeatForStorages(); } /** @@ -475,6 +518,7 @@ public class TestReplicationPolicy { } finally { bm.getDatanodeManager().getNetworkTopology().remove(newDn); } + resetHeartbeatForStorages(); } @@ -528,12 +572,8 @@ public class TestReplicationPolicy { // Suppose to place replicas on each node but two data nodes are not // available for placing replica, so here we expect a short of 2 assertTrue(((String)lastLogEntry.getMessage()).contains("in need of 2")); - - for(int i=0; i<2; i++) { - updateHeartbeatWithUsage(dataNodes[i], - 2*HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, - HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 0L, 0L, 0, 0); - } + + resetHeartbeatForStorages(); } private boolean containsWithinRange(DatanodeStorageInfo target,