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:
us...@infra.apache.org


Reply via email to