hwanju 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_r353366281
 
 

 ##########
 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, but I do not think 
the second one is strictly necessary. In general, I try to follow the style of 
the same test class module (TaskTest), which has little factored code and I 
think is better for test codes. I can factor out the common ones, but I would 
just better remove the second test, as long as you agree.

----------------------------------------------------------------
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

Reply via email to