[ https://issues.apache.org/jira/browse/HDDS-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16747999#comment-16747999 ]
Yiqun Lin commented on HDDS-699: -------------------------------- Thanks [~Sammi]. I am still reviewing some details of this patch. It looks more flexible than existing HDFS NetworkTopology. Some few initial comments from me: In {{InnerNodeImpl#remove}} {code:java} + /** + * Remove node <i>node</i> from the subtree of this node. + * @param node node to be deleted + */ + public void remove(Node node) { + ... + if (isParent(node)) { + // this node is the parent, remove it directly + ... + } else { + // find the next ancestor node + String ancestorName = getNextLevelAncestorName(node); + InnerNodeImpl childNode = (InnerNodeImpl)childrenMap.get(ancestorName); + Preconditions.checkNotNull(childNode, "InnerNode is deleted before leaf"); + // remove node from the parent node + childNode.remove(node); + // if the parent node has no children, remove the parent node too + if (childNode.getNumOfChildren() == 0) { + childrenMap.remove(ancestorName); + } + numOfLeaves--; + } + } {code} There is a corner case I am imaging: The parent node has no children, then we remove the parent node too. But after this, the parent's parent node also has no children. I think we should also do a removal. {noformat} root | InnerNode1 | remove leaf node InnerNode2 ---> root | LeafNode {noformat} Can we address this corner case in test {{testAddRemove}}? Also a similar case, we can add a node without parent existed, and to see if the parent nodes is created as we expected. Another one nit: Can we rename {{NetConf}}, {{NetUtils}} to a more readable name {{TopologyConf,}} {{TopologyUtils}}? Here {{Net}} will be understood as the meaning of Network not NetworkTopology, > Detect Ozone Network topology > ----------------------------- > > Key: HDDS-699 > URL: https://issues.apache.org/jira/browse/HDDS-699 > Project: Hadoop Distributed Data Store > Issue Type: Sub-task > Reporter: Xiaoyu Yao > Assignee: Sammi Chen > Priority: Major > Attachments: HDDS-699.00.patch, HDDS-699.01.patch > > > Traditionally this has been implemented in Hadoop via script or customizable > java class. One thing we want to add here is the flexible multi-level support > instead of fixed levels like DC/Rack/NG/Node. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org