XComp commented on a change in pull request #14847:
URL: https://github.com/apache/flink/pull/14847#discussion_r582029660
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
##########
@@ -835,8 +840,21 @@ public void updateAccumulators(final AccumulatorSnapshot
accumulatorSnapshot) {
mainThreadExecutor);
}
- private void startCheckpointScheduler(final CheckpointCoordinator
checkpointCoordinator) {
+ @Override
+ public void stopCheckpointScheduler() {
+ Preconditions.checkState(
+ getCheckpointCoordinator() != null,
+ "Checkpointing cannot be stopped since it's not enabled.");
+ getCheckpointCoordinator().stopCheckpointScheduler();
+ }
+
+ @Override
+ public void startCheckpointScheduler() {
mainThreadExecutor.assertRunningInMainThread();
+ final CheckpointCoordinator checkpointCoordinator =
getCheckpointCoordinator();
+ Preconditions.checkState(
+ checkpointCoordinator != null,
+ "Checkpointing cannot be started since it's not enabled.");
Review comment:
I went for the first option. Unfortunately, I had to remove the
`CheckpointScheduling` interface from the `CheckpointCoordinator` due to that
since the `CheckpointCoordinator` is throwing an exception in case of a
shutdown.
The approach with having a method for checking whether Checkpoint Scheduling
is enabled wasn't better either because it would have meant: Adding this check
would have exposed the shutdown behavior of the `CheckpointCoordinator` which,
as a consequence, would have required to add the `shutdown` method to the
interface as well. This method doesn't work for the Scheduler, though. Hence, I
decided to go for the approach that checks the availability of the
`CheckpointCoordinator` internally.
----------------------------------------------------------------
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]