From 555e93493e9d977147bb6e793b75b52ada2f94a8 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Tue, 4 Sep 2012 23:52:59 +0000 Subject: [PATCH] svn merge -c 1380934 from trunk for HDFS-3887. Remove redundant chooseTarget methods in BlockPlacementPolicy. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1380936 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../server/blockmanagement/BlockManager.java | 5 +- .../blockmanagement/BlockPlacementPolicy.java | 62 +-------------- .../web/resources/NamenodeWebHdfsMethods.java | 6 +- .../TestReplicationPolicy.java | 76 +++++++++---------- 5 files changed, 49 insertions(+), 103 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index a8b5ed0dc3a..4d1c647ba1a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -239,6 +239,9 @@ Release 2.0.1-alpha - UNRELEASED HDFS-3871. Change NameNodeProxies to use RetryUtils. (Arun C Murthy via szetszwo) + HDFS-3887. Remove redundant chooseTarget methods in BlockPlacementPolicy. + (Jing Zhao via szetszwo) + OPTIMIZATIONS HDFS-2982. Startup performance suffers when there are many edit log 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 3a6153e372e..45612186d98 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 @@ -1316,8 +1316,9 @@ public class BlockManager { final HashMap excludedNodes, final long blocksize) throws IOException { // choose targets for the new block to be allocated. - final DatanodeDescriptor targets[] = blockplacement.chooseTarget( - src, numOfReplicas, client, excludedNodes, blocksize); + final DatanodeDescriptor targets[] = blockplacement.chooseTarget(src, + numOfReplicas, client, new ArrayList(), false, + excludedNodes, blocksize); if (targets.length < minReplication) { throw new IOException("File " + src + " could only be replicated to " + targets.length + " nodes instead of minReplication (=" diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java index e1efae54193..472a83950ff 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicy.java @@ -68,21 +68,6 @@ public abstract class BlockPlacementPolicy { List chosenNodes, long blocksize); - /** - * Same as - * {{@link #chooseTarget(String, int, DatanodeDescriptor, List, boolean, HashMap, long)} - * with returnChosenNodes equal to false. - */ - final DatanodeDescriptor[] chooseTarget(String srcPath, - int numOfReplicas, - DatanodeDescriptor writer, - List chosenNodes, - HashMap excludedNodes, - long blocksize) { - return chooseTarget(srcPath, numOfReplicas, writer, chosenNodes, false, - excludedNodes, blocksize); - } - /** * choose numOfReplicas data nodes for writer * to re-replicate a block with size blocksize @@ -129,7 +114,7 @@ public abstract class BlockPlacementPolicy { HashMap excludedNodes, long blocksize) { return chooseTarget(srcBC.getName(), numOfReplicas, writer, - chosenNodes, excludedNodes, blocksize); + chosenNodes, false, excludedNodes, blocksize); } /** @@ -197,49 +182,4 @@ public abstract class BlockPlacementPolicy { return replicator; } - /** - * choose numOfReplicas nodes for writer to replicate - * a block with size blocksize - * If not, return as many as we can. - * - * @param srcPath a string representation of the file for which chooseTarget is invoked - * @param numOfReplicas number of replicas wanted. - * @param writer the writer's machine, null if not in the cluster. - * @param blocksize size of the data to be written. - * @return array of DatanodeDescriptor instances chosen as targets - * and sorted as a pipeline. - */ - DatanodeDescriptor[] chooseTarget(String srcPath, - int numOfReplicas, - DatanodeDescriptor writer, - long blocksize) { - return chooseTarget(srcPath, numOfReplicas, writer, - new ArrayList(), - blocksize); - } - - /** - * choose numOfReplicas nodes for writer to replicate - * a block with size blocksize - * If not, return as many as we can. - * - * @param srcPath a string representation of the file for which chooseTarget is invoked - * @param numOfReplicas number of replicas wanted. - * @param writer the writer's machine, null if not in the cluster. - * @param blocksize size of the data to be written. - * @param excludedNodes datanodes that should not be considered as targets. - * @return array of DatanodeDescriptor instances chosen as targets - * and sorted as a pipeline. - */ - public DatanodeDescriptor[] chooseTarget(String srcPath, - int numOfReplicas, - DatanodeDescriptor writer, - HashMap excludedNodes, - long blocksize) { - return chooseTarget(srcPath, numOfReplicas, writer, - new ArrayList(), - excludedNodes, - blocksize); - } - } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java index 0ee13519f6f..912dee10370 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java @@ -25,6 +25,7 @@ import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; import java.security.PrivilegedExceptionAction; +import java.util.ArrayList; import java.util.EnumSet; import javax.servlet.ServletContext; @@ -163,8 +164,9 @@ public class NamenodeWebHdfsMethods { final DatanodeDescriptor clientNode = bm.getDatanodeManager( ).getDatanodeByHost(getRemoteAddress()); if (clientNode != null) { - final DatanodeDescriptor[] datanodes = bm.getBlockPlacementPolicy( - ).chooseTarget(path, 1, clientNode, null, blocksize); + final DatanodeDescriptor[] datanodes = bm.getBlockPlacementPolicy() + .chooseTarget(path, 1, clientNode, + new ArrayList(), false, null, blocksize); if (datanodes.length > 0) { return datanodes[0]; } 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 a10a0a3cf63..23d3f5079db 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 @@ -110,30 +110,30 @@ public class TestReplicationPolicy { HdfsConstants.MIN_BLOCKS_FOR_WRITE*BLOCK_SIZE, 0L, 4, 0); // overloaded DatanodeDescriptor[] targets; - targets = replicator.chooseTarget(filename, - 0, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 0, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 0); - targets = replicator.chooseTarget(filename, - 1, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 1, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 1); assertEquals(targets[0], dataNodes[0]); targets = replicator.chooseTarget(filename, - 2, dataNodes[0], BLOCK_SIZE); + 2, dataNodes[0], new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 2); assertEquals(targets[0], dataNodes[0]); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); - targets = replicator.chooseTarget(filename, - 3, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 3, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 3); assertEquals(targets[0], dataNodes[0]); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); assertTrue(cluster.isOnSameRack(targets[1], targets[2])); - targets = replicator.chooseTarget(filename, - 4, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 4, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 4); assertEquals(targets[0], dataNodes[0]); assertTrue(cluster.isOnSameRack(targets[1], targets[2]) || @@ -248,30 +248,30 @@ public class TestReplicationPolicy { (HdfsConstants.MIN_BLOCKS_FOR_WRITE-1)*BLOCK_SIZE, 0L, 0, 0); // no space DatanodeDescriptor[] targets; - targets = replicator.chooseTarget(filename, - 0, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 0, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 0); - targets = replicator.chooseTarget(filename, - 1, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 1, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 1); assertEquals(targets[0], dataNodes[1]); - targets = replicator.chooseTarget(filename, - 2, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 2, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 2); assertEquals(targets[0], dataNodes[1]); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); - targets = replicator.chooseTarget(filename, - 3, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 3, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 3); assertEquals(targets[0], dataNodes[1]); assertTrue(cluster.isOnSameRack(targets[1], targets[2])); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); - targets = replicator.chooseTarget(filename, - 4, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 4, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 4); assertEquals(targets[0], dataNodes[1]); for(int i=1; i<4; i++) { @@ -304,23 +304,23 @@ public class TestReplicationPolicy { } DatanodeDescriptor[] targets; - targets = replicator.chooseTarget(filename, - 0, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 0, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 0); - targets = replicator.chooseTarget(filename, - 1, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 1, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 1); assertFalse(cluster.isOnSameRack(targets[0], dataNodes[0])); - targets = replicator.chooseTarget(filename, - 2, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 2, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 2); assertFalse(cluster.isOnSameRack(targets[0], dataNodes[0])); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); - targets = replicator.chooseTarget(filename, - 3, dataNodes[0], BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 3, dataNodes[0], + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 3); for(int i=0; i<3; i++) { assertFalse(cluster.isOnSameRack(targets[i], dataNodes[0])); @@ -349,21 +349,21 @@ public class TestReplicationPolicy { DFSTestUtil.getDatanodeDescriptor("7.7.7.7", "/d2/r4"); DatanodeDescriptor[] targets; - targets = replicator.chooseTarget(filename, - 0, writerDesc, BLOCK_SIZE); + targets = replicator.chooseTarget(filename, 0, writerDesc, + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 0); - - targets = replicator.chooseTarget(filename, - 1, writerDesc, BLOCK_SIZE); + + targets = replicator.chooseTarget(filename, 1, writerDesc, + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 1); - - targets = replicator.chooseTarget(filename, - 2, writerDesc, BLOCK_SIZE); + + targets = replicator.chooseTarget(filename, 2, writerDesc, + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 2); assertFalse(cluster.isOnSameRack(targets[0], targets[1])); - - targets = replicator.chooseTarget(filename, - 3, writerDesc, BLOCK_SIZE); + + targets = replicator.chooseTarget(filename, 3, writerDesc, + new ArrayList(), BLOCK_SIZE); assertEquals(targets.length, 3); assertTrue(cluster.isOnSameRack(targets[1], targets[2])); assertFalse(cluster.isOnSameRack(targets[0], targets[1]));