StefanRRichter commented on a change in pull request #7571: [FLINK-10724]
Refactor failure handling in check point coordinator
URL: https://github.com/apache/flink/pull/7571#discussion_r275871911
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java
##########
@@ -944,7 +943,13 @@ public void heartbeatFromResourceManager(final ResourceID
resourceID) {
}
return checkpointCoordinator
.triggerSavepoint(System.currentTimeMillis(),
targetDirectory)
- .thenApply(CompletedCheckpoint::getExternalPointer)
+ .thenApply(checkpointExecuteResult -> {
+ try {
+ return
checkpointExecuteResult.getCompletedCheckpoint().getExternalPointer();
+ } catch (Exception e) {
Review comment:
This `try-catch` feels bad here, just to catch the illegal state exception.
First improvement could be to check `isSuccess` instead of using exceptions for
controlflow. Second, as commented before, if we just use
`CompletableFuture<CompletedCheckpoint>` with the capability of
`#completeExceptionally` over `CheckpointExecutionResult` we can just skip the
`thenApply` and handle all cases in `handleAsync`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
