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:
[email protected]
