gaoyunhaii edited a comment on pull request #16432:
URL: https://github.com/apache/flink/pull/16432#issuecomment-877487310
Very thanks @tillrohrmann for the review and very thanks @StephanEwen for
the detail investigation and analysis!
I also agree with chaining the futures as @tillrohrmann suggested would be
correct and simpler . I'll update the PR according to this method very soon.
And for clarification, for the original method that used
```
sendingExecutor.execute(
() -> {
nonSuccessFuturesTrack.trackFuture(result);
sender.sendEvent(sendAction, result);
});
```
my understanding is that the order is ensured since it all happen inside the
main thread. For detail:
Based on the current implementation, actual event sending (namely
`sendingExecutor`) and
`OperatorCoordinatorHolder#checkpointCoordinatorInternal` are all happens in
the main thread. Then for all the events during checkpointing
1. `OperatorCoordinator#checkpointCoordinator`, happen in main thread, the
implementation may proxy it to the user thread.
2. submit `completeCheckpointOnceEventsAreDone` to the main thread, happen
in user thread.
3. actually execute `completeCheckpointOnceEventsAreDone`, happen in main
thread.
First with my understand now we should request the implementation of
`OperatorCoordinator` to ensure there is not event sending request between 1
and 2, otherwise both methods would have problem. Then for an event sending
request, it either happen before 1 and submit a Runnable to the main thread
before 3, or happen after 2 and submit a Runnable to the main thread after 3:
1. For the first case, `trackFuture` would happen before
`completeCheckpointOnceEventsAreDone` and considered.
2. For the second case, `trackFuture` must happen after
`completeCheckpointOnceEventsAreDone` and would not be considered.
--
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]