dawidwys commented on a change in pull request #18482:
URL: https://github.com/apache/flink/pull/18482#discussion_r793575880



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -498,13 +501,6 @@ public boolean isShutdown() {
             @Nullable String externalSavepointLocation,
             boolean isPeriodic) {
 
-        if (props.getCheckpointType().getPostCheckpointAction() == 
PostCheckpointAction.TERMINATE
-                && !(props.isSynchronous() && props.isSavepoint())) {
-            return FutureUtils.completedExceptionally(
-                    new IllegalArgumentException(
-                            "Only synchronous savepoints are allowed to 
advance the watermark to MAX."));
-        }

Review comment:
       I find those checks at this locaiton pointless. Valid combinations of 
properties of a `SnapshotType` are limited by factory methods of 
`CheckpointType` & `SavepointType`. I see no reason for checking it again here.
   
   Moreover a call to `props.isSynchronous()` was even more pointless as 
internally it was checking `postCheckpointAction != PostCheckpointAction.NONE`.




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