You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/01/06 16:55:35 UTC
[GitHub] [hadoop] brumi1024 commented on a change in pull request #3763: YARN-10850 Fix the application attempt ID query with the correspondin…
brumi1024 commented on a change in pull request #3763:
URL: https://github.com/apache/hadoop/pull/3763#discussion_r779638024
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -248,4 +290,36 @@ private static TimelineEntity createContainerEntity(ContainerId containerId) {
return entity;
}
+
+ private static List<TimelineEntity> createContainerEntities(ContainerId containerId) {
+ List<TimelineEntity> entities = new ArrayList<>();
+
+ TimelineEntity entity = new TimelineEntity();
+ entity.setType(ContainerMetricsConstants.ENTITY_TYPE);
+ entity.setId(containerId.toString());
+ Map<String, Object> entityInfo = new HashMap<String, Object>();
+ entityInfo.put(ContainerMetricsConstants.ALLOCATED_MEMORY_INFO, 1024);
+ entityInfo.put(ContainerMetricsConstants.ALLOCATED_VCORE_INFO, 8);
+ entityInfo.put(ContainerMetricsConstants.ALLOCATED_HOST_INFO,
+ "test host");
+ entityInfo.put(ContainerMetricsConstants.ALLOCATED_PORT_INFO, 100);
+ entityInfo
+ .put(ContainerMetricsConstants.ALLOCATED_PRIORITY_INFO, -1);
+ entityInfo.put(ContainerMetricsConstants
+ .ALLOCATED_HOST_HTTP_ADDRESS_INFO, "http://test:1234");
+ entityInfo.put(ContainerMetricsConstants.DIAGNOSTICS_INFO,
+ "test diagnostics info");
+ entityInfo.put(ContainerMetricsConstants.EXIT_STATUS_INFO, -1);
+ entityInfo.put(ContainerMetricsConstants.STATE_INFO,
+ ContainerState.COMPLETE.toString());
+ entity.setInfo(entityInfo);
+
+ TimelineEvent tEvent = new TimelineEvent();
+ tEvent.setId(ContainerMetricsConstants.CREATED_IN_RM_EVENT_TYPE);
+ tEvent.setTimestamp(123456);
+ entity.addEvent(tEvent);
+
+ entities.add(entity);
+ return entities;
+ }
Review comment:
This looks like the same as createContainerEntity except it wraps the entity in an array. I don't think the method should be duplicated, instead the original method could be called and then wrapped in an array.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +110,45 @@ public void testGetAppAttemptReport() throws IOException, YarnException {
isEqualTo("test original tracking url");
}
+ @Test
+ public void testGetContainerByAppAttempt() throws IOException, YarnException {
+ final ApplicationId appId1 = ApplicationId.newInstance(0, 1);
+ final ApplicationId appId2 = ApplicationId.newInstance(0, 2);
+
+ final ApplicationAttemptId appAttemptId1 =
+ ApplicationAttemptId.newInstance(appId1, 1);
+ final ApplicationAttemptId appAttemptId2 =
+ ApplicationAttemptId.newInstance(appId2, 2);
+
+ final ContainerId containerId1 = ContainerId.newContainerId(appAttemptId1, 1);
+ final ContainerId containerId2 = ContainerId.newContainerId(appAttemptId2, 2);
+
+
+ Map<String, String> filter1 = new HashMap<>();
+ filter1.put("appattemptId", appAttemptId1.toString());
+ Map<String, String> filter2 = new HashMap<>();
+ filter2.put("appattemptId", appAttemptId2.toString());
+ when(spyTimelineReaderClient.getContainerEntities(appId1, "ALL", filter1,
+ 0, null))
+ .thenReturn(createContainerEntities(containerId1));
+ when(spyTimelineReaderClient.getContainerEntities(appId2, "ALL", filter2,
+ 0, null))
+ .thenReturn(createContainerEntities(containerId2));
+
+ when(spyTimelineReaderClient.getApplicationEntity(appId1, "ALL", null))
+ .thenReturn(createApplicationTimelineEntity(appId1, true,
+ false));
+ when(spyTimelineReaderClient.getApplicationEntity(appId2, "ALL", null))
+ .thenReturn(createApplicationTimelineEntity(appId2, true,
+ false));
+
+ List<ContainerReport> containerList1 = client.getContainers(appAttemptId1);
+ List<ContainerReport> containerList2 = client.getContainers(appAttemptId2);
+
+ assertThat(containerList1.size()).isEqualTo(1);
+ assertThat(containerList2.size()).isEqualTo(1);
Review comment:
Can you please extend the checks to see whether the containerList contains the containers with the correct ids?
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +110,45 @@ public void testGetAppAttemptReport() throws IOException, YarnException {
isEqualTo("test original tracking url");
}
+ @Test
+ public void testGetContainerByAppAttempt() throws IOException, YarnException {
+ final ApplicationId appId1 = ApplicationId.newInstance(0, 1);
+ final ApplicationId appId2 = ApplicationId.newInstance(0, 2);
+
+ final ApplicationAttemptId appAttemptId1 =
+ ApplicationAttemptId.newInstance(appId1, 1);
+ final ApplicationAttemptId appAttemptId2 =
+ ApplicationAttemptId.newInstance(appId2, 2);
+
+ final ContainerId containerId1 = ContainerId.newContainerId(appAttemptId1, 1);
+ final ContainerId containerId2 = ContainerId.newContainerId(appAttemptId2, 2);
+
+
+ Map<String, String> filter1 = new HashMap<>();
+ filter1.put("appattemptId", appAttemptId1.toString());
+ Map<String, String> filter2 = new HashMap<>();
+ filter2.put("appattemptId", appAttemptId2.toString());
Review comment:
Nit: Maybe an inline ImmutableMap.of would be a bit cleaner, because the filter maps aren't reused.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +110,45 @@ public void testGetAppAttemptReport() throws IOException, YarnException {
isEqualTo("test original tracking url");
}
+ @Test
+ public void testGetContainerByAppAttempt() throws IOException, YarnException {
+ final ApplicationId appId1 = ApplicationId.newInstance(0, 1);
+ final ApplicationId appId2 = ApplicationId.newInstance(0, 2);
Review comment:
Can you please create an app with multiple attemps and multiple containers? To ensure only the requested appattempt's containers are returned.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org