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/10/09 14:18:38 UTC
[GitHub] [flink] tillrohrmann commented on a change in pull request #13571: [FLINK-18068] Use FatalErrorHandler to handle the error thrown from r…
tillrohrmann commented on a change in pull request #13571:
URL: https://github.com/apache/flink/pull/13571#discussion_r502425179
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
##########
@@ -359,6 +360,32 @@ ContainerStatus createTestingContainerStatus(final ContainerId containerId) {
}
}
+ @Test
+ public void testRunAsyncCausesFatalError() throws Exception {
+ new Context() {{
+ runTest(() -> {
+ // allocate a new container
+ registerSlotRequest(resourceManager, rmServices, resourceProfile1, taskHost);
+ Container testingContainer = createTestingContainer();
+ resourceManager.onContainersAllocated(ImmutableList.of(testingContainer));
+
+ // let client throw exception when requesting a new container
+ testingYarnAMRMClientAsync.setAddContainerRequestConsumer((ignored1, ignored2) -> {
+ throw new RuntimeException("Expected Exception");
+ });
+
+ // complete the container and request a new one
+ ContainerStatus testingContainerStatus = createTestingContainerStatus(testingContainer.getId());
+ resourceManager.onContainersCompleted(Collections.singletonList(testingContainerStatus));
+
+ Throwable t = testingFatalErrorHandler.getErrorFuture().get(2000L, TimeUnit.MILLISECONDS);
+ assertThat(ExceptionUtils.findThrowable(t, IllegalStateException.class).isPresent(), is(true));
Review comment:
Sure that the thrown `RuntimeException` will trigger an` IllegalStateException`?
##########
File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
##########
@@ -359,6 +360,32 @@ ContainerStatus createTestingContainerStatus(final ContainerId containerId) {
}
}
+ @Test
+ public void testRunAsyncCausesFatalError() throws Exception {
+ new Context() {{
+ runTest(() -> {
+ // allocate a new container
+ registerSlotRequest(resourceManager, rmServices, resourceProfile1, taskHost);
+ Container testingContainer = createTestingContainer();
+ resourceManager.onContainersAllocated(ImmutableList.of(testingContainer));
+
+ // let client throw exception when requesting a new container
+ testingYarnAMRMClientAsync.setAddContainerRequestConsumer((ignored1, ignored2) -> {
+ throw new RuntimeException("Expected Exception");
+ });
+
+ // complete the container and request a new one
+ ContainerStatus testingContainerStatus = createTestingContainerStatus(testingContainer.getId());
+ resourceManager.onContainersCompleted(Collections.singletonList(testingContainerStatus));
Review comment:
I think the test passes also w/o these lines. Can we make it explicit where the exception is thrown which is reported via the `TestingFatalErrorHandler`?
----------------------------------------------------------------
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