[GitHub] [hadoop] goiri commented on a change in pull request #3646: YARN-11003. Make RMNode aware of all (OContainer inclusive) allocated resources
goiri commented on a change in pull request #3646:
URL: https://github.com/apache/hadoop/pull/3646#discussion_r749613001
##
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMNodeTransitions.java
##
@@ -358,6 +375,119 @@ public void testContainerUpdate() throws
InterruptedException{
.getContainerId());
}
+ /**
+ * Tests that allocated container resources are counted correctly in
+ * {@link org.apache.hadoop.yarn.server.resourcemanager.rmnode.RMNode}
+ * upon a node update. Resources should be counted for both GUARANTEED
+ * and OPPORTUNISTIC containers.
+ */
+ @Test (timeout = 5000)
+ public void testAllocatedContainerUpdate() {
+NodeStatus mockNodeStatus = createMockNodeStatus();
+//Start the node
+node.handle(new RMNodeStartedEvent(null, null, null, mockNodeStatus));
+
+// Make sure that the node starts with no allocated resources
+Assert.assertEquals(node.getAllocatedContainerResource(),
Resources.none());
+
+ApplicationId app0 = BuilderUtils.newApplicationId(0, 0);
+final ContainerId newContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 0);
+final ContainerId runningContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 1);
+
+rmContext.getRMApps().put(app0, Mockito.mock(RMApp.class));
+
+RMNodeStatusEvent statusEventFromNode1 = getMockRMNodeStatusEvent(null);
+
+final List containerStatuses = new ArrayList<>();
+
+// Use different memory and VCores for new and running state containers
+// to test that they add up correctly
+final Resource newContainerCapability =
+Resource.newInstance(100, 1);
+final Resource runningContainerCapability =
+Resource.newInstance(200, 2);
+final Resource completedContainerCapability =
+Resource.newInstance(50, 3);
+final ContainerStatus newContainerStatusFromNode = getMockContainerStatus(
+newContainerId, newContainerCapability, ContainerState.NEW);
+final ContainerStatus runningContainerStatusFromNode =
+getMockContainerStatus(runningContainerId, runningContainerCapability,
+ContainerState.RUNNING);
+
+containerStatuses.addAll(Arrays.asList(
+newContainerStatusFromNode, runningContainerStatusFromNode));
+doReturn(containerStatuses).when(statusEventFromNode1).getContainers();
+node.handle(statusEventFromNode1);
+Assert.assertEquals(node.getAllocatedContainerResource(),
+Resource.newInstance(300, 3));
+
+final ContainerId newOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 2);
+final ContainerId runningOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 3);
+
+// Use the same resource capability as in previous for opportunistic case
+RMNodeStatusEvent statusEventFromNode2 = getMockRMNodeStatusEvent(null);
+final ContainerStatus newOppContainerStatusFromNode =
+getMockContainerStatus(newOppContainerId, newContainerCapability,
+ContainerState.NEW, ExecutionType.OPPORTUNISTIC);
+final ContainerStatus runningOppContainerStatusFromNode =
+getMockContainerStatus(runningOppContainerId,
+runningContainerCapability, ContainerState.RUNNING,
+ExecutionType.OPPORTUNISTIC);
+
+containerStatuses.addAll(Arrays.asList(
+newOppContainerStatusFromNode, runningOppContainerStatusFromNode));
+
+// Pass in both guaranteed and opportunistic container statuses
+doReturn(containerStatuses).when(statusEventFromNode2).getContainers();
+
+node.handle(statusEventFromNode2);
+
+// The result here should be double the first check,
+// since allocated resources are doubled, just
+// with different execution types
+Assert.assertEquals(node.getAllocatedContainerResource(),
+Resource.newInstance(600, 6));
+
+RMNodeStatusEvent statusEventFromNode3 = getMockRMNodeStatusEvent(null);
+final ContainerId completedContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 4);
+final ContainerId completedOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 5);
+final ContainerStatus completedContainerStatusFromNode =
+getMockContainerStatus(completedContainerId,
completedContainerCapability,
+ContainerState.COMPLETE, ExecutionType.OPPORTUNISTIC);
+final ContainerStatus completedOppContainerStatusFromNode =
+getMockContainerStatus(completedOppContainerId,
+completedContainerCapability, ContainerState.COMPLETE,
+ExecutionType.OPPORTUNISTIC);
+
+containerStatuses.addAll(Arrays.asList(
+completedContainerStatusFromNode,
[GitHub] [hadoop] goiri commented on a change in pull request #3646: YARN-11003. Make RMNode aware of all (OContainer inclusive) allocated resources
goiri commented on a change in pull request #3646:
URL: https://github.com/apache/hadoop/pull/3646#discussion_r747780345
##
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMNodeTransitions.java
##
@@ -358,6 +360,94 @@ public void testContainerUpdate() throws
InterruptedException{
.getContainerId());
}
+ /**
+ * Tests that allocated container resources are counted correctly in
+ * {@link org.apache.hadoop.yarn.server.resourcemanager.rmnode.RMNode}
+ * upon a node update. Resources should be counted for both GUARANTEED
+ * and OPPORTUNISTIC containers.
+ */
+ @Test (timeout = 5000)
+ public void testAllocatedContainerUpdate() {
+NodeStatus mockNodeStatus = createMockNodeStatus();
+//Start the node
+node.handle(new RMNodeStartedEvent(null, null, null, mockNodeStatus));
+
+NodeId nodeId = BuilderUtils.newNodeId("localhost:1", 1);
+
+ApplicationId app0 = BuilderUtils.newApplicationId(0, 0);
+ContainerId newContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 0);
+ContainerId runningContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 1);
+ContainerId newOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 2);
+ContainerId runningOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 3);
+
+rmContext.getRMApps().put(app0, Mockito.mock(RMApp.class));
+
+RMNodeStatusEvent statusEventFromNode1 = getMockRMNodeStatusEvent(null);
+ContainerStatus newContainerStatusFromNode = mock(ContainerStatus.class);
+ContainerStatus runningContainerStatusFromNode =
+mock(ContainerStatus.class);
+
+final Resource newContainerCapability =
+Resource.newInstance(100, 1);
+final Resource runningContainerCapability =
+Resource.newInstance(200, 2);
+doReturn(newContainerId).when(newContainerStatusFromNode)
+.getContainerId();
+doReturn(ContainerState.NEW).when(newContainerStatusFromNode)
+.getState();
+doReturn(newContainerCapability).when(newContainerStatusFromNode)
+.getCapability();
+doReturn(runningContainerId).when(runningContainerStatusFromNode)
+.getContainerId();
+doReturn(ContainerState.RUNNING).when(runningContainerStatusFromNode)
+.getState();
+doReturn(runningContainerCapability).when(runningContainerStatusFromNode)
+.getCapability();
+doReturn(Arrays.asList(
+newContainerStatusFromNode, runningContainerStatusFromNode))
+.when(statusEventFromNode1).getContainers();
+node.handle(statusEventFromNode1);
+Assert.assertTrue(Resources.equals(
Review comment:
Resource has equals which actual is called by this, can't we just use
assertEquals()?
##
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMNodeTransitions.java
##
@@ -358,6 +360,94 @@ public void testContainerUpdate() throws
InterruptedException{
.getContainerId());
}
+ /**
+ * Tests that allocated container resources are counted correctly in
+ * {@link org.apache.hadoop.yarn.server.resourcemanager.rmnode.RMNode}
+ * upon a node update. Resources should be counted for both GUARANTEED
+ * and OPPORTUNISTIC containers.
+ */
+ @Test (timeout = 5000)
+ public void testAllocatedContainerUpdate() {
+NodeStatus mockNodeStatus = createMockNodeStatus();
+//Start the node
+node.handle(new RMNodeStartedEvent(null, null, null, mockNodeStatus));
+
+NodeId nodeId = BuilderUtils.newNodeId("localhost:1", 1);
+
+ApplicationId app0 = BuilderUtils.newApplicationId(0, 0);
+ContainerId newContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 0);
+ContainerId runningContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 1);
+ContainerId newOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 2);
+ContainerId runningOppContainerId = BuilderUtils.newContainerId(
+BuilderUtils.newApplicationAttemptId(app0, 0), 3);
+
+rmContext.getRMApps().put(app0, Mockito.mock(RMApp.class));
+
+RMNodeStatusEvent statusEventFromNode1 = getMockRMNodeStatusEvent(null);
+ContainerStatus newContainerStatusFromNode = mock(ContainerStatus.class);
+ContainerStatus runningContainerStatusFromNode =
+mock(ContainerStatus.class);
+
+final Resource newContainerCapability =
+Resource.newInstance(100, 1);
Review comment:
It would be nice to have something explaining this values; either a
constant, a variable name or a
