[jira] [Commented] (HADOOP-8159) NetworkTopology: getLeaf should check for invalid topologies

["Lisheng Sun (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-8159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16954214#comment-16954214 ] 

Lisheng Sun commented on HADOOP-8159:
-------------------------------------

hi [~cmccabe] [~weichiu] [~elgoiri] [~ayushtkn]

the method of NetworkTopology#add

 
{code:java}
/** 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 (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)) {
  if (depthOfAllLeaves == -1) {
    depthOfAllLeaves = node.getLevel();
  }
}

if (!(node instanceof InnerNode)) is invalid，since there is already a judgement before as follow:
 if( node instanceof InnerNode ) {
  throw new IllegalArgumentException(
    "Not allow to add an inner node: "+NodeBase.getPath(node));
  }
 

{code}
so i think if (!(node instanceof InnerNode)) should be removed.  Please correct me if i was wrong. Thank you.

 

> NetworkTopology: getLeaf should check for invalid topologies
> ------------------------------------------------------------
>
>                 Key: HADOOP-8159
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8159
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Colin McCabe
>            Assignee: Colin McCabe
>            Priority: Major
>             Fix For: 1.1.0, 2.0.0-alpha
>
>         Attachments: HADOOP-8159-b1.005.patch, HADOOP-8159-b1.007.patch, HADOOP-8159.005.patch, HADOOP-8159.006.patch, HADOOP-8159.007.patch, HADOOP-8159.008.patch, HADOOP-8159.009.patch
>
>
> Currently, in NetworkTopology, getLeaf doesn't do too much validation on the InnerNode object itself. This results in us getting ClassCastException sometimes when the network topology is invalid. We should have a less confusing exception message for this case.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org