From 25bd7e8afc08751aa3122d25d70e45809405844b Mon Sep 17 00:00:00 2001 From: Russole <850905junior@gmail.com> Date: Sat, 27 Dec 2025 15:40:58 +0800 Subject: [PATCH 1/2] HDDS-13338. Move check for sufficient space in pipeline to allocateContainer --- .../scm/container/ContainerManagerImpl.java | 21 +++++++++---------- .../container/TestContainerManagerImpl.java | 12 ++++++++++- .../hdds/scm/node/TestContainerPlacement.java | 10 +++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java index d255bc9a672..cae3a6b00e1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java @@ -243,6 +243,12 @@ private ContainerInfo createContainer(Pipeline pipeline, String owner) private ContainerInfo allocateContainer(final Pipeline pipeline, final String owner) throws IOException { + if (!pipelineManager.hasEnoughSpace(pipeline, maxContainerSize)) { + LOG.debug("Cannot allocate a new container because pipeline {} does not have the required space {}.", + pipeline, maxContainerSize); + return null; + } + final long uniqueId = sequenceIdGen.getNextId(CONTAINER_ID); Preconditions.checkState(uniqueId > 0, "Cannot allocate container, negative container id" + @@ -352,24 +358,17 @@ public ContainerInfo getMatchingContainer(final long size, final String owner, synchronized (pipeline.getId()) { containerIDs = getContainersForOwner(pipeline, owner); if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) { - if (pipelineManager.hasEnoughSpace(pipeline, maxContainerSize)) { - allocateContainer(pipeline, owner); + ContainerInfo allocated = allocateContainer(pipeline, owner); + if (allocated != null) { + // New container was created, refresh IDs so it becomes eligible. containerIDs = getContainersForOwner(pipeline, owner); - } else { - LOG.debug("Cannot allocate a new container because pipeline {} does not have the required space {}.", - pipeline, maxContainerSize); } } containerIDs.removeAll(excludedContainerIDs); containerInfo = containerStateManager.getMatchingContainer( size, owner, pipeline.getId(), containerIDs); if (containerInfo == null) { - if (pipelineManager.hasEnoughSpace(pipeline, maxContainerSize)) { - containerInfo = allocateContainer(pipeline, owner); - } else { - LOG.debug("Cannot allocate a new container because pipeline {} does not have the required space {}.", - pipeline, maxContainerSize); - } + containerInfo = allocateContainer(pipeline, owner); } return containerInfo; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerManagerImpl.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerManagerImpl.java index dd5edf38193..daf0b4b4c6a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerManagerImpl.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerManagerImpl.java @@ -99,9 +99,15 @@ void setUp() throws Exception { NodeManager nodeManager = new MockNodeManager(true, 10); sequenceIdGen = new SequenceIdGenerator( conf, scmhaManager, SCMDBDefinition.SEQUENCE_ID.getTable(dbStore)); - pipelineManager = new MockPipelineManager(dbStore, scmhaManager, nodeManager); + PipelineManager base = new MockPipelineManager(dbStore, scmhaManager, nodeManager); + pipelineManager = spy(base); + + // Default: allow allocation in tests unless a test overrides it. + doReturn(true).when(pipelineManager).hasEnoughSpace(any(Pipeline.class), anyLong()); + pipelineManager.createPipeline(RatisReplicationConfig.getInstance( ReplicationFactor.THREE)); + pendingOpsMock = mock(ContainerReplicaPendingOps.class); containerManager = new ContainerManagerImpl(conf, scmhaManager, sequenceIdGen, pipelineManager, @@ -122,6 +128,8 @@ void testAllocateContainer() throws Exception { final ContainerInfo container = containerManager.allocateContainer( RatisReplicationConfig.getInstance( ReplicationFactor.THREE), "admin"); + + assertNotNull(container); assertEquals(1, containerManager.getContainers().size()); assertNotNull(containerManager.getContainer( container.containerID())); @@ -133,6 +141,8 @@ void testAllocateContainer() throws Exception { */ @Test public void testGetMatchingContainerReturnsNullWhenNotEnoughSpaceInDatanodes() throws IOException { + doReturn(false).when(pipelineManager).hasEnoughSpace(any(), anyLong()); + long sizeRequired = 256 * 1024 * 1024; // 256 MB Pipeline pipeline = pipelineManager.getPipelines().iterator().next(); // MockPipelineManager#hasEnoughSpace always returns false diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java index 90301d6fccd..fb9eeec8633 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java @@ -24,8 +24,13 @@ import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT_SCHEMA; import static org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager.maxLayoutVersion; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; import java.io.File; import java.io.IOException; @@ -154,6 +159,9 @@ SCMNodeManager createNodeManager(OzoneConfiguration config) { ContainerManager createContainerManager() throws IOException { + pipelineManager = spy(pipelineManager); + doReturn(true).when(pipelineManager).hasEnoughSpace(any(), anyLong()); + return new ContainerManagerImpl(conf, scmhaManager, sequenceIdGen, pipelineManager, SCMDBDefinition.CONTAINERS.getTable(dbStore), @@ -224,6 +232,8 @@ public void testContainerPlacementCapacity() throws IOException, SCMTestUtils.getReplicationType(conf), SCMTestUtils.getReplicationFactor(conf)), OzoneConsts.OZONE); + assertNotNull(container, "allocateContainer returned null (unexpected in this placement test)"); + int replicaCount = 0; for (DatanodeDetails datanodeDetails : datanodes) { if (replicaCount == From 41ba50864864ab508ad8ec4d90734897ddd2463d Mon Sep 17 00:00:00 2001 From: Russole <850905junior@gmail.com> Date: Sun, 28 Dec 2025 09:27:07 +0800 Subject: [PATCH 2/2] HDDS-13338. Fix CheckStyle Error --- .../apache/hadoop/hdds/scm/node/TestContainerPlacement.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java index fb9eeec8633..9b7c5c77b2c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java @@ -25,12 +25,12 @@ import static org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager.maxLayoutVersion; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException;