AHeise commented on a change in pull request #15728:
URL: https://github.com/apache/flink/pull/15728#discussion_r620498369
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/DefaultCheckpointPlanCalculator.java
##########
@@ -111,7 +112,10 @@ public void setAllowCheckpointsAfterTasksFinished(boolean
allowCheckpointsAfterT
? calculateAfterTasksFinished()
: calculateWithAllTasksRunning();
- checkTasksStarted(result.getTasksToTrigger());
+ checkTasksStarted(
+ isUnalignedCheckpoint
+ ? result.getTasksToWaitFor()
+ : result.getTasksToTrigger());
Review comment:
The main motivation is the following:
- imagine you are running a large-scale application in AC
- you checkpoint every minute because of SLA
- after some failure, Flink is in recovery but recovery takes a 2 minutes
- we have implicit backpressure as any downstream task with large state will
block upstream
previously:
- checkpoint barriers have been added to the stream while recovering
- as soon as a task finishes recovery, it can also start checkpointing
with the change (and no distinction):
- during that time no checkpoints are triggered
- only after every task has been recovered, is checkpoint started at all
- the first checkpoint is delayed compared to before
However, after having written that, I'm not too sure if the new behavior is
that bad:
- the checkpoints that is generated previously is awfully close to the
recovered checkpoint unless there is huge backpressure
- having multiple checkpoint barriers being backpressured during recovery
might lead to quite a bit of I/O to the checkpoint storage after the last task
has recovered.
So all in all, only high backpressure cases would really suffer from that
change and we want to encourage UC for them. High throughput, large scale with
AC might actually benefit from that changed behavior.
--
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]
