twalthr commented on code in PR #21248:
URL: https://github.com/apache/flink/pull/21248#discussion_r1015595768
##########
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java:
##########
@@ -765,6 +765,18 @@ public <T> boolean removeConfig(ConfigOption<T>
configOption) {
}
}
+ /**
+ * Removes given key from the configuration.
+ *
+ * @param key key of a config option to remove
+ * @return true is config has been removed, false otherwise
+ */
+ public boolean removeKey(String key) {
+ synchronized (this.confData) {
+ return this.confData.remove(key) != null ||
removePrefixMap(confData, key);
Review Comment:
We should execute both sides regardless whether the left hand side removal
was successful. In theory, both notations can coexist in a configuration
object. But the non-prefix notation has precedence.
This line does exactly the opposite of `removeConfig`. We should update
`removeConfig` and just delete everything we can find.
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/ExecutionCheckpointingOptions.java:
##########
@@ -252,4 +255,29 @@ public class ExecutionCheckpointingOptions {
"{{.Site.BaseURL}}{{.Site.LanguagePrefix}}/docs/dev/datastream/fault-tolerance/checkpointing/#checkpointing-with-parts-of-the-graph-finished-beta",
"the important
considerations"))
.build());
+
+ /**
+ * Access to this option is officially only supported via {@link
+ * CheckpointConfig#setForceCheckpointing(boolean)}, but there is no good
reason behind this.
+ *
+ * @deprecated This will be removed once iterations properly participate
in checkpointing.
+ */
+ @Internal @Deprecated @Documentation.ExcludeFromDocumentation
+ public static final ConfigOption<Boolean> FORCE_CHECKPOINTING =
+ key("execution.checkpointing.force")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Flag to force checkpointing in iterative
jobs.");
+
+ /**
+ * Access to this option is officially only supported via {@link
+ * CheckpointConfig#enableApproximateLocalRecovery(boolean)}, but there is
no good reason behind
+ * this.
+ */
+ @Internal @Documentation.ExcludeFromDocumentation
+ public static final ConfigOption<Boolean> APPROXIMATE_LOCAL_RECOVERY =
+ key("execution.checkpointing..approximate.local.recovery")
Review Comment:
`execution.checkpointing.approximate-local-recovery`: `.` for hierarchy, `-`
for white space replacement
and there is a typo
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/ExecutionCheckpointingOptions.java:
##########
@@ -252,4 +255,29 @@ public class ExecutionCheckpointingOptions {
"{{.Site.BaseURL}}{{.Site.LanguagePrefix}}/docs/dev/datastream/fault-tolerance/checkpointing/#checkpointing-with-parts-of-the-graph-finished-beta",
"the important
considerations"))
.build());
+
+ /**
+ * Access to this option is officially only supported via {@link
+ * CheckpointConfig#setForceCheckpointing(boolean)}, but there is no good
reason behind this.
+ *
+ * @deprecated This will be removed once iterations properly participate
in checkpointing.
+ */
+ @Internal @Deprecated @Documentation.ExcludeFromDocumentation
Review Comment:
nit: @Internal should always be excluded from docs by default, but it also
doesn't harm to keep it there
--
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]
