Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-25 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2168054616


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   Created a follow up jira - https://issues.apache.org/jira/browse/HDDS-13338



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-24 Thread via GitHub


siddhantsangwan commented on PR #8663:
URL: https://github.com/apache/ozone/pull/8663#issuecomment-3003381855

   > @siddhantsangwan, There is a integration test failure, can you take a look 
at it?
   
   It's unrelated and flaky. Passed in my fork - 
https://github.com/siddhantsangwan/ozone/actions/runs/15845888336


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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-24 Thread via GitHub


siddhantsangwan commented on PR #8663:
URL: https://github.com/apache/ozone/pull/8663#issuecomment-3003384600

   I re-triggered the failing test.


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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-24 Thread via GitHub


nandakumar131 commented on PR #8663:
URL: https://github.com/apache/ozone/pull/8663#issuecomment-3002357584

   @siddhantsangwan, There is a integration test failure, can you take a look 
at it?


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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-24 Thread via GitHub


nandakumar131 commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2165306162


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   @siddhantsangwan, let's do this in a separate PR.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-23 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2161706782


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   There's no handling for `null` in the `SCMClientProtocolServer` code flow 
that calls `allocateContainer`, so there can be a null pointer exception if 
`allocateContainer` returns null. I think this code path is being used for 
benchmarks. Probably better not to make changes that impact this path in this 
pull request.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158459808


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.pipelineHasEnoughSpaceForNewContainer(pipeline, 
containerSize)) {
+allocateContainer(pipeline, owner);
+containerIDs = getContainersForOwner(pipeline, owner);
+  } else {
+LOG.debug("Cannot allocate a new container because pipeline {} 
does not have the required space {}.",

Review Comment:
   Good to know that, thanks!



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


sodonnel commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158391969


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.pipelineHasEnoughSpaceForNewContainer(pipeline, 
containerSize)) {
+allocateContainer(pipeline, owner);
+containerIDs = getContainersForOwner(pipeline, owner);
+  } else {
+LOG.debug("Cannot allocate a new container because pipeline {} 
does not have the required space {}.",

Review Comment:
   Yea if the log parameters are just simple references to be passed, the 
`isDebugEnabled()` is no needed as the logger does it internally.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158459808


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.pipelineHasEnoughSpaceForNewContainer(pipeline, 
containerSize)) {
+allocateContainer(pipeline, owner);
+containerIDs = getContainersForOwner(pipeline, owner);
+  } else {
+LOG.debug("Cannot allocate a new container because pipeline {} 
does not have the required space {}.",

Review Comment:
   Good to know that slf4j with "{}" will delay the string construction, thanks!



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158284209


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   Moving the check to `allocateContainer` causes too many unrelated test 
failures because it is being used in unit tests which don't care about pipeline 
state. I could move it but we'll need to fix those other tests then.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158318181


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   Can fix them by doing something like this, what do you think?
   ```
   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 65b06bfd42..2e30b72888 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,10 @@ private ContainerInfo createContainer(Pipeline 
pipeline, String owner)
  private ContainerInfo allocateContainer(final Pipeline pipeline,
  final String owner)
  throws IOException {
   +if (!pipelineManager.hasEnoughSpace(pipeline, containerSize)) {
   +  return null;
   +}
   +
final long uniqueId = sequenceIdGen.getNextId(CONTAINER_ID);
Preconditions.checkState(uniqueId > 0,
"Cannot allocate container, negative container id" +
   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 baa09cd854..df74709f93 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
   @@ -84,7 +84,7 @@ public class TestContainerManagerImpl {
  private SequenceIdGenerator sequenceIdGen;
  private NodeManager nodeManager;
  private ContainerReplicaPendingOps pendingOpsMock;
   -  private PipelineManager pipelineManager;
   +  private MockPipelineManager pipelineManager;

  @BeforeAll
  static void init() {
   @@ -142,6 +142,7 @@ public void 
testGetMatchingContainerReturnsNullWhenNotEnoughSpaceInDatanodes() t
Pipeline pipeline = pipelineManager.getPipelines().iterator().next();
// MockPipelineManager#hasEnoughSpace always returns false
// the pipeline has no existing containers, so a new container gets 
allocated in getMatchingContainer
   +pipelineManager.setHasEnoughSpaceBehavior(false);
ContainerInfo container = containerManager
.getMatchingContainer(sizeRequired, "test", pipeline, 
Collections.emptySet());
assertNull(container);
   @@ -159,9 +160,9 @@ public void 
testGetMatchingContainerReturnsContainerWhenEnoughSpaceInDatanodes()
long sizeRequired = 256 * 1024 * 1024; // 256 MB

// create a spy to mock hasEnoughSpace to always return true
   -PipelineManager spyPipelineManager = spy(pipelineManager);
   -doReturn(true).when(spyPipelineManager)
   -.hasEnoughSpace(any(Pipeline.class), anyLong());
   +PipelineManager spyPipelineManager = pipelineManager;
   +//doReturn(true).when(spyPipelineManager)
   +//.hasEnoughSpace(any(Pipeline.class), anyLong());

// create a new ContainerManager using the spy
File tempDir = new File(testDir, "tempDir");
   diff --git 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
   index 78f1865d2b..3274de3d8a 100644
   --- 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
   +++ 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
   @@ -48,6 +48,7 @@
public class MockPipelineManager implements PipelineManager {

  private final PipelineStateManager stateManager;
   +  private boolean hasEnoughSpaceBehavior = true;

  public MockPipelineManager(DBStore dbStore, SCMHAManager scmhaManager,
 NodeManager nodeManager) throws IOException {
   @@ -352,6 +353,10 @@ public boolean isPipelineCreationFrozen() {

  @Override
  public boolean hasEnoughSpace(Pipeline pipeline, long containerSize) {
   -return 

Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


nandakumar131 commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158213817


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.hasEnoughSpace(pipeline, containerSize)) {

Review Comment:
   Can we move this check to `allocateContainer` method?



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-20 Thread via GitHub


siddhantsangwan commented on PR #8663:
URL: https://github.com/apache/ozone/pull/8663#issuecomment-2990032046

   @ChenSammi thanks for the review. Addressed your comments in the latest 
commit.


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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158186845


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/WritableECContainerProvider.java:
##
@@ -203,8 +203,14 @@ private ContainerInfo allocateContainer(ReplicationConfig 
repConfig,
 
 Pipeline newPipeline = pipelineManager.createPipeline(repConfig,
 excludedNodes, Collections.emptyList());
+// the returned ContainerInfo should not be null (due to not enough space 
in the Datanodes specifically) because

Review Comment:
   Good point. I checked, pipeline will not be null. There's a 
`checkPipeline(pipeline)` method that checks for null and throws an exception 
instead.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


siddhantsangwan commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2158146315


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.pipelineHasEnoughSpaceForNewContainer(pipeline, 
containerSize)) {
+allocateContainer(pipeline, owner);
+containerIDs = getContainersForOwner(pipeline, owner);
+  } else {
+LOG.debug("Cannot allocate a new container because pipeline {} 
does not have the required space {}.",

Review Comment:
   AFAIK the `LOG.isDebugEnabled` check isn't needed when using slf4j with "{}" 
style logging. The log string is evaluated after the debug method itself calls 
`isDebugEnabled()`.



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2157949698


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -346,14 +352,24 @@ public ContainerInfo getMatchingContainer(final long 
size, final String owner,
   synchronized (pipeline.getId()) {
 containerIDs = getContainersForOwner(pipeline, owner);
 if (containerIDs.size() < getOpenContainerCountPerPipeline(pipeline)) {
-  allocateContainer(pipeline, owner);
-  containerIDs = getContainersForOwner(pipeline, owner);
+  if (pipelineManager.pipelineHasEnoughSpaceForNewContainer(pipeline, 
containerSize)) {
+allocateContainer(pipeline, owner);
+containerIDs = getContainersForOwner(pipeline, owner);
+  } else {
+LOG.debug("Cannot allocate a new container because pipeline {} 
does not have the required space {}.",

Review Comment:
   Please add the if (LOG.isDebugEnabled) check. 



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2157952509


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManager.java:
##
@@ -191,7 +191,8 @@ default ContainerInfo getMatchingContainer(long size, 
String owner,
* @param owner - the user which requires space in its owned container
* @param pipeline - pipeline to which the container should belong.
* @param excludedContainerIDS - containerIds to be excluded.
-   * @return ContainerInfo for the matching container.
+   * @return ContainerInfo for the matching container, or null if a container 
could not be found and could not be
+   * allocated

Review Comment:
   Use  @Nullable, which is very straightforward.  



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2157948453


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java:
##
@@ -216,4 +216,13 @@ void reinitialize(Table 
pipelineStore)
* Release write lock.
*/
   void releaseWriteLock();
+
+  /**
+   * Checks whether all Datanodes in the specified pipeline have greater than 
the specified space, containerSize.
+   * @param pipeline pipeline to check
+   * @param containerSize the required amount of space
+   * @return false if all the volumes on any Datanode in the pipeline have 
space less than equal to the specified
+   * containerSize, otherwise true
+   */
+  boolean pipelineHasEnoughSpaceForNewContainer(Pipeline pipeline, long 
containerSize);

Review Comment:
   pipelineHasEnoughSpaceForNewContainer -> HasEnoughSpace



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



Re: [PR] HDDS-12468. Check for space availability for all dns while container creation in pipeline [ozone]

2025-06-19 Thread via GitHub


ChenSammi commented on code in PR #8663:
URL: https://github.com/apache/ozone/pull/8663#discussion_r2157946958


##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/WritableECContainerProvider.java:
##
@@ -203,8 +203,14 @@ private ContainerInfo allocateContainer(ReplicationConfig 
repConfig,
 
 Pipeline newPipeline = pipelineManager.createPipeline(repConfig,
 excludedNodes, Collections.emptyList());
+// the returned ContainerInfo should not be null (due to not enough space 
in the Datanodes specifically) because

Review Comment:
   Could newPipeline be null when DN is close to full? 



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