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 comment.
##########
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(
+ node.getAllocatedContainerResource(),
+ Resource.newInstance(300, 3)));
+
+ RMNodeStatusEvent statusEventFromNode2 = getMockRMNodeStatusEvent(null);
+ ContainerStatus newOppContainerStatusFromNode =
mock(ContainerStatus.class);
+ ContainerStatus runningOppContainerStatusFromNode =
+ mock(ContainerStatus.class);
+ doReturn(newOppContainerId).when(newOppContainerStatusFromNode)
+ .getContainerId();
+ doReturn(ContainerState.NEW).when(newOppContainerStatusFromNode)
+ .getState();
+ doReturn(newContainerCapability).when(newOppContainerStatusFromNode)
+ .getCapability();
+ doReturn(ExecutionType.OPPORTUNISTIC)
+ .when(newOppContainerStatusFromNode)
+ .getExecutionType();
+ doReturn(runningOppContainerId).when(runningOppContainerStatusFromNode)
+ .getContainerId();
+ doReturn(ContainerState.RUNNING).when(runningOppContainerStatusFromNode)
+ .getState();
+
doReturn(runningContainerCapability).when(runningOppContainerStatusFromNode)
+ .getCapability();
+ doReturn(ExecutionType.OPPORTUNISTIC)
+ .when(runningOppContainerStatusFromNode)
+ .getExecutionType();
+ doReturn(Arrays.asList(
+ newContainerStatusFromNode, runningContainerStatusFromNode,
+ newOppContainerStatusFromNode, runningOppContainerStatusFromNode))
+ .when(statusEventFromNode2).getContainers();
+
+ node.handle(statusEventFromNode2);
+ Assert.assertTrue(Resources.equals(
Review comment:
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);
Review comment:
Can we create this mock in a static method to make the test easier to
read?
##########
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));
Review comment:
Can we also assert for a couple more points?
Including when starts empty.
--
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]
