Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-16662

Remove unnecessary InnerNode check in NetworkTopology#add()

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.3.0, 2.8.6, 2.9.3, 3.1.4, 3.2.2, 2.10.1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      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.

        Attachments

        1. HADOOP-16662.001.patch
          1 kB
          Lisheng Sun

          Issue Links

            Activity

              People

              • Assignee:
                leosun08 Lisheng Sun
                Reporter:
                leosun08 Lisheng Sun
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: