goiri commented on code in PR #4610:
URL: https://github.com/apache/hadoop/pull/4610#discussion_r927834806
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java:
##########
@@ -610,4 +617,100 @@ public void testMergeResourceProfile() {
Assert.assertEquals(3, resource.getVirtualCores());
Assert.assertEquals(3072, resource.getMemorySize());
}
+
+ @Test
+ public void testMergeAttributesToNodesResponse() {
+ // normal response1
+ NodeAttribute gpu =
NodeAttribute.newInstance(NodeAttribute.PREFIX_CENTRALIZED, "GPU",
+ NodeAttributeType.STRING, "nvidia");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map1 = new HashMap<>();
+ List<NodeToAttributeValue> lists1 = new ArrayList<>();
+ NodeToAttributeValue attributeValue1 =
NodeToAttributeValue.newInstance("node1", gpu.getAttributeValue());
+ lists1.add(attributeValue1);
+ map1.put(gpu.getAttributeKey(), lists1);
+ GetAttributesToNodesResponse response1 =
GetAttributesToNodesResponse.newInstance(map1);
+
+ // normal response2
+ NodeAttribute docker =
NodeAttribute.newInstance(NodeAttribute.PREFIX_DISTRIBUTED, "DOCKER",
+ NodeAttributeType.STRING, "docker0");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map2 = new HashMap<>();
+ List<NodeToAttributeValue> lists2 = new ArrayList<>();
+ NodeToAttributeValue attributeValue2 =
NodeToAttributeValue.newInstance("node2", docker.getAttributeValue());
+ lists2.add(attributeValue2);
+ map2.put(docker.getAttributeKey(), lists2);
+ GetAttributesToNodesResponse response2 =
GetAttributesToNodesResponse.newInstance(map2);
+
+ // empty response3
+ GetAttributesToNodesResponse response3 =
GetAttributesToNodesResponse.newInstance(new HashMap<>());
+
+ // null response4
+ GetAttributesToNodesResponse response4 = null;
+
+ List<GetAttributesToNodesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetAttributesToNodesResponse response =
+ RouterYarnClientUtils.mergeAttributesToNodesResponse(responses);
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(2, response.getAttributesToNodes().size());
+
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> attrs =
response.getAttributesToNodes();
+ Assert.assertTrue(findHostnameAndValInMapping("node2", "docker0",
+ attrs.get(docker.getAttributeKey())));
+ }
+
+ @Test
+ public void testMergeClusterNodeAttributesResponse() {
+ // normal response1
+ NodeAttributeInfo nodeAttributeInfo1 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("GPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes1 = new HashSet<>();
+ attributes1.add(nodeAttributeInfo1);
+ GetClusterNodeAttributesResponse response1 =
GetClusterNodeAttributesResponse.newInstance(attributes1);
+
+ // normal response2
+ NodeAttributeInfo nodeAttributeInfo2 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("CPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes2 = new HashSet<>();
+ attributes2.add(nodeAttributeInfo2);
+ GetClusterNodeAttributesResponse response2 =
GetClusterNodeAttributesResponse.newInstance(attributes2);
+
+ // empty response3
+ GetClusterNodeAttributesResponse response3 =
GetClusterNodeAttributesResponse.newInstance(new HashSet<>());
+
+ // null response4
+ GetClusterNodeAttributesResponse response4 = null;
+
+ List<GetClusterNodeAttributesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetClusterNodeAttributesResponse response =
+ RouterYarnClientUtils.mergeClusterNodeAttributesResponse(responses);
+
+ Assert.assertNotNull(response);
+
+ Set<NodeAttributeInfo> nodeAttributeInfos = response.getNodeAttributes();
+ Assert.assertEquals(2, nodeAttributeInfos.size());
+
+ Object[] objectArr = nodeAttributeInfos.toArray();
+ Assert.assertEquals("rm.yarn.io/GPU(STRING)", objectArr[0].toString());
+ Assert.assertEquals("rm.yarn.io/CPU(STRING)", objectArr[1].toString());
+ }
+
+ private boolean findHostnameAndValInMapping(String hostname, String attrVal,
Review Comment:
static
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/FederationClientInterceptor.java:
##########
@@ -197,10 +196,9 @@ public void init(String userName) {
numSubmitRetries =
conf.getInt(YarnConfiguration.ROUTER_CLIENTRM_SUBMIT_RETRY,
- YarnConfiguration.DEFAULT_ROUTER_CLIENTRM_SUBMIT_RETRY);
+ YarnConfiguration.DEFAULT_ROUTER_CLIENTRM_SUBMIT_RETRY);
Review Comment:
The indentation is weird now. Probably the cleanest is:
```
numSubmitRetries = conf.getInt(
YarnConfiguration.ROUTER_CLIENTRM_SUBMIT_RETRY,
YarnConfiguration.DEFAULT_ROUTER_CLIENTRM_SUBMIT_RETRY);
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterYarnClientUtils.java:
##########
@@ -468,5 +474,65 @@ public static GetResourceProfileResponse
mergeClusterResourceProfileResponse(
profileResponse.setResource(resource);
return profileResponse;
}
+
+ /**
+ * Merges a list of GetAttributesToNodesResponse.
+ *
+ * @param responses a list of GetAttributesToNodesResponse to merge.
+ * @return the merged GetAttributesToNodesResponse.
+ */
+ public static GetAttributesToNodesResponse mergeAttributesToNodesResponse(
+ Collection<GetAttributesToNodesResponse> responses) {
+ GetAttributesToNodesResponse attributesToNodesResponse =
+ GetAttributesToNodesResponse.newInstance(new HashMap<>());
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> nodeAttributeMap = new
HashMap<>();
+ for (GetAttributesToNodesResponse response : responses) {
+ if (response != null && response.getAttributesToNodes() != null) {
+ nodeAttributeMap.putAll(response.getAttributesToNodes());
+ }
+ }
+ attributesToNodesResponse.setAttributeToNodes(nodeAttributeMap);
+ return attributesToNodesResponse;
Review Comment:
Can we do something like:
```
return GetAttributesToNodesResponse.newInstance(nodeAttributeMap);
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java:
##########
@@ -610,4 +617,100 @@ public void testMergeResourceProfile() {
Assert.assertEquals(3, resource.getVirtualCores());
Assert.assertEquals(3072, resource.getMemorySize());
}
+
+ @Test
+ public void testMergeAttributesToNodesResponse() {
+ // normal response1
+ NodeAttribute gpu =
NodeAttribute.newInstance(NodeAttribute.PREFIX_CENTRALIZED, "GPU",
+ NodeAttributeType.STRING, "nvidia");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map1 = new HashMap<>();
+ List<NodeToAttributeValue> lists1 = new ArrayList<>();
+ NodeToAttributeValue attributeValue1 =
NodeToAttributeValue.newInstance("node1", gpu.getAttributeValue());
+ lists1.add(attributeValue1);
+ map1.put(gpu.getAttributeKey(), lists1);
+ GetAttributesToNodesResponse response1 =
GetAttributesToNodesResponse.newInstance(map1);
+
+ // normal response2
+ NodeAttribute docker =
NodeAttribute.newInstance(NodeAttribute.PREFIX_DISTRIBUTED, "DOCKER",
+ NodeAttributeType.STRING, "docker0");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map2 = new HashMap<>();
+ List<NodeToAttributeValue> lists2 = new ArrayList<>();
+ NodeToAttributeValue attributeValue2 =
NodeToAttributeValue.newInstance("node2", docker.getAttributeValue());
+ lists2.add(attributeValue2);
+ map2.put(docker.getAttributeKey(), lists2);
+ GetAttributesToNodesResponse response2 =
GetAttributesToNodesResponse.newInstance(map2);
+
+ // empty response3
+ GetAttributesToNodesResponse response3 =
GetAttributesToNodesResponse.newInstance(new HashMap<>());
+
+ // null response4
+ GetAttributesToNodesResponse response4 = null;
+
+ List<GetAttributesToNodesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetAttributesToNodesResponse response =
+ RouterYarnClientUtils.mergeAttributesToNodesResponse(responses);
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(2, response.getAttributesToNodes().size());
+
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> attrs =
response.getAttributesToNodes();
+ Assert.assertTrue(findHostnameAndValInMapping("node2", "docker0",
+ attrs.get(docker.getAttributeKey())));
+ }
+
+ @Test
+ public void testMergeClusterNodeAttributesResponse() {
+ // normal response1
+ NodeAttributeInfo nodeAttributeInfo1 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("GPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes1 = new HashSet<>();
+ attributes1.add(nodeAttributeInfo1);
+ GetClusterNodeAttributesResponse response1 =
GetClusterNodeAttributesResponse.newInstance(attributes1);
+
+ // normal response2
+ NodeAttributeInfo nodeAttributeInfo2 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("CPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes2 = new HashSet<>();
+ attributes2.add(nodeAttributeInfo2);
+ GetClusterNodeAttributesResponse response2 =
GetClusterNodeAttributesResponse.newInstance(attributes2);
+
+ // empty response3
+ GetClusterNodeAttributesResponse response3 =
GetClusterNodeAttributesResponse.newInstance(new HashSet<>());
+
+ // null response4
+ GetClusterNodeAttributesResponse response4 = null;
+
+ List<GetClusterNodeAttributesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetClusterNodeAttributesResponse response =
+ RouterYarnClientUtils.mergeClusterNodeAttributesResponse(responses);
+
+ Assert.assertNotNull(response);
+
+ Set<NodeAttributeInfo> nodeAttributeInfos = response.getNodeAttributes();
+ Assert.assertEquals(2, nodeAttributeInfos.size());
+
+ Object[] objectArr = nodeAttributeInfos.toArray();
Review Comment:
Assert the size of the array here too?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java:
##########
@@ -610,4 +617,100 @@ public void testMergeResourceProfile() {
Assert.assertEquals(3, resource.getVirtualCores());
Assert.assertEquals(3072, resource.getMemorySize());
}
+
+ @Test
+ public void testMergeAttributesToNodesResponse() {
+ // normal response1
+ NodeAttribute gpu =
NodeAttribute.newInstance(NodeAttribute.PREFIX_CENTRALIZED, "GPU",
+ NodeAttributeType.STRING, "nvidia");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map1 = new HashMap<>();
+ List<NodeToAttributeValue> lists1 = new ArrayList<>();
+ NodeToAttributeValue attributeValue1 =
NodeToAttributeValue.newInstance("node1", gpu.getAttributeValue());
+ lists1.add(attributeValue1);
+ map1.put(gpu.getAttributeKey(), lists1);
+ GetAttributesToNodesResponse response1 =
GetAttributesToNodesResponse.newInstance(map1);
+
+ // normal response2
+ NodeAttribute docker =
NodeAttribute.newInstance(NodeAttribute.PREFIX_DISTRIBUTED, "DOCKER",
+ NodeAttributeType.STRING, "docker0");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map2 = new HashMap<>();
+ List<NodeToAttributeValue> lists2 = new ArrayList<>();
+ NodeToAttributeValue attributeValue2 =
NodeToAttributeValue.newInstance("node2", docker.getAttributeValue());
+ lists2.add(attributeValue2);
+ map2.put(docker.getAttributeKey(), lists2);
+ GetAttributesToNodesResponse response2 =
GetAttributesToNodesResponse.newInstance(map2);
+
+ // empty response3
+ GetAttributesToNodesResponse response3 =
GetAttributesToNodesResponse.newInstance(new HashMap<>());
+
+ // null response4
+ GetAttributesToNodesResponse response4 = null;
+
+ List<GetAttributesToNodesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetAttributesToNodesResponse response =
+ RouterYarnClientUtils.mergeAttributesToNodesResponse(responses);
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(2, response.getAttributesToNodes().size());
+
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> attrs =
response.getAttributesToNodes();
+ Assert.assertTrue(findHostnameAndValInMapping("node2", "docker0",
+ attrs.get(docker.getAttributeKey())));
+ }
+
+ @Test
+ public void testMergeClusterNodeAttributesResponse() {
+ // normal response1
+ NodeAttributeInfo nodeAttributeInfo1 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("GPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes1 = new HashSet<>();
+ attributes1.add(nodeAttributeInfo1);
+ GetClusterNodeAttributesResponse response1 =
GetClusterNodeAttributesResponse.newInstance(attributes1);
+
+ // normal response2
+ NodeAttributeInfo nodeAttributeInfo2 =
+ NodeAttributeInfo.newInstance(NodeAttributeKey.newInstance("CPU"),
NodeAttributeType.STRING);
+ Set<NodeAttributeInfo> attributes2 = new HashSet<>();
+ attributes2.add(nodeAttributeInfo2);
+ GetClusterNodeAttributesResponse response2 =
GetClusterNodeAttributesResponse.newInstance(attributes2);
+
+ // empty response3
+ GetClusterNodeAttributesResponse response3 =
GetClusterNodeAttributesResponse.newInstance(new HashSet<>());
+
+ // null response4
+ GetClusterNodeAttributesResponse response4 = null;
+
+ List<GetClusterNodeAttributesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetClusterNodeAttributesResponse response =
+ RouterYarnClientUtils.mergeClusterNodeAttributesResponse(responses);
+
+ Assert.assertNotNull(response);
+
+ Set<NodeAttributeInfo> nodeAttributeInfos = response.getNodeAttributes();
+ Assert.assertEquals(2, nodeAttributeInfos.size());
+
+ Object[] objectArr = nodeAttributeInfos.toArray();
Review Comment:
Actually, this thing about converting a set into an array to then check in a
specific order is not very clean.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java:
##########
@@ -610,4 +617,100 @@ public void testMergeResourceProfile() {
Assert.assertEquals(3, resource.getVirtualCores());
Assert.assertEquals(3072, resource.getMemorySize());
}
+
+ @Test
+ public void testMergeAttributesToNodesResponse() {
+ // normal response1
+ NodeAttribute gpu =
NodeAttribute.newInstance(NodeAttribute.PREFIX_CENTRALIZED, "GPU",
+ NodeAttributeType.STRING, "nvidia");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map1 = new HashMap<>();
+ List<NodeToAttributeValue> lists1 = new ArrayList<>();
+ NodeToAttributeValue attributeValue1 =
NodeToAttributeValue.newInstance("node1", gpu.getAttributeValue());
+ lists1.add(attributeValue1);
+ map1.put(gpu.getAttributeKey(), lists1);
+ GetAttributesToNodesResponse response1 =
GetAttributesToNodesResponse.newInstance(map1);
+
+ // normal response2
+ NodeAttribute docker =
NodeAttribute.newInstance(NodeAttribute.PREFIX_DISTRIBUTED, "DOCKER",
+ NodeAttributeType.STRING, "docker0");
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> map2 = new HashMap<>();
+ List<NodeToAttributeValue> lists2 = new ArrayList<>();
+ NodeToAttributeValue attributeValue2 =
NodeToAttributeValue.newInstance("node2", docker.getAttributeValue());
+ lists2.add(attributeValue2);
+ map2.put(docker.getAttributeKey(), lists2);
+ GetAttributesToNodesResponse response2 =
GetAttributesToNodesResponse.newInstance(map2);
+
+ // empty response3
+ GetAttributesToNodesResponse response3 =
GetAttributesToNodesResponse.newInstance(new HashMap<>());
+
+ // null response4
+ GetAttributesToNodesResponse response4 = null;
+
+ List<GetAttributesToNodesResponse> responses = new ArrayList<>();
+ responses.add(response1);
+ responses.add(response2);
+ responses.add(response3);
+ responses.add(response4);
+
+ GetAttributesToNodesResponse response =
+ RouterYarnClientUtils.mergeAttributesToNodesResponse(responses);
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(2, response.getAttributesToNodes().size());
+
+ Map<NodeAttributeKey, List<NodeToAttributeValue>> attrs =
response.getAttributesToNodes();
+ Assert.assertTrue(findHostnameAndValInMapping("node2", "docker0",
Review Comment:
Not the cleanest. Is there a better way of writing this?
--
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]
