szetszwo commented on code in PR #9621:
URL: https://github.com/apache/ozone/pull/9621#discussion_r2679989970


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##########
@@ -1564,23 +1564,48 @@ public String getLabel() {
   }
 
   /**
-   * Returns the min of no healthy volumes reported out of the set
-   * of datanodes constituting the pipeline.
+   * Returns the open container limit for a pipeline based on the given
+   * datanodes and containers-per-volume configuration.
    */
   @Override
-  public int minHealthyVolumeNum(List<DatanodeDetails> dnList) {
-    List<Integer> volumeCountList = new ArrayList<>(dnList.size());
+  public int openContainerLimit(List<DatanodeDetails> dnList,
+      int numContainerPerVolume) {
+    Preconditions.checkArgument(dnList != null && !dnList.isEmpty(),
+        "dnList must not be empty");
+
+    int min = Integer.MAX_VALUE;
+
     for (DatanodeDetails dn : dnList) {
-      try {
-        volumeCountList.add(nodeStateManager.getNode(dn).
-                getHealthyVolumeCount());
-      } catch (NodeNotFoundException e) {
-        LOG.warn("Cannot generate NodeStat, datanode {} not found.",
-                dn.getID());
+      int perDn = openContainerLimit(dn, numContainerPerVolume);
+      min = Math.min(min, perDn);
+    }
+
+    return min == Integer.MAX_VALUE ? 0 : min;
+  }
+
+  /**
+   * Returns the open container limit contributed by a single datanode.
+   */
+  private int openContainerLimit(DatanodeDetails dn,
+      int numContainerPerVolume) {
+    try {
+      int healthy = nodeStateManager.getNode(dn).getHealthyVolumeCount();
+      if (healthy <= 0) {
+        return 0;
       }
+
+      int capacity = numContainerPerVolume * healthy;
+
+      int pipelineLimit = pipelineLimit(dn); // SCMNodeManager already has this
+      int denom = Math.max(1, pipelineLimit); // avoid division-by-zero

Review Comment:
   When pipelineLimit is zero, openContainerLimit(..) should return zero since 
it won't be able to create any containers.
   
   The code in 
https://issues.apache.org/jira/secure/attachment/13080186/9621_review.patch is 
incorrect.  It should be:
   ```java
    public int openContainerLimit(List<DatanodeDetails> datanodes) {
      int min = Integer.MAX_VALUE;
      for (DatanodeDetails dn : datanodes) {
        final int pipelineLimit = pipelineLimit(dn);
        if (pipelineLimit <= 0) {
          return 0;
        }
   
        final int containerLimit = 1 + (numContainerPerVolume * 
getHealthyVolumeCount(dn) - 1) / pipelineLimit;
        if (containerLimit < min) {
          min = containerLimit;
        }
      }
      return min;
    }
   ```



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