You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/02/28 06:45:21 UTC

[GitHub] [flink] xintongsong commented on a change in pull request #11248: [FLINK-16299] Release containers recovered from previous attempt in w…

xintongsong commented on a change in pull request #11248: [FLINK-16299] Release containers recovered from previous attempt in w…
URL: https://github.com/apache/flink/pull/11248#discussion_r385530896
 
 

 ##########
 File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
 ##########
 @@ -528,6 +558,31 @@ public void testOnStartContainerError() throws Exception {
 		}};
 	}
 
+	@Test
+	public void testStartWithContainerFromPreviousAttempt() throws Exception {
+		new Context() {{
+			runTest(() -> {
+				Container runningCountainer = mockContainer("container", 1234, 1, Resource.newInstance(1024, 1));
+				Container newContainer = mockContainer("container", 1234, 2, Resource.newInstance(1024, 1));
+				resourceManager.getWorkerNodeMap().put(new ResourceID(runningCountainer.getId().toString()), new YarnWorkerNode(runningCountainer));
+				resourceManager.getWorkerNodeMap().put(new ResourceID(newContainer.getId().toString()), new YarnWorkerNode(newContainer));
+				testingNMClientAsync.containerStatuses.add(
+					ContainerStatus.newInstance(runningCountainer.getId(), ContainerState.RUNNING, "", 0));
+				testingNMClientAsync.containerStatuses.add(
+					ContainerStatus.newInstance(newContainer.getId(), ContainerState.NEW, "", 0));
+
+				CompletableFuture<?> requestContainerStatusFuture = resourceManager.runInMainThread(() -> {
+					testingNMClientAsync.getContainerStatusAsync(runningCountainer.getId(), runningCountainer.getNodeId());
+					testingNMClientAsync.getContainerStatusAsync(newContainer.getId(), newContainer.getNodeId());
+					return null;
+				});
 
 Review comment:
   I think we can make `getContainersFromPreviousAttempts` visible for testing, and call it on the main thread with a custom `RegisterApplicationMasterResponse`.
   
   The purpose of this test case is to test `YarnResourceManager` properly handles recovered containers, including querying their status from Yarn NM and handles them according to the received status. Currently, only covers second part of the workflow. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services