sodonnel commented on a change in pull request #3158:
URL: https://github.com/apache/ozone/pull/3158#discussion_r819491349
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -84,40 +85,28 @@ public PipelinePlacementPolicy(final NodeManager
nodeManager,
this.heavyNodeCriteria = dnLimit == null ? 0 : Integer.parseInt(dnLimit);
}
- int currentPipelineCount(DatanodeDetails datanodeDetails, int nodesRequired)
{
-
- // Datanodes from pipeline in some states can also be considered available
- // for pipeline allocation. Thus the number of these pipeline shall be
- // deducted from total heaviness calculation.
- int pipelineNumDeductable = 0;
- Set<PipelineID> pipelines = nodeManager.getPipelines(datanodeDetails);
- for (PipelineID pid : pipelines) {
- Pipeline pipeline;
- try {
- pipeline = stateManager.getPipeline(pid);
- } catch (PipelineNotFoundException e) {
- LOG.debug("Pipeline not found in pipeline state manager during" +
- " pipeline creation. PipelineID: {}", pid, e);
- continue;
- }
- if (pipeline != null &&
- // single node pipeline are not accounted for while determining
- // the pipeline limit for dn
- pipeline.getType() == HddsProtos.ReplicationType.RATIS &&
- (RatisReplicationConfig
- .hasFactor(pipeline.getReplicationConfig(),
ReplicationFactor.ONE)
- ||
- pipeline.getReplicationConfig().getRequiredNodes()
- == nodesRequired &&
- pipeline.getPipelineState()
- == Pipeline.PipelineState.CLOSED)) {
- pipelineNumDeductable++;
- }
- }
- return pipelines.size() - pipelineNumDeductable;
+ int currentRatisThreePipelineCount(DatanodeDetails datanodeDetails) {
+ // Safe to cast collection's size to int
+ return (int) nodeManager.getPipelines(datanodeDetails).stream()
+ .map(id -> {
+ try {
+ return stateManager.getPipeline(id);
+ } catch (PipelineNotFoundException e) {
+ LOG.debug("Pipeline not found in pipeline state manager during" +
+ " pipeline creation. PipelineID: {}", id, e);
+ return null;
+ }
+ })
+ .filter(this::isNonClosedRatisThreePipeline)
+ .count();
}
-
+ private boolean isNonClosedRatisThreePipeline(Pipeline p) {
+ return p.getType() == RATIS
Review comment:
I think this could be simplified to:
```
p.getReplicationConfig().equals(new RatisReplicationConfig(THREE)) &&
!p.isClosed()
```
And you could create a `private static final RATIS_THREE_CONFIG = new
RatisReplicationConfig(THREE)` and use the constant in the above check to avoid
creating a new object each time.
--
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]
