szetszwo commented on code in PR #9621:
URL: https://github.com/apache/ozone/pull/9621#discussion_r2678966231
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##########
@@ -387,6 +387,10 @@ private int getOpenContainerCountPerPipeline(Pipeline
pipeline) {
int minContainerCountPerDn = numContainerPerVolume *
pipelineManager.minHealthyVolumeNum(pipeline);
int minPipelineCountPerDn = pipelineManager.minPipelineLimit(pipeline);
+ // Defensive guard: avoid division by zero
+ if (minPipelineCountPerDn <= 0) {
+ minPipelineCountPerDn = 1;
+ }
Review Comment:
The calculation of this getOpenContainerCountPerPipeline(..) method actually
is incorrect -- it should not divide two mins: minContainerCountPerDn /
minPipelineCountPerDn.
Example: Suppose numContainerPerVolume = 1.
- dn1 has 4 healthy volume and pipelineLimit = 2, so it allows 2 open
containers
- dn2 has 5 healthy volume and pipelineLimit = 1, so it allows 5 open
containers
Then, they together should allow 2 open containers (i.e. the min of them).
However, using minContainerCountPerDn / minPipelineCountPerDn would allow 4/1 =
4 containers
Let's add a new openContainerLimit(..) method and
- remove this getOpenContainerCountPerPipeline(..) method,
- remove both the minHealthyVolumeNum(..) and minPipelineLimit(..) methods.
--
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]
