tillrohrmann commented on a change in pull request #9767:
[FLINK-14120][API/Datastream] Fix testImmediateShutdown
URL: https://github.com/apache/flink/pull/9767#discussion_r328470570
##########
File path:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/SystemProcessingTimeServiceTest.java
##########
@@ -154,27 +152,25 @@ public void testImmediateShutdown() throws Exception {
// the task should trigger immediately and should block
until terminated with interruption
timer.registerTimer(System.currentTimeMillis(),
timestamp -> {
synchronized (lock) {
- latch.trigger();
- Thread.sleep(100000000);
+ try {
+ latch.trigger();
+ Thread.sleep(100000000);
+ fail("should be interrupted");
+ } catch (InterruptedException ignored) {
+ }
}
});
latch.await();
timer.shutdownService();
+
+ // can only enter this scope after the task is
interrupted
synchronized (lock) {
assertTrue(timer.isTerminated());
-
- // The shutdownService() may not necessary wait
for active tasks to finish properly.
- // From the ScheduledThreadPoolExecutor Java
docs:
- // There are no guarantees beyond
best-effort attempts to stop processing actively executing tasks.
- // This implementation cancels tasks via
{@link Thread#interrupt}, so any task that
- // fails to respond to interrupts may
never terminate.
- assertThat(errorRef.get(),
is(anyOf(nullValue(), instanceOf(InterruptedException.class))));
Review comment:
I don't understand this change. Shouldn't the `InterruptedException` not be
a problem here because we allow it to be contained in `errorRef` here?
Moreover, according to the JIRA description the problem with the
`InterruptedException` failing the assertion occurs in line 196.
----------------------------------------------------------------
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
