[GitHub] [flink] StephanEwen commented on a change in pull request #16972: [FLINK-23527][core] Clarify semantics of "SourceFunction.cancel()" with respect to thread interruptions

[GitBox <gi...@apache.org>]

StephanEwen commented on a change in pull request #16972:
URL: https://github.com/apache/flink/pull/16972#discussion_r695610359



##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/source/SourceFunction.java
##########
@@ -123,11 +123,23 @@
      * <p>A typical pattern is to have an {@code "volatile boolean isRunning"} flag that is set to
      * {@code false} in this method. That flag is checked in the loop condition.
      *
-     * <p>When a source is canceled, the executing thread will also be interrupted (via {@link
-     * Thread#interrupt()}). The interruption happens strictly after this method has been called, so
-     * any interruption handler can rely on the fact that this method has completed. It is good
-     * practice to make any flags altered by this method "volatile", in order to guarantee the
-     * visibility of the effects of this method to any interruption handler.
+     * <p>In case of an ungraceful shutdown (cancellation of the source operator, possibly for
+     * failover), the executing thread will also be {@link Thread#interrupt() interrupted}) by the
+     * Flink runtime, in order to speed up the cancellation. The interruption happens strictly after
+     * this method has been called, so any interruption handler can rely on the fact that this
+     * method has completed (for example to ignore exceptions that happen after cancellation). Make
+     * any flags altered by this method "volatile" ensures the visibility of the effects of this
+     * method to any interruption handler.
+     *
+     * <p>During graceful shutdown (for example stopping a job with a savepoint), the program must
+     * cleanly exit the {@link #run(SourceContext)} method soon after this method was called. In
+     * particular, no thread interruption must happen (the Flink runtime will not interrupt the
+     * source thread), because this could interfere with the pending record processing and thus not
+     * result in a clean exit of the {@link #run(SourceContext)} method.
+     *
+     * <p>Because of that, we recommend that this cancel method never does any thread interruptions
+     * itself, but that it solely relies on the Flink runtime to interrupt threads in case of
+     * ungraceful cancellation.

Review comment:
       I am not sure about those changes. It is obvious to users what threads are created by the Flink runtime?
   The way it is currently described sounds simple and easy to understand.




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

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org