HDFS-6920. Archival Storage: check the storage type of delNodeHintStorage when deleting a replica. Contributed by Tsz Wo Nicholas Sze.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/b7ded466 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/b7ded466 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/b7ded466 Branch: refs/heads/HDFS-6581 Commit: b7ded466b00db0fe273058b844d56d810e0f8cc2 Parents: 8ea20b5 Author: Jing Zhao <ji...@apache.org> Authored: Wed Aug 27 14:08:02 2014 -0700 Committer: Jing Zhao <ji...@apache.org> Committed: Wed Aug 27 14:08:02 2014 -0700 ---------------------------------------------------------------------- .../server/blockmanagement/BlockManager.java | 28 ++++++++++++++++---- .../BlockPlacementPolicyDefault.java | 11 ++++++-- .../blockmanagement/TestBlockManager.java | 18 ++++++++++++- .../blockmanagement/TestReplicationPolicy.java | 9 ++++++- 4 files changed, 57 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/b7ded466/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 52cd41c..389409b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -2771,12 +2771,9 @@ public class BlockManager { final DatanodeStorageInfo addedNodeStorage = DatanodeStorageInfo.getDatanodeStorageInfo(nonExcess, addedNode); while (nonExcess.size() - replication > 0) { - // check if we can delete delNodeHint final DatanodeStorageInfo cur; - if (firstOne && delNodeHintStorage != null - && (moreThanOne.contains(delNodeHintStorage) - || (addedNodeStorage != null - && !moreThanOne.contains(addedNodeStorage)))) { + if (useDelHint(firstOne, delNodeHintStorage, addedNodeStorage, + moreThanOne, excessTypes)) { cur = delNodeHintStorage; } else { // regular excessive replica removal cur = replicator.chooseReplicaToDelete(bc, b, replication, @@ -2806,6 +2803,27 @@ public class BlockManager { } } + /** Check if we can use delHint */ + static boolean useDelHint(boolean isFirst, DatanodeStorageInfo delHint, + DatanodeStorageInfo added, List<DatanodeStorageInfo> moreThan1Racks, + List<StorageType> excessTypes) { + if (!isFirst) { + return false; // only consider delHint for the first case + } else if (delHint == null) { + return false; // no delHint + } else if (!excessTypes.remove(delHint.getStorageType())) { + return false; // delHint storage type is not an excess type + } else { + // check if removing delHint reduces the number of racks + if (moreThan1Racks.contains(delHint)) { + return true; // delHint and some other nodes are under the same rack + } else if (added != null && !moreThan1Racks.contains(added)) { + return true; // the added node adds a new rack + } + return false; // removing delHint reduces the number of racks; + } + } + private void addToExcessReplicate(DatanodeInfo dn, Block block) { assert namesystem.hasWriteLock(); LightWeightLinkedSet<Block> excessBlocks = excessReplicateMap.get(dn.getDatanodeUuid()); http://git-wip-us.apache.org/repos/asf/hadoop/blob/b7ded466/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java ---------------------------------------------------------------------- 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 392e350..a711a09 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 @@ -792,8 +792,15 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy { minSpaceStorage = storage; } } - final DatanodeStorageInfo storage = oldestHeartbeatStorage != null? - oldestHeartbeatStorage : minSpaceStorage; + + final DatanodeStorageInfo storage; + if (oldestHeartbeatStorage != null) { + storage = oldestHeartbeatStorage; + } else if (minSpaceStorage != null) { + storage = minSpaceStorage; + } else { + return null; + } excessTypes.remove(storage.getStorageType()); return storage; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/b7ded466/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java index 41af237..0e13e83 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java @@ -38,6 +38,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.HdfsConfiguration; +import org.apache.hadoop.hdfs.StorageType; import org.apache.hadoop.hdfs.protocol.Block; import org.apache.hadoop.hdfs.protocol.BlockListAsLongs; import org.apache.hadoop.hdfs.protocol.HdfsConstants; @@ -46,6 +47,7 @@ import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.protocol.DatanodeRegistration; import org.apache.hadoop.hdfs.server.protocol.DatanodeStorage; import org.apache.hadoop.net.NetworkTopology; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -599,5 +601,19 @@ public class TestBlockManager { new BlockListAsLongs(null, null)); assertEquals(1, ds.getBlockReportCount()); } -} + @Test + public void testUseDelHint() { + DatanodeStorageInfo delHint = new DatanodeStorageInfo( + DFSTestUtil.getLocalDatanodeDescriptor(), new DatanodeStorage("id")); + List<DatanodeStorageInfo> moreThan1Racks = Arrays.asList(delHint); + List<StorageType> excessTypes = new ArrayList<StorageType>(); + + excessTypes.add(StorageType.DEFAULT); + Assert.assertTrue(BlockManager.useDelHint(true, delHint, null, + moreThan1Racks, excessTypes)); + excessTypes.add(StorageType.SSD); + Assert.assertFalse(BlockManager.useDelHint(true, delHint, null, + moreThan1Racks, excessTypes)); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hadoop/blob/b7ded466/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java ---------------------------------------------------------------------- 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 892f849..b8f358f 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 @@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -935,6 +936,12 @@ public class TestReplicationPolicy { assertEquals(2, first.size()); assertEquals(2, second.size()); List<StorageType> excessTypes = new ArrayList<StorageType>(); + { + // test returning null + excessTypes.add(StorageType.SSD); + assertNull(replicator.chooseReplicaToDelete( + null, null, (short)3, first, second, excessTypes)); + } excessTypes.add(StorageType.DEFAULT); DatanodeStorageInfo chosen = replicator.chooseReplicaToDelete( null, null, (short)3, first, second, excessTypes); @@ -950,7 +957,7 @@ public class TestReplicationPolicy { null, null, (short)2, first, second, excessTypes); assertEquals(chosen, storages[5]); } - + /** * This testcase tests whether the default value returned by * DFSUtil.getInvalidateWorkPctPerIteration() is positive,
