You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Lisheng Sun (Jira)" <ji...@apache.org> on 2019/10/19 02:42:00 UTC
[jira] [Updated] (HADOOP-16662) Remove invalid judgment in
NetworkTopology#add()
[ https://issues.apache.org/jira/browse/HADOOP-16662?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lisheng Sun updated HADOOP-16662:
---------------------------------
Description:
The method of NetworkTopology#add() as follow:
{code:java}
/** Add a leaf node
* Update node counter & rack counter if necessary
* @param node node to be added; can be null
* @exception IllegalArgumentException if add a node to a leave
or node to be added is not a leaf
*/
public void add(Node node) {
if (node==null) return;
int newDepth = NodeBase.locationToDepth(node.getNetworkLocation()) + 1;
netlock.writeLock().lock();
try {
if( node instanceof InnerNode ) {
throw new IllegalArgumentException(
"Not allow to add an inner node: "+NodeBase.getPath(node));
}
if ((depthOfAllLeaves != -1) && (depthOfAllLeaves != newDepth)) {
LOG.error("Error: can't add leaf node {} at depth {} to topology:{}\n",
NodeBase.getPath(node), newDepth, this);
throw new InvalidTopologyException("Failed to add " + NodeBase.getPath(node) +
": You cannot have a rack and a non-rack node at the same " +
"level of the network topology.");
}
Node rack = getNodeForNetworkLocation(node);
if (rack != null && !(rack instanceof InnerNode)) {
throw new IllegalArgumentException("Unexpected data node "
+ node.toString()
+ " at an illegal network location");
}
if (clusterMap.add(node)) {
LOG.info("Adding a new node: "+NodeBase.getPath(node));
if (rack == null) {
incrementRacks();
}
if (!(node instanceof InnerNode)) {
if (depthOfAllLeaves == -1) {
depthOfAllLeaves = node.getLevel();
}
}
}
LOG.debug("NetworkTopology became:\n{}", this);
} finally {
netlock.writeLock().unlock();
}
}
{code}
{code:java}
if( node instanceof InnerNode ) {
throw new IllegalArgumentException(
"Not allow to add an inner node: "+NodeBase.getPath(node));
}
if (!(node instanceof InnerNode)) is invalid，since there is already a judgement before as follow:
if (!(node instanceof InnerNode)) {
if (depthOfAllLeaves == -1) {
depthOfAllLeaves = node.getLevel();
}
}{code}
so i think if (!(node instanceof InnerNode)) should be removed.
was:
The method of NetworkTopology#add() as follow:
{code:java}
/** Add a leaf node
* Update node counter & rack counter if necessary
* @param node node to be added; can be null
* @exception IllegalArgumentException if add a node to a leave
or node to be added is not a leaf
*/
public void add(Node node) {
if (node==null) return;
int newDepth = NodeBase.locationToDepth(node.getNetworkLocation()) + 1;
netlock.writeLock().lock();
try {
if( node instanceof InnerNode ) {
throw new IllegalArgumentException(
"Not allow to add an inner node: "+NodeBase.getPath(node));
}
if ((depthOfAllLeaves != -1) && (depthOfAllLeaves != newDepth)) {
LOG.error("Error: can't add leaf node {} at depth {} to topology:{}\n",
NodeBase.getPath(node), newDepth, this);
throw new InvalidTopologyException("Failed to add " + NodeBase.getPath(node) +
": You cannot have a rack and a non-rack node at the same " +
"level of the network topology.");
}
Node rack = getNodeForNetworkLocation(node);
if (rack != null && !(rack instanceof InnerNode)) {
throw new IllegalArgumentException("Unexpected data node "
+ node.toString()
+ " at an illegal network location");
}
if (clusterMap.add(node)) {
LOG.info("Adding a new node: "+NodeBase.getPath(node));
if (rack == null) {
incrementRacks();
}
if (!(node instanceof InnerNode)) {
if (depthOfAllLeaves == -1) {
depthOfAllLeaves = node.getLevel();
}
}
}
LOG.debug("NetworkTopology became:\n{}", this);
} finally {
netlock.writeLock().unlock();
}
}
{code}
> Remove invalid judgment in NetworkTopology#add()
> ------------------------------------------------
>
> Key: HADOOP-16662
> URL: https://issues.apache.org/jira/browse/HADOOP-16662
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Lisheng Sun
> Assignee: Lisheng Sun
> Priority: Minor
>
> The method of NetworkTopology#add() as follow:
> {code:java}
> /** Add a leaf node
> * Update node counter & rack counter if necessary
> * @param node node to be added; can be null
> * @exception IllegalArgumentException if add a node to a leave
> or node to be added is not a leaf
> */
> public void add(Node node) {
> if (node==null) return;
> int newDepth = NodeBase.locationToDepth(node.getNetworkLocation()) + 1;
> netlock.writeLock().lock();
> try {
> if( node instanceof InnerNode ) {
> throw new IllegalArgumentException(
> "Not allow to add an inner node: "+NodeBase.getPath(node));
> }
> if ((depthOfAllLeaves != -1) && (depthOfAllLeaves != newDepth)) {
> LOG.error("Error: can't add leaf node {} at depth {} to topology:{}\n",
> NodeBase.getPath(node), newDepth, this);
> throw new InvalidTopologyException("Failed to add " + NodeBase.getPath(node) +
> ": You cannot have a rack and a non-rack node at the same " +
> "level of the network topology.");
> }
> Node rack = getNodeForNetworkLocation(node);
> if (rack != null && !(rack instanceof InnerNode)) {
> throw new IllegalArgumentException("Unexpected data node "
> + node.toString()
> + " at an illegal network location");
> }
> if (clusterMap.add(node)) {
> LOG.info("Adding a new node: "+NodeBase.getPath(node));
> if (rack == null) {
> incrementRacks();
> }
> if (!(node instanceof InnerNode)) {
> if (depthOfAllLeaves == -1) {
> depthOfAllLeaves = node.getLevel();
> }
> }
> }
> LOG.debug("NetworkTopology became:\n{}", this);
> } finally {
> netlock.writeLock().unlock();
> }
> }
> {code}
> {code:java}
> if( node instanceof InnerNode ) {
> throw new IllegalArgumentException(
> "Not allow to add an inner node: "+NodeBase.getPath(node));
> }
> if (!(node instanceof InnerNode)) is invalid，since there is already a judgement before as follow:
> if (!(node instanceof InnerNode)) {
> if (depthOfAllLeaves == -1) {
> depthOfAllLeaves = node.getLevel();
> }
> }{code}
> so i think if (!(node instanceof InnerNode)) should be removed.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org