Github user dawidwys commented on a diff in the pull request:

    https://github.com/apache/flink/pull/6283#discussion_r202319384
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -339,12 +339,28 @@ public void 
setSnapshotSettings(JobCheckpointingSettings settings) {
         * Gets the settings for asynchronous snapshots. This method returns 
null, when
         * checkpointing is not enabled.
         *
    -    * @return The snapshot settings, or null, if checkpointing is not 
enabled.
    +    * @return The snapshot settings
         */
        public JobCheckpointingSettings getCheckpointingSettings() {
                return snapshotSettings;
        }
     
    +   /**
    +    * Checks if the checkpointing was enabled for this job graph
    +    *
    +    * @return true if checkpointing enabled
    +    */
    +   public boolean isCheckpointingEnabled() {
    +
    +           if (snapshotSettings == null) {
    +                   return false;
    +           }
    +
    +           long checkpointInterval = 
snapshotSettings.getCheckpointCoordinatorConfiguration().getCheckpointInterval();
    +           return checkpointInterval > 0 &&
    +                   checkpointInterval < Long.MAX_VALUE;
    --- End diff --
    
    I don't think it is true (about the checkpoint enabling). I thought the 
same based on some javadocs, but it turned out that `snapshotSetting` is always 
set in 
`org.apache.flink.streaming.api.graph.StreamingJobGraphGenerator#configureCheckpointing`.
    That's why I added this method.
    
    The problem with the second method is that the `CheckpointCoordinator` is 
created while constructing `ExecutionGraph` which requires the restartstrategy. 
I thought adding this method was the least invasive one.


---

Reply via email to