Zakelly commented on code in PR #25899:
URL: https://github.com/apache/flink/pull/25899#discussion_r1914731914


##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/SavepointType.java:
##########
@@ -54,6 +54,10 @@ public boolean isSavepoint() {
         return true;
     }
 
+    public boolean isFull() {

Review Comment:
   Well, it is a full checkpoint/savepoint as it uploads all the files and 
share nothing with other checkpoint/savepoints. So you could return `true` here.



##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointType.java:
##########
@@ -46,6 +46,10 @@ public boolean isSavepoint() {
         return false;
     }
 
+    public boolean isFull() {
+        return getSharingFilesStrategy() == SharingFilesStrategy.FORWARD;

Review Comment:
   it should be `return getSharingFilesStrategy() == 
SharingFilesStrategy.FORWARD;`
   
   
   And I suggest providing this as an default implementation for the 
`SnapshotType#isFull()`, no need to override & implement in `CheckpointType` 
and `SavepointType`. WDYT?



##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointProperties.java:
##########
@@ -283,6 +293,39 @@ public String toString() {
                     false, // Retain on suspension
                     false);
 
+    private static final CheckpointProperties FULL_CHECKPOINT_NEVER_RETAINED =
+            new CheckpointProperties(
+                    false,
+                    CheckpointType.FULL_CHECKPOINT,
+                    true,
+                    true, // Delete on success
+                    true, // Delete on cancellation
+                    true, // Delete on failure
+                    true, // Delete on suspension
+                    false);
+
+    private static final CheckpointProperties 
FULL_CHECKPOINT_RETAINED_ON_FAILURE =
+            new CheckpointProperties(
+                    false,
+                    CheckpointType.FULL_CHECKPOINT,
+                    true,
+                    true, // Delete on success
+                    true, // Delete on cancellation
+                    false, // Retain on failure
+                    true, // Delete on suspension
+                    false);
+
+    private static final CheckpointProperties 
FULL_CHECKPOINT_RETAINED_ON_CANCELLATION =
+            new CheckpointProperties(
+                    false,
+                    CheckpointType.FULL_CHECKPOINT,
+                    true,
+                    true, // Delete on success
+                    false, // Retain on cancellation
+                    false, // Retain on failure
+                    false, // Retain on suspension
+                    false);
+

Review Comment:
   Why those are introduced?



-- 
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]

Reply via email to