siddhantsangwan commented on code in PR #6050:
URL: https://github.com/apache/ozone/pull/6050#discussion_r1461780656


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -88,19 +93,25 @@ private boolean 
isContainerReplicatingOrDeleting(ContainerID containerID) {
    * 5. Container should be closed.
    * 6. If the {@link LegacyReplicationManager} is enabled, then the container 
should not be an EC container.
    * @param node DatanodeDetails for which to find candidate containers.
-   * @return NavigableSet of candidate containers that satisfy the criteria.
+   * @return Set of candidate containers that satisfy the criteria.
    */
-  public NavigableSet<ContainerID> getCandidateContainers(
+  public Set<ContainerID> getCandidateContainers(
       DatanodeDetails node, long sizeMovedAlready) {
-    NavigableSet<ContainerID> containerIDSet =
-        new TreeSet<>(orderContainersByUsedBytes().reversed());
     try {
-      containerIDSet.addAll(nodeManager.getContainers(node));
+      // Initialize containerSet for node
+      if (!setMap.containsKey(node)) {
+        addNodeToSetMap(node);
+      }
+      // In case the node is removed
+      nodeManager.getContainers(node);

Review Comment:
   Are we making this call just to see if an exception gets thrown? In that 
case this is a bit awkward and confusing. Does node manager provide an API that 
we can use first to check if SCM knows the node, and then get its containers 
(or remove them from the cache is the node isn't there anymore)?



-- 
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]

Reply via email to