sodonnel commented on code in PR #3606:
URL: https://github.com/apache/ozone/pull/3606#discussion_r924372689
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java:
##########
@@ -107,8 +107,12 @@ public List<DatanodeDetails> chooseDatanodes(
int excludedNodesCount = excludedNodes == null ? 0 : excludedNodes.size();
List<Node> availableNodes = networkTopology.getNodes(
networkTopology.getMaxLevel());
+ List<Node> unavailableNodes = new ArrayList<>();
int totalNodesCount = availableNodes.size();
if (excludedNodes != null) {
+ unavailableNodes.addAll(
+ availableNodes.stream().filter(excludedNodes::contains)
Review Comment:
I understand now what is going on here - the network location is not set
inside the DatanodeDetails and therefore it doesn't work correctly in the
topology code. So you are looking up the real datanode details if they are
present in the available nodes to get a DNDetails object that is fully
populated.
I immediately wonder how the other placement policies behave with excluded
nodes? We need to check if they have the same problem and fix them too.
However I am not sure if this fix is the best - should we not ensure that
the excluded nodes passed in have the location setup correctly inside the
DatanodeDetails? Or check for the nodes in NodeManager when inflating the
protobuf?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
