rmetzger commented on a change in pull request #14847:
URL: https://github.com/apache/flink/pull/14847#discussion_r581918367



##########
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:
       The checkpoint scheduler can be null, in situation I consider valid:
   scenario:
   - job with no restarts configured
   - savepoint creation fails
   
   - result: the EG reaches a terminal state, and sets the checkpoint 
coordinator to null. This precondition fails, killing the JobManager process. 




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