[
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
