sodonnel commented on code in PR #5929:
URL: https://github.com/apache/ozone/pull/5929#discussion_r1455774731


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitorImpl.java:
##########
@@ -398,6 +398,12 @@ private boolean checkPipelinesClosedOnNode(TrackedNode dn)
 
   private boolean checkContainersReplicatedOnNode(TrackedNode dn)
       throws NodeNotFoundException {
+    Map<String, List<ContainerID>> containerOnDn = 
getContainersReplicatedOnNode(dn, true);
+    return (containerOnDn.get("UnderReplicated").size() == 0) && 
(containerOnDn.get("UnClosed").size() == 0);
+  }
+
+  public Map<String, List<ContainerID>> 
getContainersReplicatedOnNode(TrackedNode dn, boolean updateMetrics)

Review Comment:
   With this change, the client is going to trigger a somewhat expensive 
operation on SCM. There is potential for multiple clients to issue these calls 
at the same time, and we don't really have a way to throttle it.
   
   I think it would be better, if `checkContainersReplicatedOnNode` saved the 
counts and ID lists, perhaps as a map inside TrackedNode. Then 
`getContainersReplicatedOnNode` can simply retrive the value stored inside 
tracked node. Note that there will be a period of time where the node is 
scheduled for decommission, but it has not been checked yet, so we would need 
to decide what to return in that case.



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