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_r386034735
########## File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java ########## @@ -321,9 +322,16 @@ void runTest(RunnableWithException testMethod) throws Exception { } } - void verifyContainerHasBeenStarted(Container testingContainer) { + void verifyContainerHasBeenStarted(Container testingContainer, List<ContainerId> startedContainerIds) throws Exception { verify(mockResourceManagerClient, VERIFICATION_TIMEOUT).removeContainerRequest(any(AMRMClient.ContainerRequest.class)); - verify(mockNMClient, VERIFICATION_TIMEOUT).startContainerAsync(eq(testingContainer), any(ContainerLaunchContext.class)); + // Wait the call of + for (int i = 0; i < TIMEOUT.toMilliseconds(); i += 1000) { + if (startedContainerIds.contains(testingContainer.getId())) { + return; + } + Thread.sleep(1000); + } + throw new Exception("The container has not been start before timeout."); Review comment: I think there are various benefits using `CompletableFuture`: - You don't have to handle the thread safety yourself. - You don't have to do the loop and sleep yourself. - Better readability. - Probably better performance, compared to your loop and sleep. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services