gaoyunhaii commented on a change in pull request #15466:
URL: https://github.com/apache/flink/pull/15466#discussion_r607686045



##########
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:
       Hi Till, very sorry for I ignored some details for the first comment, in 
realistic although we might taken checkpoint when the tasks are FINISHED, but 
those FINISHED tasks should never be in the list of tasks to trigger. Thus here 
the tasks should not in FINISHED tasks logically. 
   
   I'll update the original comment. And perhaps we could add an explicit 
`checkState(execution.getState() != FINISHED)` here ? If  we add this check we 
would also need to modify the test to exclude the FINISHED state. 




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