HDDS-16045. Make PendingContainerTracker StorageType aware by F64116045 · Pull Request #10928 · apache/ozone

F64116045 · 2026-08-02T14:43:46Z

What changes were proposed in this pull request?

PendingContainerTracker currently accounts pending container allocations at the datanode level. With storage policy support, pending allocations should be counted only against the StorageType they target. For example, a pending DISK allocation should not reduce the available space calculated for SSD or ARCHIVE allocations.

This PR makes PendingContainerTracker store the StorageType for each pending container allocation. It adds StorageType-aware APIs in PendingContainerTracker and NodeManager, while keeping the existing untyped APIs and behavior for callers that do not have storage tier information.

PipelineManager derives the StorageType from the pipeline StorageTier and passes it to NodeManager when checking space and recording pending allocations.

The tests cover StorageType-specific pending counts, StorageType-specific space checks, the existing untyped behavior, and propagation from StorageTier.DISK to StorageType.DISK.

What is the link to the Apache JIRA

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

How was this patch tested?

mvn -B --no-transfer-progress -pl hadoop-hdds/server-scm -am -Dtest=TestPendingContainerTracker test -DskipShade -DskipRecon -DskipDocs

CI: https://github.com/F64116045/ozone/actions/runs/30770518425

…#10657)

…NodeLimit#testMetrics (apache#10659)

…guredIncludeContainers (apache#10660)

…0641)

…ils (apache#10593)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…rationResultException (apache#10667)

…ion (apache#10621)

…out retry or failover. (apache#10479)

… -1 (apache#10585).

…ng limit selector (apache#10677).

…+ AGENTS.md) (apache#10483) Co-authored-by: Siyao Meng <50227127+smengcl@users.noreply.github.com>

…che#10679)

…he#10584) Co-authored-by: Doroszlai, Attila <adoroszlai@apache.org>

Generated-by: Codex (GPT-5.3-Codex, GPT-5.4), Claude Code (Opus 4.8)

…g fixes (apache#10574)

…for EC keys (apache#10592)

…apache#10684)

…apache#10565).

…ith the checkpoint index (apache#10943) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…defrag (apache#10951)

apache#10944)

xichen01

@F64116045 Thanks for you patch, left some comments

xichen01 · 2026-08-09T16:26:59Z

    /**
     * Add container to current window.
     */
    synchronized boolean add(ContainerID containerID) {


this old method can be removed.

Removed the untyped add(...) method. Thanks.

xichen01 · 2026-08-09T16:27:59Z

  }

  @Override
  public void recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerID containerID) {


This can be removed too, make that recordPendingAllocationForDatanode must pass a storageType

Removed the untyped recordPendingAllocationForDatanode(...); callers now pass StorageType. Thanks.

xichen01 · 2026-08-09T16:31:38Z

@@ -189,6 +190,14 @@ default int getAllNodeCount() {
   */
  boolean hasSpaceForNewContainerAllocation(DatanodeID datanodeID);


To prevent callers from forgetting to pass the storageType parameter, we should retain only the hasSpaceForNewContainerAllocation method that includes storageType.

Removed the untyped space-check API so callers need to pass StorageType.

xichen01 · 2026-08-10T02:55:27Z

    }

    containerStateManager.addContainer(containerInfoBuilder.build());
+    pipelineManager.recordPendingAllocation(pipeline, containerID);


allocateContainer can be executed in parallel;
pipelineManager.hasEnoughSpace and pipelineManager.recordPendingAllocation are not protected by locks. In some cases, a DN may be allocated too many containers.

Addressed by adapting the atomic checkSpaceAndRecordAllocation(...) flow after updating from the latest HDDS-11233 branch. Thanks.

…ive is REPLACE (apache#10752).

devmadhuu · 2026-08-10T04:38:00Z

Added HDDS-16116 for rebasing HDDS-11233 feature branch with master. Keep hold till rebase finished.

apache#10969)

…pping. (apache#10931)

devmadhuu · 2026-08-10T10:17:46Z

@F64116045 I updated the shared feature branch HDDS-11233 on apache/ozone master to bring it up to date with master. The branch now = the original feature commits (unchanged SHAs) with a single Merge 'upstream/master' commit on top. New tip: 8decb32. Kindly do merge of this feature branch to your PR branch and resolve the conflicts.

…nding-container-storage-type # Conflicts: # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestPendingContainerTracker.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java

F64116045 · 2026-08-10T15:35:50Z

@devmadhuu Thanks for helping with the feature branch update.

I noticed HDDS-11233 is now back at ed07a4b, while this PR already merged 8decb32, so the diff became very large.

Should I wait for HDDS-11233 to be updated again, or rework this PR on the current base?

devmadhuu · 2026-08-10T16:06:17Z

@devmadhuu Thanks for helping with the feature branch update.

I noticed HDDS-11233 is now back at ed07a4b, while this PR already merged 8decb32, so the diff became very large.

Should I wait for HDDS-11233 to be updated again, or rework this PR on the current base?

Apologies for this confusion. Yes I rebased and force pushed the HDDS-11233 , but we need to follow the merge process instead of rebase for feature branches and raise PRs and follow the normal PR process. So I reverted the earlier forced push and now raised this PR #10983
Kindly wait till this PR gets merged.

devmadhuu · 2026-08-18T05:36:15Z

@devmadhuu Thanks for helping with the feature branch update.
I noticed HDDS-11233 is now back at ed07a4b, while this PR already merged 8decb32, so the diff became very large.
Should I wait for HDDS-11233 to be updated again, or rework this PR on the current base?

Apologies for this confusion. Yes I rebased and force pushed the HDDS-11233 , but we need to follow the merge process instead of rebase for feature branches and raise PRs and follow the normal PR process. So I reverted the earlier forced push and now raised this PR #10983 Kindly wait till this PR gets merged.

@F64116045 #10983 is merged.

…nding-container-storage-type # Conflicts: # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/invoker/ContainerStateManagerInvoker.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/ECPipelineProvider.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SimplePipelineProvider.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestPendingContainerTracker.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementFactory.java

F64116045 · 2026-08-19T14:24:43Z

Following the discussion in apache/ozone#10983, I also moved the data-space check from SCMCommonPlacementPolicy into PendingContainerTracker.

Previously, SCMCommonPlacementPolicy did this check directly.

Now SCMCommonPlacementPolicy delegates to NodeManager.hasAvailableSpace(...), and the combined check lives in PendingContainerTracker.

F64116045 · 2026-08-19T15:15:56Z

Thanks everyone for the guidance. Since the diff became larger after syncing the feature branch and addressing the review comments, I wanted to summarize the current patch to make review easier.

The main changes are:

PendingContainerTracker now records pending allocations with StorageType and counts them per requested storage type.
Container allocation uses the atomic checkSpaceAndRecordAllocation(...) flow and passes the StorageType derived from the pipeline StorageTier.
Pending allocations are removed when container reports confirm the container as present or absent on the datanode.
Following the discussion in HDDS-16116. Merge recent commits from master to HDDS-11233 #10983, the data-space check was moved from SCMCommonPlacementPolicy into PendingContainerTracker, exposed through NodeManager.hasAvailableSpace(...).

Most of the remaining test changes are mechanical updates for the new method signatures. The main behavioral tests are in:

TestPendingContainerTracker
TestPipelineManagerImpl
TestSCMNodeManager
TestSCMCommonPlacementPolicy

I ran the targeted SCM tests locally and will keep an eye on CI, thanks.

devmadhuu

Thanks @F64116045 for addressing the review comments and rebase with latest upstream feature branch.. Just few nits. Please check.

@ashishkumar50 request you also to kindly re-review after merge master with latest upstream feature branch.

devmadhuu · 2026-08-20T12:17:04Z

      DatanodeInfo dnInfo = getNode(op.getTarget().getID());
      if (dnInfo != null) {
-        recordAllocationForDatanode(dnInfo, containerID);
+        recordAllocationForDatanode(dnInfo, containerID, StorageType.DEFAULT);


This sending StorageType.DEFAULT may not be correct as every incoming req from RM / balancer will account for space under DISK tier. Can we pass the real tier based on containerInfo from the caller of this: org.apache.hadoop.hdds.scm.container.replication.ReplicationManager#adjustPendingOpsAndMetrics

Thanks for catching. I missed that path. Fixed.
It now carries the type from containerInfo instead of defaulting in opAdded. Added a small SSD test too.

devmadhuu · 2026-08-20T12:26:34Z

+    if (storageTier == null || storageTier == StorageTier.EMPTY) {
+      return null;
+    }
+    return storageTier.getUniformStorageType();


Though once pipeline gets created with a specific tier at pipeline creation time, but here using primary storage type only and no fallback for space pending allocation is okay ? for SSD tier, this returns ONLY "SSD" Just a doubt.. @ivandika3 @xichen01

Thanks for raising this.
The existing pipeline selection path also maps the tier through storageTier.getUniformStorageType() in RatisPipelineProvider and PipelineProvider and there is no fallback, so I think it is acceptable here?

(If fallback should be handled differently, I’m happy to update it, thanks.)

ashishkumar50

@F64116045 Thanks for the update, please find comments inline:

ashishkumar50 · 2026-08-24T05:20:10Z

    // Always slot-based (maxContainerSize unit).
-    if (!nodeManager.hasAvailableSpace(datanodeInfo)) {
+    if (!nodeManager.hasAvailableSpace(
+        datanodeInfo, dataSizeRequired, storageType)) {


dataSizeRequired is not required.

ashishkumar50 · 2026-08-24T05:48:09Z

-    private Set<ContainerID> currentWindow = new HashSet<>();
-    private Set<ContainerID> previousWindow = new HashSet<>();
+    private Map<ContainerID, StorageType> currentWindow = new HashMap<>();
+    private Map<ContainerID, StorageType> previousWindow = new HashMap<>();


Instead of doing like this, storing each container and its storageType. Instead we can make TwoWindowBucket as storageType for each datanode.

DataNode1: ├── DISK -> TwoWindowBucket └── SSD -> TwoWindowBucket DataNode2: ├── DISK -> TwoWindowBucket └── SSD -> TwoWindowBucket

ashishkumar50 · 2026-08-24T05:55:02Z

     */
    private boolean hasEnoughSpaceForNode(DatanodeInfo dn) {
-      if (!tracker.hasAvailableSpace(dn)) {
+      if (!tracker.hasAvailableSpace(dn, 0, null)) {


No need to pass 0 here.

ashishkumar50 · 2026-08-24T05:58:38Z


  @Override
  public boolean checkSpaceAndRecordAllocation(Pipeline pipeline, ContainerID containerID) {
+    StorageType storageType = getStorageTypeForPendingAllocation(pipeline);


What does it mean here getStorageTypeForPendingAllocation ? It just give storageType for pipeline, if so we can just rename getStorageType(pipeline).

szetszwo and others added 30 commits July 4, 2026 07:44

HDDS-15736. Change tests to use ContainerID.getIdForTesting() (apache…

af70016

…#10657)

HDDS-11093. Fix intermittent failure in TestContainerBalancerDatanode…

878eae2

…NodeLimit#testMetrics (apache#10659)

HDDS-15743. Bump gson to 2.14.0 (apache#10664)

d58317f

HDDS-15737. Fix intermittent failure in balancerShouldOnlySelectConfi…

888077b

…guredIncludeContainers (apache#10660)

HDDS-15744. Bump javassist to 3.32.0-GA (apache#10665)

9306a78

HDDS-15719. Add check for allowed action usage in workflows (apache#1…

0d3cdbc

…0641)

HDDS-15742. Bump nimbus-jose-jwt to 10.9.1 (apache#10662)

d1bbec9

HDDS-15651. Test case for DiskBalancer when markContainerForDelete fa…

b97a11b

…ils (apache#10593)

HDDS-10307. Speed up TestOzoneManagerHAWithStoppedNodes (apache#10658)

10a9437

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

HDDS-15741. Bump awssdk to 2.46.17 (apache#10661)

dbcd9b3

HDDS-11855. Fix flaky TestContainerBalancerDatanodeNodeLimit#checkIte…

f5a5f0f

…rationResultException (apache#10667)

HDDS-15605. Fix flaky testContainerExclusionWithClosedContainerExcept…

c7d83a7

…ion (apache#10621)

HDDS-15732. Some ozone commands ignore OZONE_MODULE_ACCESS_ARGS (apac…

b974be3

…he#10655)

HDDS-15747. Address review comments for HDDS-15083 (apache#10669)

426374d

HDDS-15579. Replace SimpleSpanProcessor with BatchSpanProcessor (apac…

be54528

…he#10569)

HDDS-15746. Bump kerby to 2.1.2 (apache#10666)

4f31879

HDDS-15552. Ratis events should not be published as metrics (apache#1…

6ef19ac

…0523)

HDDS-15170. Add mock-based unit tests for DataStream write path (apac…

67fd519

…he#10230)

HDDS-15521. StreamBlockInputStream fails with TimeoutIOException with…

2076d7f

…out retry or failover. (apache#10479)

HDDS-15587. [Recon] Show 0 for offline DN pending deletion instead of…

cb9430d

… -1 (apache#10585).

HDDS-14544. OM DB Insights: Duplicate API calls triggered when changi…

38e19ec

…ng limit selector (apache#10677).

HDDS-15526. Add security threat model (THREAT_MODEL.md + SECURITY.md …

caf40cf

…+ AGENTS.md) (apache#10483) Co-authored-by: Siyao Meng <50227127+smengcl@users.noreply.github.com>

HDDS-15760. Handle large or invalid OM snapshot local data YAMLs (apa…

0eb1c67

…che#10679)

HDDS-15614. Remove Datanode download/pull container replication (apac…

675c348

…he#10584) Co-authored-by: Doroszlai, Attila <adoroszlai@apache.org>

HDDS-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 (apache#9813)

b09bcff

Generated-by: Codex (GPT-5.3-Codex, GPT-5.4), Claude Code (Opus 4.8)

HDDS-15637. Container Replica Debugger Tool improvements and minor bu…

d14bc0f

…g fixes (apache#10574)

HDDS-15581. ozone debug replicas chunk-info reports wrong block size …

0d0e0fe

…for EC keys (apache#10592)

HDDS-15733. Fix CompactOMDB showing 'om node: null' without --node-id (…

1a26653

…apache#10684)

HDDS-14577. Handle missing metadata dir when updating container state (…

7a95fdd

…apache#10565).

HDDS-15762. Remove mini-chaos-tests (apache#10682)

21c7219

smengcl and others added 3 commits August 8, 2026 15:23

HDDS-16086. installCheckpoint sets OM transaction info inconsistent w…

951ed5c

…ith the checkpoint index (apache#10943) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

HDDS-13217. Test that snapshot checkpoint content is preserved after …

3b8c05e

…defrag (apache#10951)

HDDS-16066. replicas after re-replication should span at least 2 racks (

80ffb62

apache#10944)

xichen01 reviewed Aug 10, 2026

View reviewed changes

HDDS-15852. Allow CopyObject to the same key when the metadata direct…

cb6f124

…ive is REPLACE (apache#10752).

fmorg-git and others added 2 commits August 10, 2026 11:43

HDDS-16096. Bump Ranger to 2.9.0 (apache#10957)

fce67f6

HDDS-16101. Extract bucket lifecycle operations into dedicated handler (

c8fa8da

apache#10969)

devmadhuu force-pushed the HDDS-11233 branch from ed07a4b to 2921d6e Compare August 10, 2026 07:34

sadanand48 and others added 2 commits August 10, 2026 14:24

HDDS-16076. Separate test class for testInstallSnapshotDuringBootstra…

af3e343

…pping. (apache#10931)

Merge remote-tracking branch 'upstream/master' into HDDS-11233

8decb32

devmadhuu force-pushed the HDDS-11233 branch from c6cc43d to 8decb32 Compare August 10, 2026 09:52

devmadhuu force-pushed the HDDS-11233 branch from 8decb32 to ed07a4b Compare August 10, 2026 15:21

F64116045 marked this pull request as draft August 19, 2026 00:24

HDDS-16045. Move data space checks to NodeManager

1b67400

devmadhuu requested review from ashishkumar50 and devmadhuu August 19, 2026 07:49

F64116045 marked this pull request as ready for review August 19, 2026 15:16

devmadhuu reviewed Aug 20, 2026

View reviewed changes

HDDS-16045. Preserve storage type for pending replica ops

b5ad562

ashishkumar50 reviewed Aug 24, 2026

View reviewed changes

F64116045 marked this pull request as draft August 26, 2026 00:40

		@@ -189,6 +190,14 @@ default int getAllNodeCount() {
		*/
		boolean hasSpaceForNewContainerAllocation(DatanodeID datanodeID);

Uh oh!

Conversation

F64116045 commented Aug 2, 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!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devmadhuu commented Aug 10, 2026

Uh oh!

devmadhuu commented Aug 10, 2026

Uh oh!

F64116045 commented Aug 10, 2026

Uh oh!

devmadhuu commented Aug 10, 2026

Uh oh!

devmadhuu commented Aug 18, 2026

Uh oh!

F64116045 commented Aug 19, 2026

Uh oh!

F64116045 commented Aug 19, 2026

Uh oh!

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

F64116045 commented Aug 2, 2026 •

edited

Loading