This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new acbb4395439 HDDS-13338. Move check for sufficient space in pipeline to
allocateContainer (#9568)
acbb4395439 is described below
commit acbb4395439be6283f17315167290aedb8fcf5ea
Author: Russole <[email protected]>
AuthorDate: Thu Jan 15 18:33:58 2026 +0800
HDDS-13338. Move check for sufficient space in pipeline to
allocateContainer (#9568)
---
.../hdds/scm/container/ContainerManagerImpl.java | 21 ++++++++++-----------
.../scm/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 6e5859f27b1..dc701a0be66 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
@@ -237,6 +237,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" +
@@ -350,24 +356,17 @@ public ContainerInfo getMatchingContainer(final long
size, final String owner,
synchronized (pipeline.getId()) {
containerIDs = getContainersForOwner(pipeline, owner);
if (containerIDs.size() <
pipelineManager.openContainerLimit(pipeline.getNodes())) {
- 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..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
@@ -24,7 +24,12 @@
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.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;
@@ -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 ==
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
