mxm commented on code in PR #656:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/656#discussion_r1302905833
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/KubernetesOperatorConfigOptions.java:
##########
@@ -321,7 +336,32 @@ public static String operatorConfigKey(String key) {
+ "but to complement them in special
cases. For instance, a "
+ "full checkpoint might need to be
occasionally triggered to "
+ "break the chain of incremental
checkpoints and consolidate "
- + "the partial incremental files.");
+ + "the partial incremental files. "
+ + "WARNING: not intended to be used
together with the cron-based "
+ + "periodic checkpoint triggering");
+
+ @Documentation.Section(SECTION_DYNAMIC)
+ public static final ConfigOption<String> PERIODIC_CHECKPOINT_CRON =
+ operatorConfig("periodic.checkpoint.cron")
+ .stringType()
+ .defaultValue(
+ "0 0 0 29 2 ? 1999") // Default value that is
guaranteed not to trigger (not a leap year)
+ .withDescription(
+ "A cron expression that defines when to
automatically trigger checkpoints. "
+ + "The precise triggering schedule is not
guaranteed, checkpoints will "
+ + "be triggered as part of the regular
reconcile loop. Periodic triggering "
+ + "with frequency higher than the
reconciliation loop "
+ + "("
+ + OPERATOR_RECONCILE_INTERVAL.key()
+ + ") will lead to trigger omissions. "
+ + "NOTE: checkpoints are generally managed
by Flink. This "
+ + "setting isn't meant to replace Flink's
checkpoint settings, "
+ + "but to complement them in special
cases. For instance, a "
+ + "full checkpoint might need to be
occasionally triggered to "
+ + "break the chain of incremental
checkpoints and consolidate "
+ + "the partial incremental files. "
+ + "WARNING: not intended to be used
together with the interval-based "
+ + "periodic checkpoint triggering");
Review Comment:
For the sake of simplifying this, could we unify both options under
`periodic.{checkpoint,savepoint}.interval`? We can parse the String option
using `Duration`, if it fails (i.e. throws `DateTimeParseException`) we try the
cron-based approach. I think this approach should be safe because a mixup
between the two is not possible.
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/KubernetesOperatorConfigOptions.java:
##########
@@ -321,7 +336,32 @@ public static String operatorConfigKey(String key) {
+ "but to complement them in special
cases. For instance, a "
+ "full checkpoint might need to be
occasionally triggered to "
+ "break the chain of incremental
checkpoints and consolidate "
- + "the partial incremental files.");
+ + "the partial incremental files. "
+ + "WARNING: not intended to be used
together with the cron-based "
+ + "periodic checkpoint triggering");
+
+ @Documentation.Section(SECTION_DYNAMIC)
+ public static final ConfigOption<String> PERIODIC_CHECKPOINT_CRON =
+ operatorConfig("periodic.checkpoint.cron")
+ .stringType()
+ .defaultValue(
+ "0 0 0 29 2 ? 1999") // Default value that is
guaranteed not to trigger (not a leap year)
+ .withDescription(
+ "A cron expression that defines when to
automatically trigger checkpoints. "
+ + "The precise triggering schedule is not
guaranteed, checkpoints will "
+ + "be triggered as part of the regular
reconcile loop. Periodic triggering "
+ + "with frequency higher than the
reconciliation loop "
+ + "("
+ + OPERATOR_RECONCILE_INTERVAL.key()
+ + ") will lead to trigger omissions. "
+ + "NOTE: checkpoints are generally managed
by Flink. This "
+ + "setting isn't meant to replace Flink's
checkpoint settings, "
+ + "but to complement them in special
cases. For instance, a "
+ + "full checkpoint might need to be
occasionally triggered to "
+ + "break the chain of incremental
checkpoints and consolidate "
+ + "the partial incremental files. "
+ + "WARNING: not intended to be used
together with the interval-based "
+ + "periodic checkpoint triggering");
Review Comment:
Would the interval and the cron-based logic really not work together, or is
this more about warning the user about unwanted checkpoint triggering?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
