Details
-
Bug
-
Status: Resolved
-
Minor
-
Resolution: Fixed
-
1.3.0
-
None
-
None
-
None
Description
In NetworkTopology#add(Node node), an incorrect node may be added to the cluster even if an exception is thrown.
This is the original code:
if (clusterMap.add(node)) { LOG.info("Adding a new node: "+NodeBase.getPath(node)); if (rack == null) { numOfRacks++; } if (!(node instanceof InnerNode)) { if (depthOfAllLeaves == -1) { depthOfAllLeaves = node.getLevel(); } else { if (depthOfAllLeaves != node.getLevel()) { LOG.error("Error: can't add leaf node at depth " + node.getLevel() + " to topology:\n" + oldTopoStr); throw new InvalidTopologyException("Invalid network topology. " + "You cannot have a rack and a non-rack node at the same " + "level of the network topology."); } } }
This is a potential bug, because a wrong leaf node is already added to the cluster before throwing the exception. However, we can't check this (depthOfAllLeaves != node.getLevel()) before if (clusterMap.add(node)), because node.getLevel() will work correctly only after clusterMap.add(node) has been executed.
A possible solution to this is checking the depthOfAllLeaves in clusterMap.add(node). Note that this is a recursive call. A check should be put at the bottom of this recursive call. If check fails, don't add this leaf and all its upstream racks.