timmylicheng commented on a change in pull request #678: HDDS-3179 Pipeline 
placement based on Topology does not have fallback
URL: https://github.com/apache/hadoop-ozone/pull/678#discussion_r398621936
 
 

 ##########
 File path: 
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
 ##########
 @@ -43,36 +47,98 @@
   private MockNodeManager nodeManager;
   private OzoneConfiguration conf;
   private PipelinePlacementPolicy placementPolicy;
+  private NetworkTopologyImpl cluster;
   private static final int PIPELINE_PLACEMENT_MAX_NODES_COUNT = 10;
 
+  private List<DatanodeDetails> nodesWithOutRackAwareness = new ArrayList<>();
+  private List<DatanodeDetails> nodesWithRackAwareness = new ArrayList<>();
+
   @Before
   public void init() throws Exception {
-    nodeManager = new MockNodeManager(true,
-        PIPELINE_PLACEMENT_MAX_NODES_COUNT);
+    cluster = initTopology();
+    // start with nodes with rack awareness.
+    nodeManager = new MockNodeManager(cluster, getNodesWithRackAwareness(),
+        false, PIPELINE_PLACEMENT_MAX_NODES_COUNT);
     conf = new OzoneConfiguration();
     conf.setInt(OZONE_DATANODE_PIPELINE_LIMIT, 5);
     placementPolicy = new PipelinePlacementPolicy(
         nodeManager, new PipelineStateManager(), conf);
   }
 
+  private NetworkTopologyImpl initTopology() {
+    NodeSchema[] schemas = new NodeSchema[]
+        {ROOT_SCHEMA, RACK_SCHEMA, LEAF_SCHEMA};
+    NodeSchemaManager.getInstance().init(schemas, true);
+    NetworkTopologyImpl topology =
+        new NetworkTopologyImpl(NodeSchemaManager.getInstance());
+    return topology;
+  }
+
+  private List<DatanodeDetails> getNodesWithRackAwareness() {
+    List<DatanodeDetails> datanodes = new ArrayList<>();
+    for (Node node : NODES) {
+      DatanodeDetails datanode = overwriteLocationInNode(
+          getNodesWithoutRackAwareness(), node);
+      nodesWithRackAwareness.add(datanode);
+      datanodes.add(datanode);
+    }
+    return datanodes;
+  }
+
+  private DatanodeDetails getNodesWithoutRackAwareness() {
+    DatanodeDetails node = MockDatanodeDetails.randomDatanodeDetails();
+    nodesWithOutRackAwareness.add(node);
+    return node;
+  }
+
   @Test
-  public void testChooseNodeBasedOnNetworkTopology() {
-    List<DatanodeDetails> healthyNodes =
-        nodeManager.getNodes(HddsProtos.NodeState.HEALTHY);
-    DatanodeDetails anchor = placementPolicy.chooseNode(healthyNodes);
+  public void testChooseNodeBasedOnNetworkTopology() throws SCMException {
+    DatanodeDetails anchor = 
placementPolicy.chooseNode(nodesWithRackAwareness);
     // anchor should be removed from healthyNodes after being chosen.
-    Assert.assertFalse(healthyNodes.contains(anchor));
+    Assert.assertFalse(nodesWithRackAwareness.contains(anchor));
 
     List<DatanodeDetails> excludedNodes =
         new ArrayList<>(PIPELINE_PLACEMENT_MAX_NODES_COUNT);
     excludedNodes.add(anchor);
     DatanodeDetails nextNode = placementPolicy.chooseNodeFromNetworkTopology(
         nodeManager.getClusterNetworkTopologyMap(), anchor, excludedNodes);
     Assert.assertFalse(excludedNodes.contains(nextNode));
-    // nextNode should not be the same as anchor.
+    // next node should not be the same as anchor.
     Assert.assertTrue(anchor.getUuid() != nextNode.getUuid());
+    // next node should be on the same rack based on topology.
+    Assert.assertEquals(anchor.getNetworkLocation(),
+        nextNode.getNetworkLocation());
   }
 
+  @Test
+  public void testChooseNodeWithSingleNodeRack() throws SCMException {
+    // There is only one node on 3 racks altogether.
+    List<DatanodeDetails> datanodes = new ArrayList<>();
+    for (Node node : SINGLE_NODE_RACK) {
+      DatanodeDetails datanode = overwriteLocationInNode(
+          MockDatanodeDetails.randomDatanodeDetails(), node);
+      datanodes.add(datanode);
+    }
+    MockNodeManager localNodeManager = new MockNodeManager(null, datanodes,
 
 Review comment:
   You are right. I updated this part. Thanks!

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


With regards,
Apache Git Services

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

Reply via email to