Merge -r 675049:675050 from trunk to branch 0.17 to reflect the change made by HADOOP-3685.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/branches/branch-0.17@675058 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -9,6 +9,8 @@ Release 0.17.2 - Unreleased
 
     HADOOP-3002. Hold off block removal while in safe mode. (shv)
 
+    HADOOP-3685. Unbalanced replication target. (hairong)
+
 Release 0.17.1 - 2008-06-23
 
   INCOMPATIBLE CHANGES

src/java/org/apache/hadoop/dfs/ReplicationTargetChooser.java

@@ -140,7 +140,8 @@ class ReplicationTargetChooser {
     }
       
     int numOfResults = results.size();
-    if (writer == null && (numOfResults==1 || numOfResults==2)) {
+    boolean newBlock = (numOfResults==0);
+    if (writer == null && !newBlock) {
       writer = (DatanodeDescriptor)results.get(0);
     }
       
@@ -153,18 +154,21 @@ class ReplicationTargetChooser {
           break;
         }
       case 1:
-        chooseRemoteRack(1, writer, excludedNodes, 
+        chooseRemoteRack(1, results.get(0), excludedNodes, 
                          blocksize, maxNodesPerRack, results);
         if (--numOfReplicas == 0) {
           break;
         }
       case 2:
         if (clusterMap.isOnSameRack(results.get(0), results.get(1))) {
-          chooseRemoteRack(1, writer, excludedNodes,
+          chooseRemoteRack(1, results.get(0), excludedNodes,
                            blocksize, maxNodesPerRack, results);
-        } else {
+        } else if (newBlock){
           chooseLocalRack(results.get(1), excludedNodes, blocksize, 
                           maxNodesPerRack, results);
+        } else {
+          chooseLocalRack(writer, excludedNodes, blocksize,
+                          maxNodesPerRack, results);
         }
         if (--numOfReplicas == 0) {
           break;

src/test/org/apache/hadoop/dfs/TestReplicationPolicy.java

@@ -124,7 +124,7 @@ public class TestReplicationPolicy extends TestCase {
 
   /**
    * In this testcase, client is dataNodes[0], but the dataNodes[1] is
-   * not allowed to be choosen. So the 1st replica should be
+   * not allowed to be chosen. So the 1st replica should be
    * placed on dataNodes[0], the 2nd replica should be placed on a different
    * rack, the 3rd should be on same rack as the 2nd replica, and the rest
    * should be placed on a third rack.
@@ -180,7 +180,7 @@ public class TestReplicationPolicy extends TestCase {
 
   /**
    * In this testcase, client is dataNodes[0], but dataNodes[0] is not qualified
-   * to be choosen. So the 1st replica should be placed on dataNodes[1], 
+   * to be chosen. So the 1st replica should be placed on dataNodes[1], 
    * the 2nd replica should be placed on a different rack,
    * the 3rd replica should be placed on the same rack as the 2nd replica,
    * and the rest should be placed on the third rack.
@@ -233,7 +233,7 @@ public class TestReplicationPolicy extends TestCase {
   
   /**
    * In this testcase, client is dataNodes[0], but none of the nodes on rack 1
-   * is qualified to be choosen. So the 1st replica should be placed on either
+   * is qualified to be chosen. So the 1st replica should be placed on either
    * rack 2 or rack 3. 
    * the 2nd replica should be placed on a different rack,
    * the 3rd replica should be placed on the same rack as the 1st replica,
@@ -309,65 +309,63 @@ public class TestReplicationPolicy extends TestCase {
   }
   
   /**
-   * This testcase tests re-replication, when dataNodes[0] is already choosen.
+   * This testcase tests re-replication, when dataNodes[0] is already chosen.
    * So the 1st replica can be placed on random rack. 
    * the 2nd replica should be placed on different node by same rack as 
    * the 1st replica. The 3rd replica can be placed randomly.
    * @throws Exception
    */
   public void testRereplicate1() throws Exception {
-    List<DatanodeDescriptor> choosenNodes = new ArrayList<DatanodeDescriptor>();
-    choosenNodes.add(dataNodes[0]);    
+    List<DatanodeDescriptor> chosenNodes = new ArrayList<DatanodeDescriptor>();
+    chosenNodes.add(dataNodes[0]);    
     DatanodeDescriptor[] targets;
     
     targets = replicator.chooseTarget(
-                                      0, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      0, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 0);
     
     targets = replicator.chooseTarget(
-                                      1, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      1, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 1);
     assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
     
     targets = replicator.chooseTarget(
-                                      2, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      2, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 2);
-    assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
-    assertTrue(cluster.isOnSameRack(targets[0], targets[1]));
+    assertTrue(cluster.isOnSameRack(dataNodes[0], targets[0]));
+    assertFalse(cluster.isOnSameRack(targets[0], targets[1]));
     
     targets = replicator.chooseTarget(
-                                      3, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      3, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 3);
-    assertTrue(cluster.isOnSameRack(targets[0], targets[1]) ||
-               cluster.isOnSameRack(targets[1], targets[2]));
+    assertTrue(cluster.isOnSameRack(dataNodes[0], targets[0]));
     assertFalse(cluster.isOnSameRack(targets[0], targets[2]));
-    assertFalse(cluster.isOnSameRack(dataNodes[0], targets[1]));    
   }
 
   /**
    * This testcase tests re-replication, 
-   * when dataNodes[0] and dataNodes[1] are already choosen.
+   * when dataNodes[0] and dataNodes[1] are already chosen.
    * So the 1st replica should be placed on a different rack than rack 1. 
    * the rest replicas can be placed randomly,
    * @throws Exception
    */
   public void testRereplicate2() throws Exception {
-    List<DatanodeDescriptor> choosenNodes = new ArrayList<DatanodeDescriptor>();
-    choosenNodes.add(dataNodes[0]);
-    choosenNodes.add(dataNodes[1]);
+    List<DatanodeDescriptor> chosenNodes = new ArrayList<DatanodeDescriptor>();
+    chosenNodes.add(dataNodes[0]);
+    chosenNodes.add(dataNodes[1]);
 
     DatanodeDescriptor[] targets;
     targets = replicator.chooseTarget(
-                                      0, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      0, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 0);
     
     targets = replicator.chooseTarget(
-                                      1, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      1, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 1);
     assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
     
     targets = replicator.chooseTarget(
-                                      2, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      2, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 2);
     assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
     assertFalse(cluster.isOnSameRack(dataNodes[0], targets[1]));
@@ -375,31 +373,42 @@ public class TestReplicationPolicy extends TestCase {
 
   /**
    * This testcase tests re-replication, 
-   * when dataNodes[0] and dataNodes[2] are already choosen.
-   * So the 1st replica should be placed on rack 2. 
+   * when dataNodes[0] and dataNodes[2] are already chosen.
+   * So the 1st replica should be placed on the rack that the writer resides. 
    * the rest replicas can be placed randomly,
    * @throws Exception
    */
   public void testRereplicate3() throws Exception {
-    List<DatanodeDescriptor> choosenNodes = new ArrayList<DatanodeDescriptor>();
-    choosenNodes.add(dataNodes[0]);
-    choosenNodes.add(dataNodes[2]);
+    List<DatanodeDescriptor> chosenNodes = new ArrayList<DatanodeDescriptor>();
+    chosenNodes.add(dataNodes[0]);
+    chosenNodes.add(dataNodes[2]);
     
     DatanodeDescriptor[] targets;
     targets = replicator.chooseTarget(
-                                      0, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      0, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 0);
     
     targets = replicator.chooseTarget(
-                                      1, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                                      1, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
+    assertEquals(targets.length, 1);
+    assertTrue(cluster.isOnSameRack(dataNodes[0], targets[0]));
+    assertFalse(cluster.isOnSameRack(dataNodes[2], targets[0]));
+    
+    targets = replicator.chooseTarget(
+                               1, dataNodes[2], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 1);
-    assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
     assertTrue(cluster.isOnSameRack(dataNodes[2], targets[0]));
+    assertFalse(cluster.isOnSameRack(dataNodes[0], targets[0]));
+
+    targets = replicator.chooseTarget(
+                                      2, dataNodes[0], chosenNodes, null, BLOCK_SIZE);
+    assertEquals(targets.length, 2);
+    assertTrue(cluster.isOnSameRack(dataNodes[0], targets[0]));
     
     targets = replicator.chooseTarget(
-                                      2, dataNodes[0], choosenNodes, null, BLOCK_SIZE);
+                               2, dataNodes[2], chosenNodes, null, BLOCK_SIZE);
     assertEquals(targets.length, 2);
-    assertTrue(cluster.isOnSameRack(dataNodes[2], targets[0]) ||
-               cluster.isOnSameRack(dataNodes[2], targets[1]));
+    assertTrue(cluster.isOnSameRack(dataNodes[2], targets[0]));
   }
+  
 }