Author: szetszwo
Date: Wed Oct 24 00:01:27 2012
New Revision: 1401514
URL: http://svn.apache.org/viewvc?rev=1401514&view=rev
Log:
HDFS-4093. Fix a bug that AzureBlockPlacementPolicy#chooseTarget only returns
one DN when replication factor is greater than 3. Contributed by Jing Zhao
Modified:
hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt
hadoop/common/branches/branch-1-win/src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java
hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java
Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt?rev=1401514&r1=1401513&r2=1401514&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt (original)
+++ hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt Wed Oct 24
00:01:27 2012
@@ -184,3 +184,7 @@ Branch-hadoop-1-win - unreleased
HADOOP-8564. Port and extend Hadoop native libraries for Windows to address
datanode concurrent reading and writing issue. (Chuan Liu via suresh)
+
+ HADOOP-4093. Fix a bug that AzureBlockPlacementPolicy#chooseTarget only
+ returns one DN when replication factor is greater than 3. (Jing Zhao via
+ szetszwo)
Modified:
hadoop/common/branches/branch-1-win/src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java?rev=1401514&r1=1401513&r2=1401514&view=diff
==============================================================================
---
hadoop/common/branches/branch-1-win/src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java
(original)
+++
hadoop/common/branches/branch-1-win/src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java
Wed Oct 24 00:01:27 2012
@@ -24,7 +24,6 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
-import java.util.Random;
import java.util.Set;
import java.util.TreeSet;
@@ -46,8 +45,6 @@ import org.apache.hadoop.net.NodeBase;
*/
public class AzureBlockPlacementPolicy extends BlockPlacementPolicyDefault {
- private static final Random random = new Random();
-
private static final int DEFAULT_MIN_FAULT_DOMAINS = 2;
private static final int DEFAULT_MIN_UPGRADE_DOMAINS = 3;
@@ -69,34 +66,42 @@ public class AzureBlockPlacementPolicy e
* @param maxReplicasPerRack
* @param results
* selected nodes so far
+ * @param numOfReplicas the number of replicas required
* @throws NotEnoughReplicasException
*/
private void chooseRack(HashMap<Node, Node> excludedNodes, long blocksize,
- int maxReplicasPerRack, List<DatanodeDescriptor> results)
- throws NotEnoughReplicasException {
+ int maxReplicasPerRack, List<DatanodeDescriptor> results,
+ int numOfReplicas) throws NotEnoughReplicasException {
// if results.size is 0, this function chooses a rack randomly.
// It makes more sense to call chooseLocalNode when there are no results at
// all.
assert (results != null);
-
- ArrayList<DatanodeDescriptor> selectedNodes = selectNodes(excludedNodes,
- results);
- int numOfReplicas = 1;
-
- if (selectedNodes.size() >= numOfReplicas) {
- for (DatanodeDescriptor result : selectedNodes) {
- if (isGoodTarget(result, blocksize, maxReplicasPerRack, results)) {
- numOfReplicas--;
- results.add(result);
- break;
+ if (numOfReplicas <= 0) {
+ return;
+ }
+ while (numOfReplicas > 0) {
+ boolean placedReplica = false;
+ ArrayList<DatanodeDescriptor> selectedNodes = selectNodes(excludedNodes,
+ results);
+ if (selectedNodes.size() > 0) {
+ for (DatanodeDescriptor result : selectedNodes) {
+ if (isGoodTarget(result, blocksize, maxReplicasPerRack, results)) {
+ if (!excludedNodes.containsKey(result)) {
+ numOfReplicas--;
+ results.add(result);
+ excludedNodes.put(result, result);
+ placedReplica = true;
+ break;
+ }
+ }
}
+ }
+ if (!placedReplica) {
+ throw new NotEnoughReplicasException(
+ "Not able to place enough replicas");
}
}
-
- if (numOfReplicas > 0) {
- throw new NotEnoughReplicasException("Not able to place enough
replicas");
- }
}
/**
@@ -361,6 +366,7 @@ public class AzureBlockPlacementPolicy e
return writer;
}
+ int targetNum = numOfReplicas;
int numOfResults = results.size();
boolean newBlock = (numOfResults == 0);
if (writer == null && !newBlock) {
@@ -368,30 +374,18 @@ public class AzureBlockPlacementPolicy e
}
try {
- switch (numOfResults) {
- case 0:
+ if (numOfResults == 0) {
writer = chooseLocalNode(writer, excludedNodes, blocksize,
maxNodesPerRack, results);
if (--numOfReplicas == 0) {
- break;
- }
- case 1:
- chooseRack(excludedNodes, blocksize, maxNodesPerRack, results);
- if (--numOfReplicas == 0) {
- break;
+ return writer;
}
- case 2:
- chooseRack(excludedNodes, blocksize, maxNodesPerRack, results);
- if (--numOfReplicas == 0) {
- break;
- }
- default:
- chooseRack(excludedNodes, blocksize, maxNodesPerRack, results);
-
}
+ chooseRack(excludedNodes, blocksize, maxNodesPerRack, results,
+ numOfReplicas);
} catch (NotEnoughReplicasException e) {
LOG.warn("Not able to place enough replicas, still in need of "
- + numOfReplicas);
+ + (targetNum - results.size() + numOfResults));
}
return writer;
}
Modified:
hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java?rev=1401514&r1=1401513&r2=1401514&view=diff
==============================================================================
---
hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java
(original)
+++
hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java
Wed Oct 24 00:01:27 2012
@@ -126,6 +126,14 @@ public class TestAzureBlockPlacementPoli
assertEquals(targets[0], NODE1_FD0_UD0);
assertTrue(replicator.verifyBlockPlacement(filename,
getLocatedBlock(targets), (short) 4) == 0);
+
+ // Choose 7 targets. Should be able to return 7 nodes
+ targets = replicator.chooseTarget(filename, NUM_OF_DATANODES,
+ NODE1_FD0_UD0, BLOCK_SIZE);
+ assertEquals(targets.length, NUM_OF_DATANODES);
+ assertEquals(targets[0], NODE1_FD0_UD0);
+ assertTrue(replicator.verifyBlockPlacement(filename,
+ getLocatedBlock(targets), (short) 7) == 0);
NODE1_FD0_UD0.updateHeartbeat(2 * FSConstants.MIN_BLOCKS_FOR_WRITE
* BLOCK_SIZE, 0L, FSConstants.MIN_BLOCKS_FOR_WRITE * BLOCK_SIZE, 0);
@@ -189,6 +197,14 @@ public class TestAzureBlockPlacementPoli
assertFalse(areInvalidNodesSelected(invalidNodes, targets));
assertTrue(replicator.verifyBlockPlacement(filename,
getLocatedBlock(targets), (short) 4) == 0);
+
+ // Choose 7 targets. Should be able to return 6 nodes
+ excludedNodes = getMap(invalidNodes);
+ chosenNodes.clear();
+ targets = repl.chooseTarget(NUM_OF_DATANODES, NODE1_FD0_UD0, chosenNodes,
+ excludedNodes, BLOCK_SIZE);
+ assertEquals(targets.length, NUM_OF_DATANODES - 1);
+ assertEquals(targets[0], NODE1_FD0_UD0);
}
/**
