sodonnel commented on code in PR #3606:
URL: https://github.com/apache/ozone/pull/3606#discussion_r924332915
##########
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:
Why do we need to ensure the excluded node is in the available nodes? Can we
not just use the excluded nodes as it is, without having to iterate over the
available nodes?
The available nodes list could be quite large, maybe over 1000 entries. That
would mean it needs scan the excluded list for each of these. Lets say the
excluded list contains 10 entries, that would mean 10k comparisons in the worst
case.
I think I don't understand how this fixes the issue, as the original code
just had `unavailableNodes.addAll(excludedNodes);`
--
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]
