[
https://issues.apache.org/jira/browse/HDDS-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16786885#comment-16786885
]
Sammi Chen commented on HDDS-699:
---------------------------------
Thanks [~szetszwo] for such detail comments. Yes, getLeaf() in InnerNodeImpl
and chooseRandom() in NetworkTopologyImpl is a little bit complicated. Let me
do some explanation.
Currently InnerNodeImpl is not a thread safe implementation, so I would not
suggest other modules to call it directly. All the functions provided by
InnerNodeImpl should be proxy-ed by NetworkTopologyImpl. Based on this
assumption, excludedScope and excludedNodes overlap check is done in
NetworkTopologyImpl#chooseRandom(), so the result can be shared by
InnerNodeImpl#getLeaf and NetworkTopologyImpl#getAvailableNodesCountMutable.
Here are the code pieces,
{code:java}
// check overlap of excludedScope and scope, make sure excludedScope is a
// subset of scope
if (excludedScope != null) {
// excludeScope covers scope
if (scope.startsWith(excludedScope)) {
return null;
}
// excludeScope and scope share nothing
if (!excludedScope.startsWith(scope)) {
excludedScope = null;
}
}
{code}
and
{code:java}
Collection<Node> mutableExNodes = null;
if (excludedNodes != null) {
// Remove duplicate in excludedNodes
mutableExNodes =
excludedNodes.stream().distinct().collect(Collectors.toList());
}
InnerNode innerNode = (InnerNode)scopeNode;
// remove duplicate nodes in mutableExNodes and excludedScope
excludedScope = NetUtils.removeDuplicate(this, mutableExNodes,
excludedScope, ancestorGen);
{code}
bq. let's consistently use "level" instead of "generation" in the code.
Currenlty level has the different definition from generation. Level 1 is ROOT,
level 2 is ROOT's children, level 3 is ROOT's grand children, and so on.
Generation is from leaf node's point of view. Generation 1 is leaf's parent,
which is probabily not ROOT. I worry about using the same word will cause
confusion. What do you think?
{quote} getAncestorNodeMap seems incorrect since it does not consider
numLevelToExclude{quote}
I think here you mean getAncestorCounts. getAncestorCounts is to return exclude
node's ancestor at specific generation, to find the proper InnerNode in
childrenMap. AncestorGen is not involved purposely.
{quote}Let's change getAncestor(0) to return this.{quote}
I would like to keep current behavior to fit its function name. Otherwise
people may has questions.
After gone through your sample code, I see a way to make code more concise.
Will try to improve the getLeaf functions in next patch. And all the rest
comments will be addressed in next patch too.
> 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, HDDS-699.02.patch,
> HDDS-699.03.patch, HDDS-699.04.patch, HDDS-699.05.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
