[GitHub] flink issue #5394: [FLINK-6571][tests] Catch InterruptedException in StreamS...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5394 I think neither solve the problem. Variant 2 looks identical to what we have in master. Variant 1 only allows interrupts after the task was canceled. According to what @StephanEwen said, if the UDF throws an exception after the task was canceled the exception will be suppressed and should not lead to a test failure. Since the test did fail it thus must've been thrown _before_ the task was cancelled. Given that variant 1 still throws an exception in this case we aren't solving the stability issue. ---
[GitHub] flink issue #5394: [FLINK-6571][tests] Catch InterruptedException in StreamS...
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/5394 What's the state @zentol? Would Stephan's proposal work? ---
[GitHub] flink issue #5394: [FLINK-6571][tests] Catch InterruptedException in StreamS...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5394 I have often handled it like one of the below variants. What do you think about that pattern? ### Variant 1: Handle interruption if still running ```java public void run(SourceContext ctx) throws Exception { while (running) { try { // do stuff Thread.sleep(20); } catch (InterruptedException e) { // restore interruption flag Thread.currentThread().interrupt(); if (running) { throw new FlinkException("interrupted while still running", e); } // else fall through the loop } } ``` ### Variant 2: Simple let InterruptedException bubble out This variant is also fine, because the Task status is set to CANCELED before the interruption, so any exception bubbling out be suppresses. ```java public void run(SourceContext ctx) throws Exception { while (running) { // do stuff // the InterruptedException from here simply fails the execution Thread.sleep(20); } } ``` ---
[GitHub] flink issue #5394: [FLINK-6571][tests] Catch InterruptedException in StreamS...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5394 How about calling `Thread.currentThread().interrupt();` only after having left the loop? ``` public void run(SourceContext ctx) throws Exception { boolean setInterruptFlag = false; while (running) { try { Thread.sleep(20); } catch (InterruptedException ignored) { setInterruptFlag = true; } } if (setInterruptFlag) { Thread.currentThread().interrupt(); } } ``` This should behave like the original proposal, without the hot loop. ---
[GitHub] flink issue #5394: [FLINK-6571][tests] Catch InterruptedException in StreamS...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5394 It may not be a problem in this test, but I wanted to raise that this pattern is a bit dangerous. If the thread ever gets interrupted while 'running' is still true, this goes into a hot loop constantly throwing exceptions: Every time it falls through the loop and attempts to sleep again, it will immediately throw an Interrupted Exception. ---