ashishkumar50 commented on code in PR #10497:
URL: https://github.com/apache/ozone/pull/10497#discussion_r3443181140


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##########
@@ -208,11 +213,45 @@ public boolean checkSpaceAndRecordAllocation(DatanodeInfo 
datanodeInfo, Containe
   }
 
   /**
-   * Remove a pending container allocation from a specific DataNode.
-   * Removes from both current and previous windows.
-   * Called when container is confirmed.
+   * Returns true if the given datanode has at least one allocatable container 
slot
+   * available, accounting for pending in-flight allocations.
+   *
+   * <p>Slot availability is based on {@code maxContainerSize}: a slot exists 
for each
+   * {@code maxContainerSize}-worth of usable space on any volume. This 
read-only check
+   * is intended for the placement policy and does not consume a slot.
+   *
+   * @param datanodeInfo the datanode to check
+   * @return true if at least one container slot is available
+   */
+  public boolean hasAvailableSpace(DatanodeInfo datanodeInfo) {
+    Objects.requireNonNull(datanodeInfo, "datanodeInfo == null");
+    List<StorageReportProto> storageReports = datanodeInfo.getStorageReports();
+    if (storageReports.isEmpty()) {
+      return false;
+    }
+    TwoWindowBucket bucket = datanodeInfo.getPendingContainerAllocations();
+    bucket.rollIfNeeded();
+    final int pendingCount = bucket.getCount();
+    long allocatableCount = 0;
+    for (StorageReportProto report : storageReports) {
+      allocatableCount += Math.max(0L, VolumeUsage.getUsableSpace(report)) / 
maxContainerSize;

Review Comment:
   DN actually removes fail volume. And so SCM also don't get those fail volume 
list. I have added defensive check though.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##########
@@ -105,7 +105,11 @@ synchronized void rollIfNeeded() {
         previousWindow.clear();
         currentWindow.clear();
         lastRollTime = now;
-        LOG.debug("Double roll interval elapsed ({}ms): dropped {} pending 
containers", elapsed, dropped);
+        if (dropped > 0) {
+          LOG.warn("PendingContainerTracker: force-dropped {} unconfirmed 
pending containers "
+              + "on DN {} after {}ms (2x rollInterval). "
+              + "Container reports may have been lost.", dropped, datanodeID, 
elapsed);

Review Comment:
   Adding metrics here means, we need to update `TwoWindowBucket` constructor 
and also `DatanodeInfo` constructor to accept metrics object. Since 
`DatanodeInfo` class is used by many test class, it requires changing in them 
too. If we need it then it's better to do in other Jira.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##########
@@ -105,7 +105,11 @@ synchronized void rollIfNeeded() {
         previousWindow.clear();
         currentWindow.clear();
         lastRollTime = now;
-        LOG.debug("Double roll interval elapsed ({}ms): dropped {} pending 
containers", elapsed, dropped);
+        if (dropped > 0) {
+          LOG.warn("PendingContainerTracker: force-dropped {} unconfirmed 
pending containers "
+              + "on DN {} after {}ms (2x rollInterval). "
+              + "Container reports may have been lost.", dropped, datanodeID, 
elapsed);
+        }
       } else if (elapsed >= rollIntervalMs) {
         previousWindow.clear();

Review Comment:
   Same as above



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