gaoyunhaii commented on a change in pull request #15466:
URL: https://github.com/apache/flink/pull/15466#discussion_r607707313
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/DefaultCheckpointPlanCalculator.java
##########
@@ -147,11 +147,7 @@ private void checkAllTasksInitiated() throws
CheckpointException {
*/
private void checkTasksStarted(List<Execution> toTrigger) throws
CheckpointException {
for (Execution execution : toTrigger) {
- if (execution.getState() == ExecutionState.CREATED
- || execution.getState() == ExecutionState.RECOVERING
- || execution.getState() == ExecutionState.SCHEDULED
- || execution.getState() == ExecutionState.DEPLOYING) {
-
+ if (execution.getState() != ExecutionState.RUNNING) {
Review comment:
Sorry for I just modified the comment and might cause some confusion...
`Although we would also trigger checkpoint after tasks finished, but the
finished tasks should not be in the list of tasks to trigger` This part is I
just modified, and the original comment said `...to change the condition to
only allows for RUNNING status or FINISHED status`.
I also think we should change the
`checkTasksStarted(result.getTasksToTrigger());`. I think Till's concern should
be why we not change it to `if (execution.getState() != ExecutionState.RUNNING
&& execution.getState() != ExecutionState.FINISHED )`
--
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]
