yanghua commented on a change in pull request #8322: [FLINK-12364] Introduce a
CheckpointFailureManager to centralized manage checkpoint failure
URL: https://github.com/apache/flink/pull/8322#discussion_r286773309
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java
##########
@@ -512,24 +513,19 @@ public boolean isArchived() {
}
public void enableCheckpointing(
- long interval,
- long checkpointTimeout,
- long minPauseBetweenCheckpoints,
- int maxConcurrentCheckpoints,
- CheckpointRetentionPolicy retentionPolicy,
+ CheckpointCoordinatorConfiguration chkConfig,
List<ExecutionJobVertex> verticesToTrigger,
List<ExecutionJobVertex> verticesToWaitFor,
List<ExecutionJobVertex> verticesToCommitTo,
List<MasterTriggerRestoreHook<?>> masterHooks,
CheckpointIDCounter checkpointIDCounter,
CompletedCheckpointStore checkpointStore,
StateBackend checkpointStateBackend,
- CheckpointStatsTracker statsTracker,
- boolean isPreferCheckpointForRecovery) {
+ CheckpointStatsTracker statsTracker) {
// simple sanity checks
- checkArgument(interval >= 10, "checkpoint interval must not be
below 10ms");
- checkArgument(checkpointTimeout >= 10, "checkpoint timeout must
not be below 10ms");
+ checkArgument(chkConfig.getCheckpointInterval() >= 10,
"checkpoint interval must not be below 10ms");
Review comment:
Actually, the interval in many places is not united. In CheckpointConfig it
used -1 as a disabled flag and the value which large than 0 is legal. So in
CheckpointCoordinatorConfiguration the sanity check based on 1. It's better to
unify them. Either -1 or larger than or equal to 10. So agree with you!
----------------------------------------------------------------
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]
With regards,
Apache Git Services
