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]

Reply via email to