leletan commented on code in PR #20852:
URL: https://github.com/apache/flink/pull/20852#discussion_r974889748
##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointRetentionPolicy.java:
##########
@@ -24,6 +24,9 @@
@Internal
public enum CheckpointRetentionPolicy {
+ /** Full Checkpoints should be retained on cancellation and failure. */
+ FULL_RETAIN_ON_CANCELLATION,
Review Comment:
Good catch @pnowojski. This was one of the places I was not very confident
with. As a hindsight, totally agree it was a bad design.
The reason why I did that was as following:
I was not sure if a `CheckpointProperty` will be valid if it is too far away
from
[these](https://github.com/apache/flink/blob/d8a4304b892412eab4c5c19b5deb84166943d3bb/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointProperties.java#L246)
pre-configured ones, so I was thinking of creating one as close to an existing
pre-configured as possible. Meanwhile the public method to create a
pre-configured `CheckpointProperty` was through
[this](https://github.com/apache/flink/blob/d8a4304b892412eab4c5c19b5deb84166943d3bb/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointProperties.java#L349)
method with a `CheckpointRetentionPolicy`. And I did not notice the
`@Internal` for the `CheckpointRetentionPolicy` just saw it was `public`.
Meanwhile I would love to hear your thoughts
1. Would it be better just to start with our specific use case - always only
trigger a full checkpoint, i.e. reuse all `CheckpointProperty` and only
replace `CheckpointType` with [`FULL_CHECKPOINT
`](https://github.com/apache/flink/blob/d8a4304b892412eab4c5c19b5deb84166943d3bb/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointType.java#L30),
and then allow community to iterate on this API?
2. Or we already know quite a few other use cases for checkpoint manual
triggering so we can start with a more general API? In this case, what are the
options we should expose in this API? Or maybe better to start a mailing list
thread / FLIP to discuss this?
--
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]
