Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/750#issuecomment-181350773
I strongly agree with @StephanEwen not to make the distinction between
streaming and batch jobs in the runtime explicit. Even though this might only
be a cosmetic change for this PR but who knows what other people will build on
top of it once it is there. I fear that we won't be able to remove such a flag
once introduced.
Concerning whether all `SourceFunction` should have a `stop` method: I
think it is not a priori clear that all streaming sources are gracefully
stoppable. Take for example the `FlinkKafkaConsumer08`. Currently, the stop
method is implemented by calling `cancel`. However, `cancel` does not guarantee
that the sources stop gracefully. Exceptions might be thrown and the state
might be inconsistent. In fact, with the current implementation, `stop` won't
stop the `FlinkKafkaConsumer08` at all. The `cancel` call only works in
combination with the `TaskCanceler` which interrupts the `Task` thread (in this
case `LegacyFetcher.run`). If the interrupt does not happen, the
`SimpleConsumerThreads` might be stuck for all eternity in the fetch loop.
Furthermore, adding the `stop` method to the `SourceFunction` interface
will be an API breaking change and all users would have to implement `stop`,
henceforth. I think a `StoppableOperator` interface would be more modular and
less restrictive because you have the freedom not to implement a graceful stop
operation.
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
