sodonnel commented on a change in pull request #2300:
URL: https://github.com/apache/ozone/pull/2300#discussion_r645443897
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
##########
@@ -174,95 +176,19 @@ public AllocatedBlock allocateBlock(final long size,
INVALID_BLOCK_SIZE);
}
- /*
- Here is the high level logic.
-
- 1. We try to find pipelines in open state.
-
- 2. If there are no pipelines in OPEN state, then we try to create one.
-
- 3. We allocate a block from the available containers in the selected
- pipeline.
-
- TODO : #CLUTIL Support random picking of two containers from the list.
- So we can use different kind of policies.
- */
-
- ContainerInfo containerInfo;
-
- //TODO we need to continue the refactor to use ReplicationConfig everywhere
- //in downstream managers.
-
- while (true) {
- List<Pipeline> availablePipelines =
- pipelineManager
- .getPipelines(replicationConfig, Pipeline.PipelineState.OPEN,
- excludeList.getDatanodes(), excludeList.getPipelineIds());
- Pipeline pipeline = null;
- if (availablePipelines.size() == 0 && !excludeList.isEmpty()) {
- // if no pipelines can be found, try finding pipeline without
- // exclusion
- availablePipelines = pipelineManager
- .getPipelines(replicationConfig, Pipeline.PipelineState.OPEN);
- }
- if (availablePipelines.size() == 0) {
- try {
- // TODO: #CLUTIL Remove creation logic when all replication types and
- // factors are handled by pipeline creator
- pipeline = pipelineManager.createPipeline(replicationConfig);
-
- // wait until pipeline is ready
- pipelineManager.waitPipelineReady(pipeline.getId(), 0);
- } catch (SCMException se) {
- LOG.warn("Pipeline creation failed for replicationConfig {} " +
- "Datanodes may be used up.", replicationConfig, se);
- break;
- } catch (IOException e) {
- LOG.warn("Pipeline creation failed for replicationConfig: {}. "
- + "Retrying get pipelines call once.", replicationConfig, e);
- availablePipelines = pipelineManager
- .getPipelines(replicationConfig, Pipeline.PipelineState.OPEN,
- excludeList.getDatanodes(), excludeList.getPipelineIds());
- if (availablePipelines.size() == 0 && !excludeList.isEmpty()) {
- // if no pipelines can be found, try finding pipeline without
- // exclusion
- availablePipelines = pipelineManager
- .getPipelines(replicationConfig, Pipeline.PipelineState.OPEN);
- }
- if (availablePipelines.size() == 0) {
- LOG.info(
- "Could not find available pipeline of replicationConfig: {} "
- + "even after retrying",
- replicationConfig);
- break;
- }
- }
- }
-
- if (null == pipeline) {
- PipelineRequestInformation pri =
- PipelineRequestInformation.Builder.getBuilder()
- .setSize(size)
- .build();
- pipeline = pipelineChoosePolicy.choosePipeline(
- availablePipelines, pri);
- }
-
- // look for OPEN containers that match the criteria.
- containerInfo = containerManager.getMatchingContainer(size, owner,
- pipeline, excludeList.getContainerIds());
-
- if (containerInfo != null) {
- return newBlock(containerInfo);
- }
+ ContainerInfo containerInfo = writableContainerFactory.getContainer(
+ size, replicationConfig, owner, excludeList);
+
+ if (containerInfo != null) {
+ return newBlock(containerInfo);
+ } else {
+ // we have tried all strategies we know and but somehow we are not able
Review comment:
Makes sense. I have removed the else clause.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
