gaoyunhaii commented on a change in pull request #17023:
URL: https://github.com/apache/flink/pull/17023#discussion_r704923725



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
##########
@@ -825,22 +824,9 @@ public void updateAccumulators(final AccumulatorSnapshot 
accumulatorSnapshot) {
 
         final CheckpointCoordinator checkpointCoordinator =
                 executionGraph.getCheckpointCoordinator();
-        if (checkpointCoordinator == null) {
-            throw new IllegalStateException(
-                    String.format("Job %s is not a streaming job.", 
jobGraph.getJobID()));
-        } else if (targetDirectory == null
-                && 
!checkpointCoordinator.getCheckpointStorage().hasDefaultSavepointLocation()) {
-            log.info(
-                    "Trying to cancel job {} with savepoint, but no savepoint 
directory configured.",
-                    jobGraph.getJobID());
-
-            throw new IllegalStateException(
-                    "No savepoint directory configured. You can either specify 
a directory "
-                            + "while cancelling via -s :targetDirectory or 
configure a cluster-wide "

Review comment:
       Hi @RocMarshal , sorry I might miss this point: since now we share the 
same message for stop-with-savepoint, normal savepoint and legacy cancelling 
with savepoint, this description seems not always right:
   1. For normal savepoint, the command line format is `bin/flink savepoint 
<jobId> <savepointPath>`
   2. For stop-with-savepoint, the command line format is `bin/flink stop 
<jobId> -p <savepointPath>`.
   3. For the legacy cancel with savepoint, the command line is indeed 
`bin/flink cancel <jobId> -s <savepointPath>`. 
   
   Thus the message here seems not cover all the situations.
   
   Although it is not introduced in this PR, perhaps we could also change it to 
   
   ```
   "You can either specify a directory via configure a cluster-wide 
   + "default via key '"
   + CheckpointingOptions.SAVEPOINT_DIRECTORY.key() 
   + "' or specify a directory in the command line, like
   + "-s :targetDirectory for cancelling, -p :targetDirectory for stopping or 
:targetDirectory for "
   + "purely taking savepoint"
   ```
   




-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to