Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3522#discussion_r108451291
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
 ---
    @@ -758,17 +769,36 @@ else if (checkpoint != null) {
         * @throws CheckpointException if the completion failed
         */
        private void completePendingCheckpoint(PendingCheckpoint 
pendingCheckpoint) throws CheckpointException {
    +           assert Thread.holdsLock(lock);
    +
                final long checkpointId = pendingCheckpoint.getCheckpointId();
    -           CompletedCheckpoint completedCheckpoint = null;
     
    +           final boolean externalize =
    +                           
completedCheckpointStore.requiresExternalizedCheckpoints() ||
    +                           
pendingCheckpoint.getProps().externalizeCheckpoint() ||
    +                           pendingCheckpoint.getProps().isSavepoint();
    +
    +           CompletedCheckpoint completedCheckpoint = null;
                try {
                        // externalize the checkpoint if required
    -                   if 
(pendingCheckpoint.getProps().externalizeCheckpoint()) {
    +                   if (externalize) {
                                completedCheckpoint = 
pendingCheckpoint.finalizeCheckpointExternalized();
                        } else {
                                completedCheckpoint = 
pendingCheckpoint.finalizeCheckpointNonExternalized();
                        }
     
    +                   try {
    +                           if (globalStateHooks != null) {
    +                                   completedCheckpoint.registerDisposeHook(
    +                                                   
globalStateHooks.createCheckpointDisposeHook(completedCheckpoint));
    +                           }
    +                   }
    +                   catch (Exception e) {
    +                           // catch all exception, to not let errors in 
cleanup hooks fail the checkpoint
    +                           LOG.warn("Failed to create the cleanup hook for 
checkpoint " + checkpointId + 
    +                                           ". Generic cleanup path will be 
executed...");
    --- End diff --
    
    `e` might be interesting to log since it could contain information what 
went wrong.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to