pnowojski commented on a change in pull request #10387: [FLINK-14949]
[Runtime/Task] Task cancellation can be stuck against out-of-thread error
URL: https://github.com/apache/flink/pull/10387#discussion_r353582178
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java
##########
@@ -825,6 +828,78 @@ public void testFatalErrorAfterUnInterruptibleInvoke()
throws Exception {
}
}
+ /**
+ * Tests that a fatal error gotten from canceling task is notified.
+ */
+ @Test
+ public void testFatalErrorOnCanceling() throws Exception {
+ final AwaitFatalErrorTaskManagerActions taskManagerActions =
+ new AwaitFatalErrorTaskManagerActions();
+
+ final Configuration config = new Configuration();
+ config.setLong(TaskManagerOptions.TASK_CANCELLATION_INTERVAL,
5);
+ config.setLong(TaskManagerOptions.TASK_CANCELLATION_TIMEOUT,
50);
+
+ final Task task = spy(createTaskBuilder()
+ .setInvokable(InvokableBlockingWithTrigger.class)
+ .setTaskManagerConfig(config)
+ .setTaskManagerActions(taskManagerActions)
+ .build());
+
+
doThrow(OutOfMemoryError.class).when(task).cancelOrFailAndCancelInvokableInternal(eq(ExecutionState.CANCELING),
eq(null));
+
+ try {
+ task.startTaskThread();
+
+ awaitLatch.await();
+
+ task.cancelExecution();
+
+ // wait for the notification of notifyFatalError
+ taskManagerActions.latch.await();
+ } catch (Throwable t) {
+ fail("No exception is expected to be thrown by fatal
error handling");
+ } finally {
+ triggerLatch.trigger();
+ }
+ }
+
+ /**
+ * Tests that a fatal error gotten from externally failing task is
notified.
+ */
+ @Test
+ public void testFatalErrorOnFailingExternally() throws Exception {
Review comment:
> I added the second test just to increase the coverage
~how was it increasing the test coverage? What was the difference between
them?~
edit: oh, I see it now. One was calling `cancelExecution` and another
`failExternally`. Ehhh, since this test is already relaying very heavily on the
details of the implementation, I would say it's fine to remove one of them.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
