[HADOOP-16662] Remove unnecessary InnerNode check in NetworkTopology#add()

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 3.3.0, 2.8.6, 2.9.3, 3.1.4, 3.2.2, 2.10.1
• Hadoop Flags: Reviewed
• Assignee: Lisheng Sun
• Reporter: Lisheng Sun
• Created: 19/Oct/19 02:39
• Updated: 09/Dec/19 20:10
• Resolved: 19/Oct/19 05:39

Description

The method of NetworkTopology#add() as follow:

/** Add a leaf node
 * Update node counter & rack counter if necessary
 * @param node node to be added; can be null
 * @exception IllegalArgumentException if add a node to a leave 
                                       or node to be added is not a leaf
 */
public void add(Node node) {
  if (node==null) return;
  int newDepth = NodeBase.locationToDepth(node.getNetworkLocation()) + 1;
  netlock.writeLock().lock();
  try {
    if( node instanceof InnerNode ) {
      throw new IllegalArgumentException(
        "Not allow to add an inner node: "+NodeBase.getPath(node));
    }
    if ((depthOfAllLeaves != -1) && (depthOfAllLeaves != newDepth)) {
      LOG.error("Error: can't add leaf node {} at depth {} to topology:{}\n",
          NodeBase.getPath(node), newDepth, this);
      throw new InvalidTopologyException("Failed to add " + NodeBase.getPath(node) +
          ": You cannot have a rack and a non-rack node at the same " +
          "level of the network topology.");
    }
    Node rack = getNodeForNetworkLocation(node);
    if (rack != null && !(rack instanceof InnerNode)) {
      throw new IllegalArgumentException("Unexpected data node " 
                                         + node.toString() 
                                         + " at an illegal network location");
    }
    if (clusterMap.add(node)) {
      LOG.info("Adding a new node: "+NodeBase.getPath(node));
      if (rack == null) {
        incrementRacks();
      }
      if (!(node instanceof InnerNode)) {
        if (depthOfAllLeaves == -1) {
          depthOfAllLeaves = node.getLevel();
        }
      }
    }
    LOG.debug("NetworkTopology became:\n{}", this);
  } finally {
    netlock.writeLock().unlock();
  }
}

if( node instanceof InnerNode ) {
  throw new IllegalArgumentException(
    "Not allow to add an inner node: "+NodeBase.getPath(node));
}

if (!(node instanceof InnerNode)) is invalid，since there is already a judgement before as follow:

if (!(node instanceof InnerNode)) {
  if (depthOfAllLeaves == -1) {
    depthOfAllLeaves = node.getLevel();
  }
}

so i think if (!(node instanceof InnerNode)) should be removed.

Issue Links

• relates to: HADOOP-8159 NetworkTopology: getLeaf should check for invalid topologies