HDFS-13736. BlockPlacementPolicyDefault can not choose favored nodes when 'dfs.namenode.block-placement-policy.default.prefer-local-node' set to false. Contributed by hu xiaodong.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java

@@ -240,9 +240,10 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy {
       DatanodeDescriptor favoredNode = favoredNodes.get(i);
       // Choose a single node which is local to favoredNode.
       // 'results' is updated within chooseLocalNode
-      final DatanodeStorageInfo target =
+      final DatanodeStorageInfo target = chooseLocalOrFavoredStorage(
-          chooseLocalStorage(favoredNode, favoriteAndExcludedNodes, blocksize,
+          favoredNode, true, favoriteAndExcludedNodes, blocksize,
-            maxNodesPerRack, results, avoidStaleNodes, storageTypes, false);
+          maxNodesPerRack, results, avoidStaleNodes, storageTypes);
+
       if (target == null) {
         LOG.warn("Could not find a target for file " + src
             + " with favored node " + favoredNode);
@@ -546,16 +547,41 @@ public class BlockPlacementPolicyDefault extends BlockPlacementPolicy {
       List<DatanodeStorageInfo> results, boolean avoidStaleNodes,
       EnumMap<StorageType, Integer> storageTypes)
       throws NotEnoughReplicasException {
+    return chooseLocalOrFavoredStorage(localMachine, false,
+        excludedNodes, blocksize, maxNodesPerRack, results, avoidStaleNodes,
+        storageTypes);
+  }
+
+  /**
+   * Choose storage of local or favored node.
+   * @param localOrFavoredNode local or favored node
+   * @param isFavoredNode if target node is favored node
+   * @param excludedNodes datanodes that should not be considered as targets
+   * @param blocksize size of the data to be written
+   * @param maxNodesPerRack max nodes allowed per rack
+   * @param results the target nodes already chosen
+   * @param avoidStaleNodes avoid stale nodes in replica choosing
+   * @param storageTypes storage type to be considered for target
+   * @return storage of local or favored node (not chosen node)
+   * @throws NotEnoughReplicasException
+   */
+  protected DatanodeStorageInfo chooseLocalOrFavoredStorage(
+      Node localOrFavoredNode, boolean isFavoredNode, Set<Node> excludedNodes,
+      long blocksize, int maxNodesPerRack, List<DatanodeStorageInfo> results,
+      boolean avoidStaleNodes, EnumMap<StorageType, Integer> storageTypes)
+      throws NotEnoughReplicasException {
     // if no local machine, randomly choose one node
-    if (localMachine == null) {
+    if (localOrFavoredNode == null) {
       return chooseRandom(NodeBase.ROOT, excludedNodes, blocksize,
           maxNodesPerRack, results, avoidStaleNodes, storageTypes);
     }
-    if (preferLocalNode && localMachine instanceof DatanodeDescriptor
+    if ((preferLocalNode || isFavoredNode)
-        && clusterMap.contains(localMachine)) {
+        && localOrFavoredNode instanceof DatanodeDescriptor
-      DatanodeDescriptor localDatanode = (DatanodeDescriptor) localMachine;
+        && clusterMap.contains(localOrFavoredNode)) {
+      DatanodeDescriptor localDatanode =
+          (DatanodeDescriptor) localOrFavoredNode;
       // otherwise try local machine first
-      if (excludedNodes.add(localMachine) // was not in the excluded list
+      if (excludedNodes.add(localOrFavoredNode) // was not in the excluded list
           && isGoodDatanode(localDatanode, maxNodesPerRack, false,
               results, avoidStaleNodes)) {
         for (Iterator<Map.Entry<StorageType, Integer>> iter = storageTypes

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java

@@ -1519,6 +1519,29 @@ public class TestReplicationPolicy extends BaseReplicationPolicyTest {
     }
   }
 
+  @Test
+  public void testChooseFromFavoredNodesWhenPreferLocalSetToFalse() {
+    ((BlockPlacementPolicyDefault) replicator).setPreferLocalNode(false);
+    try {
+      DatanodeStorageInfo[] targets;
+      List<DatanodeDescriptor> expectedTargets = new ArrayList<>();
+      expectedTargets.add(dataNodes[0]);
+      expectedTargets.add(dataNodes[2]);
+      List<DatanodeDescriptor> favouredNodes = new ArrayList<>();
+      favouredNodes.add(dataNodes[0]);
+      favouredNodes.add(dataNodes[2]);
+      targets = chooseTarget(2, dataNodes[3], null,
+          favouredNodes);
+      assertEquals(targets.length, 2);
+      for (int i = 0; i < targets.length; i++) {
+        assertTrue("Target should be a part of Expected Targets",
+            expectedTargets.contains(targets[i].getDatanodeDescriptor()));
+      }
+    } finally {
+      ((BlockPlacementPolicyDefault) replicator).setPreferLocalNode(true);
+    }
+  }
+
   private DatanodeStorageInfo[] chooseTarget(int numOfReplicas,
       DatanodeDescriptor writer, Set<Node> excludedNodes,
       List<DatanodeDescriptor> favoredNodes) {