Myasuka commented on a change in pull request #16582:
URL: https://github.com/apache/flink/pull/16582#discussion_r677299128
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamOperator.java
##########
@@ -43,7 +43,7 @@
* @param <OUT> The output type of the operator
*/
@PublicEvolving
-public interface StreamOperator<OUT> extends CheckpointListener, KeyContext,
Serializable {
+public interface StreamOperator<OUT> extends InternalCheckpointListener,
KeyContext, Serializable {
Review comment:
@pnowojski I think this needs more discussion here.
My previous idea was to add a default no-op in `CheckpointListener` so that
could be public. Roman convinced me that this new interface does not make so
much senses for most users, and could be acted as an internal API just like
what `InternalKvState` did.
However, current PR has to change several places to make the logic more
clean, which makes me to think whether previous `AbstractStreamOperator` did
not welcome such internal semantics here.
The main usage of `notifyCheckpointSubsumed` is for abstract keyed state
backend, we can compare the relationship of `AbstractStreamOperator` with
`AbstractKeyedStateBackend`. If `AbstractStreamOperator` is open for users to
implement and `AbstractKeyedStateBackend` is actually also open for users to
implement their own state backend.
If so, we might treat `notifyCheckpointSubsumed` as public also.
The only left questions are can we add a new method in `@Public` class and
we need to remember to implement it in necessary places.
cc @rkhachatryan
--
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]
