sodonnel 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_r398601166
##########
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:
This test doesn't reproduce the error if the fix in this PR is removed. I
commented out the fix in PipelinePlacement policy and added back in the old
logic and ran this and it still passed. The reason, is that
PipelinePlacementPolicy did not believe networkTopology was present. Changing
this line as follows makes the test reproduce the problem:
```
MockNodeManager localNodeManager = new MockNodeManager(initTopology(),
datanodes,
false, datanodes.size());
```
Note I added `initTopology()` when constructing the NodeManager.
With that, the test fails without the fix, and passes with the fix in this
PR, so that is good.
----------------------------------------------------------------
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
