HDDS-1879. Support multiple excluded scopes when choosing datanodes in NetworkTopology (#1194)

Author: ChenSammi

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hdds.scm.net;
 
 import java.util.Collection;
+import java.util.List;
 
 /**
  * The interface defines an inner node in a network topology.
@@ -72,13 +73,13 @@ N newInnerNode(String name, String location, InnerNode parent, int level,
    *
    * @param leafIndex ode's index, start from 0, skip the nodes in
    *                  excludedScope and excludedNodes with ancestorGen
-   * @param excludedScope the excluded scope
+   * @param excludedScopes the excluded scopes
    * @param excludedNodes nodes to be excluded. If ancestorGen is not 0,
    *                      the chosen node will not share same ancestor with
    *                      those in excluded nodes at the specified generation
    * @param ancestorGen ignored with value is 0
    * @return the leaf node corresponding to the given index
    */
-  Node getLeaf(int leafIndex, String excludedScope,
+  Node getLeaf(int leafIndex, List<String> excludedScopes,
       Collection<Node> excludedNodes, int ancestorGen);
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java

@@ -22,6 +22,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
 import com.google.common.base.Preconditions;
@@ -276,7 +277,7 @@ public Node getLeaf(int leafIndex) {
    *
    * @param leafIndex node's index, start from 0, skip the nodes in
    *                 excludedScope and excludedNodes with ancestorGen
-   * @param excludedScope the exclude scope
+   * @param excludedScopes the exclude scopes
    * @param excludedNodes nodes to be excluded from. If ancestorGen is not 0,
    *                      the chosen node will not share same ancestor with
    *                      those in excluded nodes at the specified generation
@@ -300,7 +301,7 @@ public Node getLeaf(int leafIndex) {
    *
    *   Input:
    *   leafIndex = 2
-   *   excludedScope = /dc2
+   *   excludedScope = /dc2/rack2
    *   excludedNodes = {/dc1/rack1/n1}
    *   ancestorGen = 1
    *
@@ -313,12 +314,12 @@ public Node getLeaf(int leafIndex) {
    *   means picking the 3th available node, which is n5.
    *
    */
-  public Node getLeaf(int leafIndex, String excludedScope,
+  public Node getLeaf(int leafIndex, List<String> excludedScopes,
       Collection<Node> excludedNodes, int ancestorGen) {
     Preconditions.checkArgument(leafIndex >= 0 && ancestorGen >= 0);
     // come to leaf parent layer
     if (isLeafParent()) {
-      return getLeafOnLeafParent(leafIndex, excludedScope, excludedNodes);
+      return getLeafOnLeafParent(leafIndex, excludedScopes, excludedNodes);
     }
 
     int maxLevel = NodeSchemaManager.getInstance().getMaxLevel();
@@ -328,22 +329,24 @@ public Node getLeaf(int leafIndex, String excludedScope,
     Map<Node, Integer> countMap =
         getAncestorCountMap(excludedNodes, ancestorGen, currentGen);
     // nodes covered by excluded scope
-    int excludedNodeCount = getExcludedScopeNodeCount(excludedScope);
+    Map<String, Integer> excludedNodeCount =
+        getExcludedScopeNodeCount(excludedScopes);
 
-    for(Node child : childrenMap.values()) {
+    for (Node child : childrenMap.values()) {
       int leafCount = child.getNumOfLeaves();
-      // skip nodes covered by excluded scope
-      if (excludedScope != null &&
-          excludedScope.startsWith(child.getNetworkFullPath())) {
-        leafCount -= excludedNodeCount;
+      // skip nodes covered by excluded scopes
+      for (Map.Entry<String, Integer> entry: excludedNodeCount.entrySet()) {
+        if (entry.getKey().startsWith(child.getNetworkFullPath())) {
+          leafCount -= entry.getValue();
+        }
       }
       // skip nodes covered by excluded nodes and ancestorGen
       Integer count = countMap.get(child);
       if (count != null) {
         leafCount -= count;
       }
       if (leafIndex < leafCount) {
-        return ((InnerNode)child).getLeaf(leafIndex, excludedScope,
+        return ((InnerNode)child).getLeaf(leafIndex, excludedScopes,
             excludedNodes, ancestorGen);
       } else {
         leafIndex -= leafCount;
@@ -424,18 +427,22 @@ private Map<Node, Integer> getAncestorCountMap(Collection<Node> nodes,
    *  Get the node with leafIndex, considering skip nodes in excludedScope
    *  and in excludeNodes list.
    */
-  private Node getLeafOnLeafParent(int leafIndex, String excludedScope,
+  private Node getLeafOnLeafParent(int leafIndex, List<String> excludedScopes,
       Collection<Node> excludedNodes) {
     Preconditions.checkArgument(isLeafParent() && leafIndex >= 0);
     if (leafIndex >= getNumOfChildren()) {
       return null;
     }
     for(Node node : childrenMap.values()) {
-      if ((excludedNodes != null && (excludedNodes.contains(node))) ||
-          (excludedScope != null &&
-              (node.getNetworkFullPath().startsWith(excludedScope)))) {
+      if (excludedNodes != null && excludedNodes.contains(node)) {
         continue;
       }
+      if (excludedScopes != null && excludedScopes.size() > 0) {
+        if (excludedScopes.stream().anyMatch(scope ->
+            node.getNetworkFullPath().startsWith(scope))) {
+          continue;
+        }
+      }
       if (leafIndex == 0) {
         return node;
       }
@@ -484,12 +491,19 @@ private Node getChildNode(int index) {
     return node;
   }
 
-  /** Get how many leaf nodes are covered by the excludedScope. */
-  private int getExcludedScopeNodeCount(String excludedScope) {
-    if (excludedScope == null) {
-      return 0;
+  /** Get how many leaf nodes are covered by the excludedScopes(no overlap). */
+  private Map<String, Integer> getExcludedScopeNodeCount(
+      List<String> excludedScopes) {
+    HashMap<String, Integer> nodeCounts = new HashMap<>();
+    if (excludedScopes == null || excludedScopes.isEmpty()) {
+      return nodeCounts;
+    }
+
+    for (String scope: excludedScopes) {
+      Node excludedScopeNode = getNode(scope);
+      nodeCounts.put(scope, excludedScopeNode == null ? 0 :
+          excludedScopeNode.getNumOfLeaves());
     }
-    Node excludedScopeNode = getNode(excludedScope);
-    return excludedScopeNode == null ? 0 : excludedScopeNode.getNumOfLeaves();
+    return nodeCounts;
   }
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetUtils.java

@@ -17,13 +17,15 @@
  */
 package org.apache.hadoop.hdds.scm.net;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * Utility class to facilitate network topology functions.
@@ -71,35 +73,38 @@ public static int locationToDepth(String location) {
    *  Remove node from mutableExcludedNodes if it's covered by excludedScope.
    *  Please noted that mutableExcludedNodes content might be changed after the
    *  function call.
-   * @return the new excludedScope
    */
-  public static String removeDuplicate(NetworkTopology topology,
-      Collection<Node> mutableExcludedNodes, String excludedScope,
+  public static void removeDuplicate(NetworkTopology topology,
+      Collection<Node> mutableExcludedNodes, List<String> mutableExcludedScopes,
       int ancestorGen) {
-    if (mutableExcludedNodes == null || mutableExcludedNodes.size() == 0 ||
-        excludedScope == null || topology == null) {
-      return excludedScope;
+    if (CollectionUtils.isEmpty(mutableExcludedNodes) ||
+        CollectionUtils.isEmpty(mutableExcludedScopes) || topology == null) {
+      return;
     }
 
     Iterator<Node> iterator = mutableExcludedNodes.iterator();
-    while (iterator.hasNext()) {
+    while (iterator.hasNext() && (!mutableExcludedScopes.isEmpty())) {
       Node node = iterator.next();
       Node ancestor = topology.getAncestor(node, ancestorGen);
       if (ancestor == null) {
         LOG.warn("Fail to get ancestor generation " + ancestorGen +
             " of node :" + node);
         continue;
       }
-      if (excludedScope.startsWith(ancestor.getNetworkFullPath())) {
-        // reset excludedScope if it's covered by exclude node's ancestor
-        return null;
-      }
-      if (ancestor.getNetworkFullPath().startsWith(excludedScope)) {
-        // remove exclude node if it's covered by excludedScope
-        iterator.remove();
-      }
+      // excludedScope is child of ancestor
+      List<String> duplicateList = mutableExcludedScopes.stream()
+          .filter(scope -> scope.startsWith(ancestor.getNetworkFullPath()))
+          .collect(Collectors.toList());
+      mutableExcludedScopes.removeAll(duplicateList);
+
+      // ancestor is covered by excludedScope
+      mutableExcludedScopes.stream().forEach(scope -> {
+        if (ancestor.getNetworkFullPath().startsWith(scope)) {
+          // remove exclude node if it's covered by excludedScope
+          iterator.remove();
+        }
+      });
     }
-    return excludedScope;
   }
 
   /**
@@ -109,7 +114,7 @@ public static String removeDuplicate(NetworkTopology topology,
    */
   public static void removeOutscope(Collection<Node> mutableExcludedNodes,
       String scope) {
-    if (mutableExcludedNodes == null || scope == null) {
+    if (CollectionUtils.isEmpty(mutableExcludedNodes) || scope == null) {
       return;
     }
     synchronized (mutableExcludedNodes) {
@@ -134,7 +139,7 @@ public static void removeOutscope(Collection<Node> mutableExcludedNodes,
   public static List<Node> getAncestorList(NetworkTopology topology,
       Collection<Node> nodes, int generation) {
     List<Node> ancestorList = new ArrayList<>();
-    if (topology == null ||nodes == null || nodes.size() == 0 ||
+    if (topology == null || CollectionUtils.isEmpty(nodes) ||
         generation == 0) {
       return ancestorList;
     }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java

@@ -39,15 +39,13 @@ public InvalidTopologyException(String msg) {
    */
   void add(Node node);
 
-
   /**
    * Remove a node from the network topology. This will be called when a
    * existing datanode is removed from the system.
    * @param node node to be removed; cannot be null
    */
   void remove(Node node);
 
-
   /**
    * Check if the tree already contains node <i>node</i>.
    * @param node a node
@@ -68,7 +66,6 @@ public InvalidTopologyException(String msg) {
    */
   boolean isSameAncestor(Node node1, Node node2, int ancestorGen);
 
-
   /**
    * Get the ancestor for node on generation <i>ancestorGen</i>.
    *
@@ -119,11 +116,11 @@ public InvalidTopologyException(String msg) {
    * Randomly choose a node in the scope, ano not in the exclude scope.
    * @param scope range of nodes from which a node will be chosen. cannot start
    *              with ~
-   * @param excludedScope the chosen node cannot be in this range. cannot
+   * @param excludedScopes the chosen nodes cannot be in these ranges. cannot
    *                      starts with ~
    * @return the chosen node
    */
-  Node chooseRandom(String scope, String excludedScope);
+  Node chooseRandom(String scope, List<String>  excludedScopes);
 
   /**
    * Randomly choose a leaf node from <i>scope</i>.
@@ -160,34 +157,14 @@ public InvalidTopologyException(String msg) {
   Node chooseRandom(String scope, Collection<Node> excludedNodes,
       int ancestorGen);
 
-
-  /**
-   * Randomly choose a leaf node.
-   *
-   * @param scope range from which a node will be chosen, cannot start with ~
-   * @param excludedNodes nodes to be excluded
-   * @param excludedScope excluded node range. Cannot start with ~
-   * @param ancestorGen matters when excludeNodes is not null. It means the
-   * ancestor generation that's not allowed to share between chosen node and the
-   * excludedNodes. For example, if ancestorGen is 1, means chosen node
-   * cannot share the same parent with excludeNodes. If value is 2, cannot
-   * share the same grand parent, and so on. If ancestorGen is 0, then no
-   * effect.
-   *
-   * @return the chosen node
-   */
-  Node chooseRandom(String scope, String excludedScope,
-      Collection<Node> excludedNodes, int ancestorGen);
-
-
   /**
    * Randomly choose one node from <i>scope</i>, share the same generation
    * ancestor with <i>affinityNode</i>, and exclude nodes in
    * <i>excludeScope</i> and <i>excludeNodes</i>.
    *
    * @param scope range of nodes from which a node will be chosen, cannot start
    *              with ~
-   * @param excludedScope range of nodes to be excluded, cannot start with ~
+   * @param excludedScopes ranges of nodes to be excluded, cannot start with ~
    * @param excludedNodes nodes to be excluded
    * @param affinityNode  when not null, the chosen node should share the same
    *                     ancestor with this node at generation ancestorGen.
@@ -198,7 +175,7 @@ Node chooseRandom(String scope, String excludedScope,
    *                     excludedNodes if affinityNode is null
    * @return the chosen node
    */
-  Node chooseRandom(String scope, String excludedScope,
+  Node chooseRandom(String scope, List<String>  excludedScopes,
       Collection<Node> excludedNodes, Node affinityNode, int ancestorGen);
 
   /**
@@ -210,7 +187,7 @@ Node chooseRandom(String scope, String excludedScope,
    *                  excludedNodes
    * @param scope range of nodes from which a node will be chosen, cannot start
    *              with ~
-   * @param excludedScope range of nodes to be excluded, cannot start with ~
+   * @param excludedScopes ranges of nodes to be excluded, cannot start with ~
    * @param excludedNodes nodes to be excluded
    * @param affinityNode  when not null, the chosen node should share the same
    *                     ancestor with this node at generation ancestorGen.
@@ -221,7 +198,7 @@ Node chooseRandom(String scope, String excludedScope,
    *                     excludedNodes if affinityNode is null
    * @return the chosen node
    */
-  Node getNode(int leafIndex, String scope, String excludedScope,
+  Node getNode(int leafIndex, String scope, List<String> excludedScopes,
       Collection<Node> excludedNodes, Node affinityNode, int ancestorGen);
 
   /** Return the distance cost between two nodes