sodonnel commented on code in PR #11861:
URL: https://github.com/apache/ozone/pull/11861#discussion_r4483418186
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##########
@@ -179,8 +180,7 @@ public ContainerInfo allocateContainer(
Pipeline pipeline;
ContainerInfo containerInfo = null;
try {
- pipelines = pipelineManager
- .getPipelines(replicationConfig, Pipeline.PipelineState.OPEN);
+ pipelines = getOpenPipelinesWithRegisteredNodes(replicationConfig);
Review Comment:
I would be concerned about the performance of this this change. In the
current master code, selecting from an open pipeline is just a map_lookup ->
List and then its accessed via a random integer to pick one.
With this change this becomes an iteration over all pipelines with at least
3 nodeManager map lookups per pipeline. There could be 1000's of pipelines and
hence several thousand map lookups and it will scale linearly with the number
of open pipelines.
One simple answer is to assume that a "bad" pipeline should be rare, and
something should remove those bad pipelines in a reasonable time. So a more
efficient algorithm is to select a pipeline as it does now and then check that
only that selected pipeline is "good". If it is return it, otherwise try again
until there are no good pipelines to try.
That adds some overhead, but not nearly as much.
That said, where is this ContainerManager.allocateContainer used verses the
WritableRatisContainerProvider?
If feel this change would be a lot smaller if we removed the open+bad
pipelines from the open list at source.
--
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]
