[jira] [Commented] (HADOOP-12173) NetworkTopology#add calls NetworkTopology#toString always

["Inigo Goiri (JIRA)" <ji...@apache.org>]

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

Inigo Goiri commented on HADOOP-12173:
--------------------------------------

Currently, the code does:
{code}
  public void add(Node node) {
    if (node==null) return;
    int newDepth = NodeBase.locationToDepth(node.getNetworkLocation()) + 1;
    netlock.writeLock().lock();
    try {
      String oldTopoStr = this.toString();
      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 " + NodeBase.getPath(node) +
            " at depth " + newDepth + " to topology:\n" + oldTopoStr);
        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.");
      }
{code}

This implies doing a toString for every single addition.

> NetworkTopology#add calls NetworkTopology#toString always
> ---------------------------------------------------------
>
>                 Key: HADOOP-12173
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12173
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 2.7.0
>            Reporter: Inigo Goiri
>             Fix For: 2.7.1
>
>
> It always does a toString of the whole topology but this is not required when there are no errors. This is adding a very big overhead to large clusters as it's walking the whole tree every time we add a node to the cluster.
> HADOOP-10953 did some fix in that area but the errors is still there.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)