[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.

[GitBox <gi...@apache.org>]

ChenSammi commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r376760574
 
 

 ##########
 File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
 ##########
 @@ -220,50 +243,57 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
   public List<DatanodeDetails> getResultSet(
       int nodesRequired, List<DatanodeDetails> healthyNodes)
       throws SCMException {
+    if (nodesRequired != HddsProtos.ReplicationFactor.THREE.getNumber()) {
+      throw new SCMException("Nodes required number is not supported: " +
+          nodesRequired, SCMException.ResultCodes.INVALID_CAPACITY);
+    }
+
+    // Assume rack awareness is not enabled.
+    boolean rackAwareness = false;
     List <DatanodeDetails> results = new ArrayList<>(nodesRequired);
     // Since nodes are widely distributed, the results should be selected
     // base on distance in topology, rack awareness and load balancing.
     List<DatanodeDetails> exclude = new ArrayList<>();
     // First choose an anchor nodes randomly
     DatanodeDetails anchor = chooseNode(healthyNodes);
-    if (anchor == null) {
-      LOG.warn("Unable to find healthy node for anchor(first) node." +
-              " Required nodes: {}, Found nodes: {}",
-          nodesRequired, results.size());
-      throw new SCMException("Unable to find required number of nodes.",
+    if (anchor != null) {
+      results.add(anchor);
+      exclude.add(anchor);
+    } else {
+      LOG.warn("Unable to find healthy node for anchor(first) node.");
+      throw new SCMException("Unable to find anchor node.",
           SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
     }
     if (LOG.isDebugEnabled()) {
       LOG.debug("First node chosen: {}", anchor);
     }
 
-    results.add(anchor);
-    exclude.add(anchor);
 
     // Choose the second node on different racks from anchor.
-    DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+    DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
         healthyNodes, exclude,
         nodeManager.getClusterNetworkTopologyMap(), anchor);
-    if (nodeOnDifferentRack == null) {
-      LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
-          "racks that meets the criteria. Required nodes: {}, Found nodes:" +
-          " {}", nodesRequired, results.size());
-      throw new SCMException("Unable to find required number of nodes.",
-          SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+    if (nextNode != null) {
+      // Rack awareness is detected.
+      rackAwareness = true;
+      results.add(nextNode);
+      exclude.add(nextNode);
+    } else {
+      LOG.debug("Pipeline Placement: Unable to find 2nd node on different " +
+          "rack based on rack awareness.");
     }
     if (LOG.isDebugEnabled()) {
 
 Review comment:
   Better to move this if statement into if (nextNode != null).

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org