ifndef-SleePy commented on a change in pull request #10332: 
[FLINK-13905][checkpointing] Separate checkpoint triggering into several 
asynchronous stages
URL: https://github.com/apache/flink/pull/10332#discussion_r367774356
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
 ##########
 @@ -485,76 +481,151 @@ public boolean isShutdown() {
                        CheckpointProperties props,
                        @Nullable String externalSavepointLocation,
                        boolean isPeriodic,
-                       boolean advanceToEndOfTime) throws CheckpointException {
+                       boolean advanceToEndOfTime) {
 
                if (advanceToEndOfTime && !(props.isSynchronous() && 
props.isSavepoint())) {
-                       throw new IllegalArgumentException("Only synchronous 
savepoints are allowed to advance the watermark to MAX.");
+                       return FutureUtils.completedExceptionally(new 
IllegalArgumentException(
+                               "Only synchronous savepoints are allowed to 
advance the watermark to MAX."));
                }
 
-               // make some eager pre-checks
+               final CompletableFuture<CompletedCheckpoint> 
onCompletionPromise =
+                       new CompletableFuture<>();
                synchronized (lock) {
-                       preCheckBeforeTriggeringCheckpoint(isPeriodic, 
props.forceCheckpoint());
-               }
-
-               // check if all tasks that we need to trigger are running.
-               // if not, abort the checkpoint
-               Execution[] executions = new Execution[tasksToTrigger.length];
-               for (int i = 0; i < tasksToTrigger.length; i++) {
-                       Execution ee = 
tasksToTrigger[i].getCurrentExecutionAttempt();
-                       if (ee == null) {
-                               LOG.info("Checkpoint triggering task {} of job 
{} is not being executed at the moment. Aborting checkpoint.",
-                                               
tasksToTrigger[i].getTaskNameWithSubtaskIndex(),
-                                               job);
-                               throw new 
CheckpointException(CheckpointFailureReason.NOT_ALL_REQUIRED_TASKS_RUNNING);
-                       } else if (ee.getState() == ExecutionState.RUNNING) {
-                               executions[i] = ee;
-                       } else {
-                               LOG.info("Checkpoint triggering task {} of job 
{} is not in state {} but {} instead. Aborting checkpoint.",
-                                               
tasksToTrigger[i].getTaskNameWithSubtaskIndex(),
-                                               job,
-                                               ExecutionState.RUNNING,
-                                               ee.getState());
-                               throw new 
CheckpointException(CheckpointFailureReason.NOT_ALL_REQUIRED_TASKS_RUNNING);
+                       if (isTriggering || !triggerRequestQueue.isEmpty()) {
 
 Review comment:
   `triggerRequestQueue` might be accessed in main thread, through `shutdown` 
and `stopCheckpointScheduler`.

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


With regards,
Apache Git Services

Reply via email to