dawidwys commented on a change in pull request #16351:
URL: https://github.com/apache/flink/pull/16351#discussion_r663821036
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/AbstractStreamOperator.java
##########
@@ -322,30 +322,11 @@ protected boolean isUsingCustomRawKeyedState() {
@Override
public void open() throws Exception {}
- /**
- * This method is called after all records have been added to the
operators via the methods
- * {@link OneInputStreamOperator#processElement(StreamRecord)}, or {@link
- * TwoInputStreamOperator#processElement1(StreamRecord)} and {@link
- * TwoInputStreamOperator#processElement2(StreamRecord)}.
- *
- * <p>The method is expected to flush all remaining buffered data.
Exceptions during this
- * flushing of buffered should be propagated, in order to cause the
operation to be recognized
- * asa failed, because the last data items are not processed properly.
- *
- * @throws Exception An exception in this method causes the operator to
fail.
- */
@Override
- public void close() throws Exception {}
+ public void finish() throws Exception {}
- /**
- * This method is called at the very end of the operator's life, both in
the case of a
- * successful completion of the operation, and in the case of a failure
and canceling.
- *
- * <p>This method is expected to make a thorough effort to release all
resources that the
- * operator has acquired.
- */
@Override
- public void dispose() throws Exception {
+ public void close() throws Exception {
Review comment:
That's definitely a good observation. I am a bit hesitant to introducing
the change as it implies changing again the API of the `StreamOperator`.
Underneath the framework calls `StreamOperator#close`. If we wanted to
introduce `closeAndCleanupState` the way you described it that it'd call
`close()`, we'd need to do it in the `StreamOperator`. If we wanted to do it
the other way around and make `AbstractStreamOperator#close` final and call
`abstract closeAndCleanupState` or similar we'd need to change all operators
and most probably all user's operators as it's virtually impossible to
implement an operator without extending one of the `AbstractStreamOperatorV2`.
How about we create a JIRA ticket for that and we try to fix it once we work
making the operator API "more public". There is a desire to expose e.g. the
`MailboxProcessor` and similar features in a better thought through manner.
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/AbstractStreamOperator.java
##########
@@ -322,30 +322,11 @@ protected boolean isUsingCustomRawKeyedState() {
@Override
public void open() throws Exception {}
- /**
- * This method is called after all records have been added to the
operators via the methods
- * {@link OneInputStreamOperator#processElement(StreamRecord)}, or {@link
- * TwoInputStreamOperator#processElement1(StreamRecord)} and {@link
- * TwoInputStreamOperator#processElement2(StreamRecord)}.
- *
- * <p>The method is expected to flush all remaining buffered data.
Exceptions during this
- * flushing of buffered should be propagated, in order to cause the
operation to be recognized
- * asa failed, because the last data items are not processed properly.
- *
- * @throws Exception An exception in this method causes the operator to
fail.
- */
@Override
- public void close() throws Exception {}
+ public void finish() throws Exception {}
- /**
- * This method is called at the very end of the operator's life, both in
the case of a
- * successful completion of the operation, and in the case of a failure
and canceling.
- *
- * <p>This method is expected to make a thorough effort to release all
resources that the
- * operator has acquired.
- */
@Override
- public void dispose() throws Exception {
+ public void close() throws Exception {
Review comment:
Ah, right. Missed that bit :facepalm: Yes, will update those three
operators.
--
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]
