pnowojski commented on a change in pull request #16972:
URL: https://github.com/apache/flink/pull/16972#discussion_r695503202
##########
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 would recommend something a bit different:
> Because of that, this method should never cancel any thread that was
created by the Flink runtime itself, like the task thread or or the source
thread running {@link #run(SourceContext)}. If this is needed, Flink will
interrupt those thread on its own.
Additionally we could mention:
> For some sources implementation it might be necessary to interrupt in this
method the threads spawned by the {@link SourceFunction} itself, but that's
not recommended unless very carefully thought through.
But I would personally drop this second part.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
