xiangforever2014 commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1405754517
##########
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:
Thanks for your comment, I have updated the related docs manually.
##########
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:
These is a "-d" option for savepoints already(SAVEPOINT_DISPOSE_OPTION), so
I think we can not just simply reuse the existing detached
option(DETACHED_OPTION) since it's short option is also "-d", if we force to
reuse the option, we need to modify either SAVEPOINT_DISPOSE_OPTION or
DETACHED_OPTION, which may cause some influence to existing users, so I think
it's better to add a new option to support detached savepoint, WDYT?
##########
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:
Thanks for your insightful comments, the reason we add the default method
here is that we only want to implement the detached mode for rest api in this
commit. I have resolved this comment, PTAL.
##########
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:
Thanks for your observant comments, resolved~
##########
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:
Thanks for your comment, I have unified the detached/non-detached savepoint
to one method~
--
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
