hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From szets...@apache.org
Subject svn commit: r1401514 - in /hadoop/common/branches/branch-1-win: CHANGES.branch-1-win.txt src/hdfs/org/apache/hadoop/hdfs/server/namenode/AzureBlockPlacementPolicy.java src/test/org/apache/hadoop/hdfs/server/namenode/TestAzureBlockPlacementPolicy.java
Date Wed, 24 Oct 2012 00:01:27 GMT
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);
   }
 
   /**



Mime
View raw message