masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1403926743
##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java:
##########
@@ -157,6 +157,14 @@ public class CliFrontendParser {
+ " for changing state backends, native = a
specific format for the"
+ " chosen state backend, might be faster to take
and restore from.");
+ public static final Option SAVEPOINT_DETACH_OPTION =
Review Comment:
Whatever we define a new option or just reuse existing one, we should update
all related docs.
##########
flink-clients/src/main/java/org/apache/flink/client/program/rest/RestClusterClient.java:
##########
@@ -608,6 +616,27 @@ private CompletableFuture<String> triggerSavepoint(
});
}
+ private CompletableFuture<String> triggerDetachSavepoint(
+ final JobID jobId,
+ final @Nullable String savepointDirectory,
+ final boolean cancelJob,
+ final SavepointFormatType formatType) {
+ final SavepointTriggerHeaders savepointTriggerHeaders =
Review Comment:
The previous logic is similar with savepoint, right ?
Could we unify them into a method ?
##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java:
##########
@@ -157,6 +157,14 @@ public class CliFrontendParser {
+ " for changing state backends, native = a
specific format for the"
+ " chosen state backend, might be faster to take
and restore from.");
+ public static final Option SAVEPOINT_DETACH_OPTION =
+ new Option(
+ "dcp",
Review Comment:
BTW, I also thinked this again.
Could `stop-with-savepoint` also has this problem ?
If ture, I think we could resolve this together (you could add a new commit).
Then maybe reusing "detached" command looks good to me.
##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java:
##########
@@ -157,6 +157,14 @@ public class CliFrontendParser {
+ " for changing state backends, native = a
specific format for the"
+ " chosen state backend, might be faster to take
and restore from.");
+ public static final Option SAVEPOINT_DETACH_OPTION =
+ new Option(
+ "dcp",
Review Comment:
How about "dsp" ?
Or Could we just reuse existing "detached" ?
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -178,6 +178,23 @@ CompletableFuture<String> stopWithSavepoint(
CompletableFuture<String> triggerSavepoint(
JobID jobId, @Nullable String savepointDirectory,
SavepointFormatType formatType);
+ /**
+ * Triggers a detach savepoint for the job identified by the job id. The
savepoint will be
+ * written to the given savepoint directory, or {@link
+ *
org.apache.flink.configuration.CheckpointingOptions#SAVEPOINT_DIRECTORY} if it
is null.
+ * Notice that: detach savepoint will return with a savepoint trigger id
instead of the path
+ * future, that means the client will return very quickly.
+ *
+ * @param jobId job id
+ * @param savepointDirectory directory the savepoint should be written to
+ * @param formatType a binary format of the savepoint
+ * @return The savepoint trigger id
+ */
+ default CompletableFuture<String> triggerDetachSavepoint(
+ JobID jobId, @Nullable String savepointDirectory,
SavepointFormatType formatType) {
+ return triggerSavepoint(jobId, savepointDirectory, formatType);
Review Comment:
Do we really need the default method ?
It's not a public interface, and seems the default semantic doesn't match
`detach`
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -178,6 +178,23 @@ CompletableFuture<String> stopWithSavepoint(
CompletableFuture<String> triggerSavepoint(
JobID jobId, @Nullable String savepointDirectory,
SavepointFormatType formatType);
+ /**
+ * Triggers a detach savepoint for the job identified by the job id. The
savepoint will be
Review Comment:
```suggestion
* Triggers a detached savepoint for the job identified by the job id.
The savepoint will be
```
--
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
