[jira] [Commented] (HADOOP-16662) Remove invalid judgment in NetworkTopology#add()

["Ayush Saxena (Jira)" <ji...@apache.org>]

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

Ayush Saxena commented on HADOOP-16662:
---------------------------------------

Thanx [~leosun08] for the patch.
v001 LGTM +1(Pending Jenkins)

> Remove invalid judgment in NetworkTopology#add()
> ------------------------------------------------
>
>                 Key: HADOOP-16662
>                 URL: https://issues.apache.org/jira/browse/HADOOP-16662
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Lisheng Sun
>            Assignee: Lisheng Sun
>            Priority: Minor
>         Attachments: HADOOP-16662.001.patch
>
>
> The method of NetworkTopology#add() as follow:
> {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 ((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();
>   }
> }
> {code}
> {code:java}
> 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();
>   }
> }{code}
> so i think if (!(node instanceof InnerNode)) should be removed.



--
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