[ 
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

Reply via email to