HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. by ashishkumar50 · Pull Request #10000 · apache/ozone

ashishkumar50 · 2026-03-30T04:23:43Z

What changes were proposed in this pull request?

Maintain space accounting during container allocation in SCM. More detail description is in Jira.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14921

How was this patch tested?

UT and IT.

…allocation tracking.

rakeshadr

Thanks @ashishkumar50 for providing the patch. Added a few comments, please take care.

rakeshadr · 2026-04-01T11:29:18Z

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java

+            if (!alreadyOnDn && getContainerManager() instanceof ContainerManagerImpl) {
+              ((ContainerManagerImpl) getContainerManager())
+                  .getPendingContainerTracker()
+                  .removePendingAllocation(dd, id);


Say, DN is healthy, all containers confirmed, no new allocations → that DN's bucket never rolls even though heartbeats come every 30 seconds, right?

t=0 Container C1 allocated → pending recorded in tracker t=60-120 FCR arrives from DN → cid = C1 → alreadyInDn = expectedContainersInDatanode.remove(C1) = FALSE → !alreadyInDn = TRUE → removePendingAllocation called → rollIfNeeded fires ✓ → C1 added to NM DN-set

How abt rolls on every processHeartbeat, every 30 seconds regardless of container state changes ?

Added roll in every node report which is per minute from DN.

rakeshadr · 2026-04-01T11:47:21Z

...p-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java

+      }
+
+      // Cleanup empty buckets to prevent memory leak
+      if (bucket.isEmpty()) {


Potentially hits concurrency issue. Say two threads entered this block.

Thread-1 (removePendingAllocation): bucket.isEmpty(), returns true

Thread-2 (recordPendingAllocationForDatanode): computeIfAbsent(uuid) returns same bucket
reference (key still exists), calls bucket.add(containerID) and now the bucket will be non-empty

Thread-1: datanodeBuckets.remove(uuid, bucket), then removes the non-empty bucket and now the containerID will be in a detached bucket object, right?

I think, we need to add synchronization to avoid detached bucket object.

Added sync at bucket level.

rakeshadr · 2026-04-01T11:55:20Z

...ds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestPendingContainerTracker.java

+  }
+
+  @Test
+  public void testRemoveFromBothWindows() {


Do we have test scenario covering roll over?

The two-window rolling behavior (container in previousWindow roll after 2× interval). Say, add C1 in currentWindow, then moves C1 to previousWindow, then wait for the roll over.

Added test for this testTwoWindowRollAgesOutContainerAfterTwoIntervals.

sumitagrawl

@ashishkumar50 Thanks for working over this, have few review comments.

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java

sumitagrawl · 2026-04-01T13:07:50Z

...p-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java

+   * @param pipeline The pipeline where container is allocated
+   * @param containerID The container being allocated
+   */
+  public void recordPendingAllocation(Pipeline pipeline, ContainerID containerID) {


This needs to be part of SCMNodeManager, more specific to SCMNodeStat. Reason,

need handle even like stale node / dead node handler as cleanup

May need report this when reporting to CLI for available space in the DN

To be used for pipeline allocation policy, where container manager does not come in role
Its datanode space, just trying to identify already allocated space. And needs to be part of committed space at SCM when reporting to CLI, or other breakup.

Moved to node package

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java

sumitagrawl · 2026-04-01T13:12:20Z

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java

            processContainerReplica(dd, container, replicaProto, publisher, detailsForLogging);
+
+            // Remove from pending tracker when container is added to DN
+            if (!alreadyOnDn && getContainerManager() instanceof ContainerManagerImpl) {


Please check if node report is also send in ICR, this is for reason that node information should be updated with ICR at same time.

sumitagrawl · 2026-04-01T13:18:32Z

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java

+        // (1*5GB) + (2*5GB) = 15GB → actually 3 containers
+        long totalCapacity = 0L;
+        long effectiveAllocatableSpace = 0L;
+        for (StorageReportProto report : storageReports) {


Instead of calcuating all available and then removing, we can do progressive base, like,
required=pending+newAllocation
for each report
required = required - volumeUsage in roundoff value
if (required <= 0)
return true

But we need to reserve also, can do first add and check, if not present, remove containerId

OR other way,
when DN report storage handling, total consolidate value can also be added to memory to avoid looping on every call.

Updated the logic to break when enough space is available on any volume

...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java

...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java

aswinshakil · 2026-04-02T20:54:41Z

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java

+      return true;
+    } catch (Exception e) {
+      LOG.warn("Error checking space for pipeline {}", pipeline.getId(), e);
+      return true;


If we are not sure if we can create container here, Should we still choose this pipeline? Instead of making it generic, we can specify what to do for each exception we might see.

Moved the code, there is no exception here.

aswinshakil · 2026-04-02T21:02:09Z

...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java

+            // Remove from pending tracker when container is added to DN
+            // This container was just confirmed for the first time on this DN
+            // No need to remove on subsequent reports (it's already been removed)
+            if (container != null && getContainerManager() instanceof ContainerManagerImpl) {


Why not just add this to the ContainerManager interface? We can avoid these conversions. Is this because Recon uses the same code path and we don't want it to this? For Recon we can just make it a No-Op.

Moved to node package, so not required these conversions now.

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java

...p-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java

...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java

aswinshakil · 2026-04-02T22:11:35Z

...p-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java

+     * Roll the windows: previous = current, current = empty.
+     * Called when current time exceeds lastRollTime + rollIntervalMs.
+     */
+    synchronized void rollIfNeeded() {


Pending allocations can persist beyond 2× roll interval after long idle periods because rollIfNeeded() only rolls once.A single roll doesn’t clear entries older than two windows which can incorrectly block new allocations.

Roll will be done when node report comes which is every minute, so old entries will get clear.

aswinshakil · 2026-04-02T22:14:32Z

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java

+        long effectiveRemaining = effectiveAllocatableSpace - pendingAllocations;
+
+        // Check if there's enough space for a new container
+        if (effectiveRemaining < maxContainerSize) {


This makes the allocation little aggressive right? Even if we just have 5GB we allocate it. Should we have leave some buffer when allocating a container?

No need of extra buffer here, as we are anyway going to give buffer in DN by considering soft and hard limit. So in case of some overflow DN will accept it until hard limit.

...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java

ashishkr200 added 4 commits March 30, 2026 02:54

HDDS-14921. Improve space accounting in SCM with In-Flight container …

511ffd0

…allocation tracking.

Fix PMD

32caf74

Safe cast ozone conf

8706c1c

Fix test case

634c94e

ashishkumar50 marked this pull request as draft March 30, 2026 04:24

ashishkumar50 requested review from aswinshakil, rakeshadr, sumitagrawl and szetszwo March 30, 2026 04:39

ashishkr200 added 3 commits March 30, 2026 23:40

update test case

4150f5d

Handle unregistered datanodes

0895224

Fix test

5008cd4

rakeshadr reviewed Apr 1, 2026

View reviewed changes

sumitagrawl reviewed Apr 1, 2026

View reviewed changes

aswinshakil reviewed Apr 2, 2026

View reviewed changes

ashishkr200 added 2 commits April 5, 2026 01:36

Move PendingContainerTracker to node package and fix review comments

848a8d8

Move declaration to top

f74bc9b

ashishkumar50 marked this pull request as ready for review April 4, 2026 21:51

Conversation

ashishkumar50 commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

rakeshadr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ashishkumar50 commented Mar 30, 2026 •

edited

Loading