kezhuw commented on a change in pull request #15557:
URL: https://github.com/apache/flink/pull/15557#discussion_r614264965



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/operators/coordination/OperatorCoordinator.java
##########
@@ -202,8 +202,7 @@ default void notifyCheckpointAborted(long checkpointId) {}
          * target TaskManager. The future is completed exceptionally if the 
event cannot be sent.
          * That includes situations where the target task is not running.
          */
-        CompletableFuture<Acknowledge> sendEvent(OperatorEvent evt, int 
targetSubtask)
-                throws TaskNotRunningException;
+        CompletableFuture<Acknowledge> sendEvent(OperatorEvent evt, int 
targetSubtask);

Review comment:
       I was thinking whether the signature is mixed with two different 
requirements:
   * `void transferState(OperatorStateEvent evt) throws 
TaskNotRunningException`(throwing is optional but might be useful) Sending 
failure will failover subtask. This actually means that coordinator want to 
transfer part of its state to subtask.
   * ` CompletableFuture<Xyz> sendEvent(OperatorEvent evt, Duration timeout)` 
Send an event with not exactly-once guarantee. It is author's responsibility to 
dealing with failure hence the timeout parameter. The event does not contribute 
or belong to state in either side.
   




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to