chungen0126 commented on code in PR #10865:
URL: https://github.com/apache/ozone/pull/10865#discussion_r3673370395
##########
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;
Review Comment:
Node duplication is only an issue when excludeNode is not empty to begin
with which should be a rare edge case anyway. Moreover, duplicate nodes in the
list won't cause problems in downstream operations. Since subsequent operations
only work with lists, converting from List to Set and back to List feels
redundant.
Unless our downstream choose-policy operations can also work directly with a
Set, there's no need to use a Set here. If we do want to change it, I'd prefer
creating a separate Jira ticket for that rather than including it in this PR.
--
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]