rmetzger commented on a change in pull request #14948: URL: https://github.com/apache/flink/pull/14948#discussion_r588384037
########## File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/stopwithsavepoint/StopWithSavepointOperationManager.java ########## @@ -45,18 +57,28 @@ public StopWithSavepointTerminationManager( * Enforces the correct completion order of the passed {@code CompletableFuture} instances in * accordance to the contract of {@link StopWithSavepointTerminationHandler}. * - * @param completedSavepointFuture The {@code CompletableFuture} of the savepoint creation step. + * @param terminate Flag indicating whether to terminate or suspend the job. + * @param targetDirectory Target for the savepoint. * @param terminatedExecutionStatesFuture The {@code CompletableFuture} of the termination step. * @param mainThreadExecutor The executor the {@code StopWithSavepointTerminationHandler} * operations run on. * @return A {@code CompletableFuture} containing the path to the created savepoint. */ - public CompletableFuture<String> stopWithSavepoint( - CompletableFuture<CompletedCheckpoint> completedSavepointFuture, + public CompletableFuture<String> trackStopWithSavepoint( + boolean terminate, Review comment: Yes! I believe this is necessary. The argument order is different in various parts of the codebase, and the semantics of the boolean flag are defined different between flink-runtime and flink-core. However, I would like to tackle this in a follow up PR, as this change alone will be difficult to review (I started doing it, but realized that this is touching a lot of classes): https://issues.apache.org/jira/browse/FLINK-21638 ``` flink-clients/src/main/java/org/apache/flink/client/deployment/application/EmbeddedJobClient.java | 5 +++-- flink-core/src/main/java/org/apache/flink/core/execution/JobClient.java | 6 +++--- flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java | 6 +++--- flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointProperties.java | 17 ++++++++--------- flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/StopWithSavepointOperations.java | 12 +++++++++++- flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java | 6 ++++-- flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java | 7 +++++-- flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMasterGateway.java | 3 ++- flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java | 5 +++-- flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniClusterJobClient.java | 5 +++-- flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/job/savepoints/SavepointHandlers.java | 9 +++++++-- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java | 9 +++++---- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerNG.java | 4 +++- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveScheduler.java | 9 +++++---- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/Executing.java | 7 ++++--- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/StopWithSavepoint.java | 17 +++++++++-------- flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/stopwithsavepoint/StopWithSavepointOperationManager.java | 8 +++++--- flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/RestfulGateway.java | 5 +++-- flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinatorTest.java | 3 ++- flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/TestingStopWithSavepointOperations.java | 2 +- flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/ExecutingTest.java | 4 +++- flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/StopWithSavepointTest.java | 7 ++++--- flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/stopwithsavepoint/StopWithSavepointOperationManagerTest.java | 5 +++-- 23 files changed, 99 insertions(+), 62 deletions(-) ``` ---------------------------------------------------------------- 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