[
https://issues.apache.org/jira/browse/HDDS-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16751901#comment-16751901
]
Xiaoyu Yao edited comment on HDDS-699 at 1/25/19 6:07 AM:
----------------------------------------------------------
Thanks [~Sammi] for working on this. Patch v2 LGTM overall. Here are some
comments. I will add more comments on unit test later.
DatanodeDetails.java
Line 220: is there a reason to remove the compareTo @Override annotation?
Node.java
Line 45: NIT: typo "contamination" -> "concatenation"
Line 72: do we really want to allow setLevel() explictly or it is for testing
only? This may mess up with the
actual level that is set up via setParent().
InnerNode.java
Line 89: does the leafIndex count the excludedNodes?
InnerNodeImpl.java
Line 295-301: can we document this in the getLeaf() API?
network-topology-nodegroup.xml
Line 56 and 68: the nodegroup in 68 does not seem to match with the prefix ng
NodeSchema.java
Line 80-90: maybe we can add a build class to help reducing the # of
constructors.
Line 102: do we assume case sensitive for the network path and prefix here?
NodeSchemaLoader.java
Line 126/128/130/186/187: NIT can we get these perdefined tag name defined as
static const?
NodeSchemaManager.java
Line 98: can we add javadoc for completePath()?
was (Author: xyao):
Thanks [~Sammi] for working on this. Patch v2 LGTM overall. Here are some
comments. I will add more comments on unit test later.
DatanodeDetails.java
Line 220: is there a reason to remove the compareTo @Override annotation?
Node.java
Line 45: NIT: typo "contamination" -> "concatenation"
Line 72: do we really want to allow setLevel() explictly or it is for testing
only? This may mess up with the
actual level that is set up via setParent().
InnerNode.java
Line 89: does the leafIndex count the excludedNodes?
InnerNodeImpl.java
Line 295-301: can we document this in the getLeaf() API?
NodeSchema.java
Line 80-90: maybe we can add a build class to help reducing the # of
constructors.
Line 102: do we assume case sensitive for the network path and prefix here?
NodeSchemaLoader.java
Line 126/128/130/186/187: NIT can we get these perdefined tag name defined as
static const?
NodeSchemaManager.java
Line 98: can we add javadoc for completePath()?
> 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
