Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-issues@hadoop.apache.org
Date: Tue, 13 Nov 2012 01:36:12 +0000 (UTC)
From: "Jing Zhao (JIRA)" <jira@apache.org>
To: common-issues@hadoop.apache.org
Message-ID: <421074995.104524.1352770572609.JavaMail.jiratomcat@arcas>
In-Reply-To: <79078274.85721.1347821047421.JavaMail.jiratomcat@arcas>
Subject: [jira] [Commented] (HADOOP-8820) Backport HADOOP-8469 and
 HADOOP-8470: add "NodeGroup" layer in new NetworkTopology (also known as
 NetworkTopologyWithNodeGroup)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Jing Zhao commented on HADOOP-8820:
-----------------------------------

Besides, some other small issues:
1. {noformat}
+    @Override
+    boolean isRack() {
+      // it is node group
+      if (getChildren().isEmpty()) {
+        return false;
+      }
+
+      Node firstChild = children.get(0);
+
+      if (firstChild instanceof InnerNode) {
+        Node firstGrandChild = (((InnerNode) firstChild).children).get(0);
+        if (firstGrandChild instanceof InnerNode) {
{noformat}

I think here maybe firstChild.children can be empty thus the get(0) may cause an IndexOutofBoundsException. So maybe we need to check if children is empty first (and if it is, we can return true)?

2. {noformat}
+  public String getNodeGroup(String loc) {
+    try {
+      netlock.readLock().lock();
+      loc = InnerNode.normalize(loc);
+      Node locNode = getNode(loc);
+      if (locNode instanceof InnerNodeWithNodeGroup) {
+        InnerNodeWithNodeGroup node = (InnerNodeWithNodeGroup) locNode;
+        if (node.isNodeGroup()) {
+          return loc;
+        } else if (node.isRack()) {
+          // not sure the node group for a rack
+          return null;
+        } else {
+          // may be a leaf node
+          return getNodeGroup(node.getNetworkLocation());
+        }
{noformat}

Since the "locNode" is already an instance of InnerNodeWithNodeGroup, I think it cannot be a leaf node anymore, thus the code may never run into the "else" part?

I can create a separate jira to fix if you think the two issues stand.
                
> Backport HADOOP-8469 and HADOOP-8470: add "NodeGroup" layer in new NetworkTopology (also known as NetworkTopologyWithNodeGroup)
> -------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-8820
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8820
>             Project: Hadoop Common
>          Issue Type: New Feature
>          Components: net
>    Affects Versions: 1.0.0
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: HADOOP-8820.b1.002.patch, HADOOP-8820.patch
>
>
> This patch backport HADOOP-8469 and HADOOP-8470 to branch-1 and includes:
> 1. Make NetworkTopology class pluggable for extension.
> 2. Implement a 4-layer NetworkTopology class (named as NetworkTopologyWithNodeGroup) to use in virtualized environment (or other situation with additional layer between host and rack).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira