rkhachatryan commented on a change in pull request #15496:
URL: https://github.com/apache/flink/pull/15496#discussion_r613265233



##########
File path: 
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/StreamTaskTest.java
##########
@@ -2419,4 +2462,48 @@ public void notifyCheckpointComplete(long checkpointId) {
             throw new ExpectedTestException();
         }
     }
+
+    public static class FailedSource extends RichParallelSourceFunction<String>
+            implements CheckpointedFunction {
+
+        private static CountDownLatch runningLatch = null;
+
+        private volatile boolean running;
+
+        public FailedSource() {
+            runningLatch = new CountDownLatch(1);
+        }
+
+        @Override
+        public void open(Configuration parameters) throws Exception {
+            running = true;
+        }
+
+        @Override
+        public void run(SourceContext<String> ctx) throws Exception {
+            runningLatch.countDown();
+            while (running) {
+                LockSupport.parkNanos(1_000);

Review comment:
       I think the fix causes two things to happen:
   1. interrupt the source thread
   1. call `cancel()` (which sets `running` flag to `false` here)
   
   The test currently only verifies the 2nd (because `parkNanos` ignores 
interrupts).
   Can we replace `parkNanos` with something interruptible (like 
`Thread.sleep`) to also test 1st? It would be more efficient (less iterations) 
and readable IMO.
   
   WDYT?




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


Reply via email to