szetszwo commented on code in PR #10865:
URL: https://github.com/apache/ozone/pull/10865#discussion_r3667689555
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java:
##########
@@ -224,18 +217,39 @@ public Pipeline createForRead(
.build();
}
- private List<DatanodeDetails> filterPipelineEngagement() {
+ private List<DatanodeDetails> chooseThreeFactorDatanodes(
+ List<DatanodeDetails> excludedNodes, List<DatanodeDetails> favoredNodes,
int requiredNode)
+ throws IOException {
final NodeManager nodeManager = getNodeManager();
final PipelineStateManager stateManager = getPipelineStateManager();
final List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
- final List<DatanodeDetails> excluded = new ArrayList<>();
+ excludedNodes = excludedNodes.isEmpty() ? new ArrayList<>() :
excludedNodes;
+ List<DatanodeDetails> strictExcludedNodes = null;
for (DatanodeDetails d : healthyNodes) {
final int count =
PipelinePlacementPolicy.currentRatisThreePipelineCount(nodeManager,
stateManager, d);
if (count >= nodeManager.pipelineLimit(d)) {
- excluded.add(d);
+ excludedNodes.add(d);
+ } else if (!d.hasPort(RATIS_DATASTREAM)) {
+ if (strictExcludedNodes == null) {
+ strictExcludedNodes = new ArrayList<>();
+ }
+ strictExcludedNodes.add(d);
}
}
- return excluded;
+ if (strictExcludedNodes != null) {
+ strictExcludedNodes.addAll(excludedNodes);
+ try {
+ return placementPolicy.chooseDatanodes(strictExcludedNodes,
+ favoredNodes, requiredNode, minRatisVolumeSizeBytes,
+ containerSizeBytes);
+ } catch (SCMException scmException) {
+ LOG.debug("Failed to allocate datanodes with strict exclusion
(non-streaming nodes excluded)." +
+ " Falling back.", scmException);
Review Comment:
- "strictExcludedNodes" may mean that "must be excluded".
Let call it additionalExcludedNodes.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java:
##########
@@ -224,18 +217,39 @@ public Pipeline createForRead(
.build();
}
- private List<DatanodeDetails> filterPipelineEngagement() {
+ private List<DatanodeDetails> chooseThreeFactorDatanodes(
+ List<DatanodeDetails> excludedNodes, List<DatanodeDetails> favoredNodes,
int requiredNode)
+ throws IOException {
final NodeManager nodeManager = getNodeManager();
final PipelineStateManager stateManager = getPipelineStateManager();
final List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
- final List<DatanodeDetails> excluded = new ArrayList<>();
+ excludedNodes = excludedNodes.isEmpty() ? new ArrayList<>() :
excludedNodes;
Review Comment:
Since empty excludedNodes is a common case, let's set it to null first and
create the list when it is needed.
```java
excludedNodes = excludedNodes.isEmpty() ? null : excludedNodes;
List<DatanodeDetails> strictExcludedNodes = null;
for (DatanodeDetails d : healthyNodes) {
final int count =
PipelinePlacementPolicy.currentRatisThreePipelineCount(nodeManager,
stateManager, d);
if (count >= nodeManager.pipelineLimit(d)) {
if (excludedNodes == null) {
excludedNodes = new ArrayList<>();
}
excludedNodes.add(d);
} else if (!d.hasPort(RATIS_DATASTREAM)) {
```
--
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]
