You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "xiangforever2014 (via GitHub)" <gi...@apache.org> on 2023/08/22 09:16:04 UTC
[GitHub] [flink] xiangforever2014 opened a new pull request, #23253: Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
xiangforever2014 opened a new pull request, #23253:
URL: https://github.com/apache/flink/pull/23253
<!--
*Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
*Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.*
## Contribution Checklist
- Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.
- Name the pull request in the form "[FLINK-XXXX] [component] Title of the pull request", where *FLINK-XXXX* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component.
Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Azure Pipelines CI to do that following [this guide](https://cwiki.apache.org/confluence/display/FLINK/Azure+Pipelines#AzurePipelines-Tutorial:SettingupAzurePipelinesforaforkoftheFlinkrepository).
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message (including the JIRA id)
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
## What is the purpose of the change
Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
*(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)*
## Brief change log
*(for example:)*
- *The TaskInfo is stored in the blob store on job creation time as a persistent artifact*
- *Deployments RPC transmits only the blob storage reference*
- *TaskManagers retrieve the TaskInfo from the blob cache*
1. Trigger a detach savepoint in CLI, and return the savepoint id (generated by client) immediately,
2. The detached savepoint id will pass to JobManager, and JM will use the id as the savepoint directory name directly
## Verifying this change
Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing
*(Please pick either of the following options)*
This change added tests and can be verified as follows:
-Added test that validates that detach savepoint can be triggered successfully.
`CliFrontendSavepointTest.testTriggerDetachSavepointSuccess`
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (100MB)*
- *Extended integration test for recovery after master (JobManager) failure*
- *Added test that validates that TaskInfo is transferred only once across recoveries*
- *Manually verified the change by running a 4 node cluster with 2 JobManagers and 4 TaskManagers, a stateful streaming program, and killing one JobManager and two TaskManagers during the execution, verifying that recovery happens correctly.*
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (yes / no / don't know)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (not documented)
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1415176143
##########
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:
Thanks for the clarification.
Seems we have to use a new option because of the historical reason.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1854996169
> @masteryhx @Zakelly Following your constructive comments, I have rearranged the commits and rebased to the master branch, please have a look, many thanks~
Thanks for the update.
I saw some minor comments haven't been resolved.
Do you have any other concern ? We could discuss in every comment.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1827056077
@masteryhx hope to get your reply, many thanks~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1427483790
##########
docs/content.zh/docs/deployment/cli.md:
##########
@@ -125,6 +125,43 @@ Lastly, you can optionally provide what should be the [binary format]({{< ref "d
The path to the savepoint can be used later on to [restart the Flink job](#starting-a-job-from-a-savepoint).
+If the state of the job is quite big, the client will get a timeout exception since it should wait for the savepoint finished.
+```
+Triggering savepoint for job bec5244e09634ad71a80785937a9732d.
+Waiting for response...
+
+--------------------------------------------------------------
+The program finished with the following exception:
+
+org.apache.flink.util.FlinkException: Triggering a savepoint for the job bec5244e09634ad71a80785937a9732d failed.
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend. java:828)
+ at org.apache.flink.client.cli.CliFrontend.lambda$savepopint$8(CliFrontend.java:794)
+ at org.apache.flink.client.cli.CliFrontend.runClusterAction(CliFrontend.java:1078)
+ at org.apache.flink.client.cli.CliFrontend.savepoint(CliFrontend.java:779)
+ at org.apache.flink.client.cli.CliFrontend.parseAndRun(CliFrontend.java:1150)
+ at org.apache.flink.client.cli.CliFrontend.lambda$mainInternal$9(CliFrontend.java:1226)
+ at org.apache.flink.runtime.security.contexts.NoOpSecurityContext.runSecured(NoOpSecurityContext.java:28)
+ at org.apache.flink.client.cli.CliFrontend.mainInternal(CliFrontend.java:1226)
+ at org.apache.flink.client.cli.CliFrontend.main(CliFronhtend.java:1194)
+Caused by: java.util.concurrent.TimeoutException
+ at java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1784)
+ at java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1928)
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend.java:822)
+ ... 8 more
+```
+In this case, we could use "-dsp" option to trigger a detached savepoint, the client will return immediately as soon as the trigger id returns.
+```bash
+$ ./bin/flink savepoint \
+ $JOB_ID \
+ /tmp/flink-savepoints
+ -dsp
+```
+```
+Triggering savepoint in detached mode for job bec5244e09634ad71a80785937a9732d.
+Successfully trigger manual savepoint, triggerId: 2505bbd12c5b58fd997d0f193db44b97
+```
+We can get the status of the detached savepoint by rest api "/jobs/:jobid/savepoints/:triggerid".
Review Comment:
done~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1839926426
@masteryhx Kindly remind, hope to get your reply, many thanks ^_^
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1416745308
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -188,6 +207,21 @@ CompletableFuture<String> triggerSavepoint(
*/
CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType checkpointType);
+ /**
+ * Triggers a detached 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
+ */
+ CompletableFuture<String> triggerDetachSavepoint(
Review Comment:
Thanks for your comment, sorry but I want to ensure that this means we should implement `stopWithDetachedSavepoint` by invoking `triggerDetachedSavepoint`? Or we should abstract an interface which is responsible for implementing normal detached savepoint and stop-with-detached-savepoint? I think currently it is clear that we use two different function to trigger savepoint in detached mode, that is `triggerDetachSavepoint` & `stopWithDetachedSavepoint`, do you want to merge these two into one function? Hope to get your opinion~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1415222035
##########
docs/content.zh/docs/deployment/cli.md:
##########
@@ -125,6 +125,43 @@ Lastly, you can optionally provide what should be the [binary format]({{< ref "d
The path to the savepoint can be used later on to [restart the Flink job](#starting-a-job-from-a-savepoint).
+If the state of the job is quite big, the client will get a timeout exception since it should wait for the savepoint finished.
Review Comment:
```suggestion
If the state size of the job is quite big, the client will get a timeout exception since it has to wait for the savepoint finished.
```
##########
docs/content.zh/docs/deployment/cli.md:
##########
@@ -125,6 +125,43 @@ Lastly, you can optionally provide what should be the [binary format]({{< ref "d
The path to the savepoint can be used later on to [restart the Flink job](#starting-a-job-from-a-savepoint).
+If the state of the job is quite big, the client will get a timeout exception since it should wait for the savepoint finished.
+```
+Triggering savepoint for job bec5244e09634ad71a80785937a9732d.
+Waiting for response...
+
+--------------------------------------------------------------
+The program finished with the following exception:
+
+org.apache.flink.util.FlinkException: Triggering a savepoint for the job bec5244e09634ad71a80785937a9732d failed.
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend. java:828)
+ at org.apache.flink.client.cli.CliFrontend.lambda$savepopint$8(CliFrontend.java:794)
+ at org.apache.flink.client.cli.CliFrontend.runClusterAction(CliFrontend.java:1078)
+ at org.apache.flink.client.cli.CliFrontend.savepoint(CliFrontend.java:779)
+ at org.apache.flink.client.cli.CliFrontend.parseAndRun(CliFrontend.java:1150)
+ at org.apache.flink.client.cli.CliFrontend.lambda$mainInternal$9(CliFrontend.java:1226)
+ at org.apache.flink.runtime.security.contexts.NoOpSecurityContext.runSecured(NoOpSecurityContext.java:28)
+ at org.apache.flink.client.cli.CliFrontend.mainInternal(CliFrontend.java:1226)
+ at org.apache.flink.client.cli.CliFrontend.main(CliFronhtend.java:1194)
+Caused by: java.util.concurrent.TimeoutException
+ at java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1784)
+ at java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1928)
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend.java:822)
+ ... 8 more
+```
+In this case, we could use "-dsp" option to trigger a detached savepoint, the client will return immediately as soon as the trigger id returns.
Review Comment:
```suggestion
In this case, we could use "-dsp" option to trigger a detached savepoint, the client will return the trigger id immediately.
```
##########
docs/content.zh/docs/deployment/cli.md:
##########
@@ -125,6 +125,43 @@ Lastly, you can optionally provide what should be the [binary format]({{< ref "d
The path to the savepoint can be used later on to [restart the Flink job](#starting-a-job-from-a-savepoint).
+If the state of the job is quite big, the client will get a timeout exception since it should wait for the savepoint finished.
+```
+Triggering savepoint for job bec5244e09634ad71a80785937a9732d.
+Waiting for response...
+
+--------------------------------------------------------------
+The program finished with the following exception:
+
+org.apache.flink.util.FlinkException: Triggering a savepoint for the job bec5244e09634ad71a80785937a9732d failed.
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend. java:828)
+ at org.apache.flink.client.cli.CliFrontend.lambda$savepopint$8(CliFrontend.java:794)
+ at org.apache.flink.client.cli.CliFrontend.runClusterAction(CliFrontend.java:1078)
+ at org.apache.flink.client.cli.CliFrontend.savepoint(CliFrontend.java:779)
+ at org.apache.flink.client.cli.CliFrontend.parseAndRun(CliFrontend.java:1150)
+ at org.apache.flink.client.cli.CliFrontend.lambda$mainInternal$9(CliFrontend.java:1226)
+ at org.apache.flink.runtime.security.contexts.NoOpSecurityContext.runSecured(NoOpSecurityContext.java:28)
+ at org.apache.flink.client.cli.CliFrontend.mainInternal(CliFrontend.java:1226)
+ at org.apache.flink.client.cli.CliFrontend.main(CliFronhtend.java:1194)
+Caused by: java.util.concurrent.TimeoutException
+ at java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1784)
+ at java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1928)
+ at org.apache.flink.client.cli.CliFrontend.triggerSavepoint(CliFrontend.java:822)
+ ... 8 more
+```
+In this case, we could use "-dsp" option to trigger a detached savepoint, the client will return immediately as soon as the trigger id returns.
+```bash
+$ ./bin/flink savepoint \
+ $JOB_ID \
+ /tmp/flink-savepoints
+ -dsp
+```
+```
+Triggering savepoint in detached mode for job bec5244e09634ad71a80785937a9732d.
+Successfully trigger manual savepoint, triggerId: 2505bbd12c5b58fd997d0f193db44b97
+```
+We can get the status of the detached savepoint by rest api "/jobs/:jobid/savepoints/:triggerid".
Review Comment:
Could we just add a link of REST API here ?
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -188,6 +207,21 @@ CompletableFuture<String> triggerSavepoint(
*/
CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType checkpointType);
+ /**
+ * Triggers a detached 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
+ */
+ CompletableFuture<String> triggerDetachSavepoint(
Review Comment:
Let's use 'triggerDetachedSavepoint' aligned with 'stopWithDetachedSavepoint'.
You could check other places about 'detach savepoint'.
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -188,6 +207,21 @@ CompletableFuture<String> triggerSavepoint(
*/
CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType checkpointType);
+ /**
+ * Triggers a detached 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
+ */
+ CompletableFuture<String> triggerDetachSavepoint(
Review Comment:
Let's use 'triggerDetachedSavepoint' aligned with 'stopWithDetachedSavepoint'.
You could check other places about 'detach savepoint'.
##########
flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendSavepointTest.java:
##########
@@ -209,6 +209,34 @@ void testDisposeSavepointSuccess() throws Exception {
}
}
+ // ------------------------------------------------------------------------
+ // detach savepoint
+ // ------------------------------------------------------------------------
+
+ @Test
+ void testTriggerDetachSavepointSuccess() throws Exception {
Review Comment:
Could you also test stop-with-detached-savepoint ?
##########
flink-clients/src/main/java/org/apache/flink/client/program/MiniClusterClient.java:
##########
@@ -115,6 +124,12 @@ public CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType che
return miniCluster.triggerCheckpoint(jobId, checkpointType);
}
+ @Override
+ public CompletableFuture<String> triggerDetachSavepoint(
+ JobID jobId, @Nullable String savepointDirectory, SavepointFormatType formatType) {
+ throw new UnsupportedOperationException();
Review Comment:
Could you implement them simply ?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1832964678
> @masteryhx kindly remind, hope to get your reply, many thanks~
Seems there are some conflicts.
Could you rebase the master and resolve them at first ?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] xiangforever2014 commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1688077105
@flinkbot run azure
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] xiangforever2014 commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1689552969
@flinkbot run azure
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1687805721
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "af3d7a2f1d21c543be14b2ee12b1ec8c7c2f4086",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "af3d7a2f1d21c543be14b2ee12b1ec8c7c2f4086",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* af3d7a2f1d21c543be14b2ee12b1ec8c7c2f4086 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1858801528
@masteryhx Hello, the CI is succeed, thank you~
<img width="546" alt="image" src="https://github.com/apache/flink/assets/23545296/76f908e7-5652-4a48-825a-96e42755970b">
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1851829103
> > > > Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'. WDYT?
> > > > And I agree we could adjust all these CLI options in 2.0.
> > >
> > >
> > > Thanks for your comment~ But I wonder that is there any problem in current implementation? Or why we need to change it to "-D" or "-detach"? Hope to get your suggestion~
> >
> >
> > Hi @xiangforever2014 . Short commands are designed to be readable or easy for users to remember. It can be omitted if there is no suitable abbreviation. The '-dcp' is not that easy to remember or understand, user may not know what it stands for. WDYT?
>
> Thanks for your suggestion, and I think that's indeed more friendly to users, I have followed your suggestion and use '-detach' for the detached mode~
That's cool. +1 from my side.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1426092346
##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java:
##########
@@ -164,6 +164,14 @@ public class CliFrontendParser {
false,
"Defines whether to trigger this checkpoint as a full one.");
+ static final Option SAVEPOINT_DETACH_OPTION =
Review Comment:
1. How about just using 'detached' here aligned with DETACHED_OPTION ? It should not be conflict with other one because it will only be checked in savepoint and stop-with-savepoint command, right ? It will help us migrating in the future.
2. I saw this could also be used for manual checkpoint. Of course, we could support this in other ticket/pr.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1833181042
@masteryhx Hello, I have rebased the code to the master, PTAL~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1415217180
##########
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:
It will be clearer If we could make it aligned in Flink 2.0.
cc @Zakelly
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1427486664
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -188,6 +207,21 @@ CompletableFuture<String> triggerSavepoint(
*/
CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType checkpointType);
+ /**
+ * Triggers a detached 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
+ */
+ CompletableFuture<String> triggerDetachSavepoint(
Review Comment:
Thanks for your careful comments, I have rename it to "detached" and checked all other places~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] masteryhx commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1738497112
@xiangforever2014
Thanks for the detailed clarification.
I think remaining too many mapping info in CheckpointCoordinator is not necessary which brings too much unnecessary code path just for sepreated savepoint info.
I think the core problem is that current savepoint in detached mode lacks some returned infos to further lookup such as checkpoint id or savepoint id, right ?
BTW, This problem also exists in manual checkpoint, so I'd suggest to consider them together.
Current REST API could use trigger id to see the status of savepoint which should also work for CLI.
So if you pass or generate a trigger id and print it in CLI when detached mode, you could also use it to check the status of savepoint, right ?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] xiangforever2014 commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1720713759
@masteryhx
Many thanks for your comment~
Here are my requirements:
1. support triggering savepoint in detached mode, so the cli process can return quickly and will not occupy the resource for a long time;
2. get the status of detach savepoint information through platform, that is whether the detach savepoint has been made successfully, or failed, or producing at the time.
To your question:
With non-detach savepoint, we could know whether this savepoint has been successfully triggered and made from the CLI output, but in detach mode, if we just change code in flink-clients, that is, just send the request and return, we could not know whether the savepoint triggered this time is in which status from the current REST API, at least we need to know the savepoint id, in this case, we could use api "/jobs/:jobid/checkpoints/details/:checkpointid" to get the savepoint status.
But in this case, the client needs to wait for the savepoint id msg from jm, that makes the client needs to wait for a while.
So in this pr, I support triggering savepoint in detach mode as following:
1. client generates a savepoint-id, which is produced by RandomUUID, and when triggering savepoint in detach mode, this info will put into the request body;
2. when jm receives the request, it will check the savepoint-id info, if this field is set, it will directly use this as the savepoint id instead of generating a id like savepoint-xxxxxx.
In this way, client does not need to wait for any message, it triggers the detach savepoint and generates a savepoint id, then its job is finished. If we want to know the status of the detach savepoint, we could use the dump-pending-savepoint api and get all savepoint ids, then compare the savepoint id generated by client.
Hope to get your suggestion, thank you~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1427489342
##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java:
##########
@@ -164,6 +164,14 @@ public class CliFrontendParser {
false,
"Defines whether to trigger this checkpoint as a full one.");
+ static final Option SAVEPOINT_DETACH_OPTION =
Review Comment:
1. Good advice, I think using 'detached' option here can make it easily for users to understand, thanks for comment.
2. Yes, I have notice this point too, maybe we could finish this ticket first ~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1849238533
> > Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'. WDYT?
> > And I agree we could adjust all these CLI options in 2.0.
>
> Thanks for your comment~ But I wonder that is there any problem in current implementation? Or why we need to change it to "-D" or "-detach"? Hope to get your suggestion~
Hi @xiangforever2014 . Short commands are designed to be readable or easy for users to remember. It can be omitted if there is no suitable abbreviation. The '-dcp' is not that easy to remember or understand, user may not know what it stands for. WDYT?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1842188948
> Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'. WDYT?
>
> And I agree we could adjust all these CLI options in 2.0.
Thanks for your comment~
But I wonder that is there any problem in current implementation? Or why we need to change it to "-D" or "-detach"?
Hope to get your suggestion~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] xiangforever2014 commented on pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1688085796
@flinkbot run azure
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1827533221
@masteryhx Many thanks for your insightful comments, I have updated the code according to the comments, PTAL, thanks again bro~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1831543321
@masteryhx kindly remind, hope to get your reply, many thanks~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1840388226
Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'. WDYT?
I agree we could adjust these CLI options in 2.0.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1849650290
> > > Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'. WDYT?
> > > And I agree we could adjust all these CLI options in 2.0.
> >
> >
> > Thanks for your comment~ But I wonder that is there any problem in current implementation? Or why we need to change it to "-D" or "-detach"? Hope to get your suggestion~
>
> Hi @xiangforever2014 . Short commands are designed to be readable or easy for users to remember. It can be omitted if there is no suitable abbreviation. The '-dcp' is not that easy to remember or understand, user may not know what it stands for. WDYT?
Thanks for your suggestion, and I think that's indeed more friendly to users, I have followed your suggestion and use '-detach' for the detached mode~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1845053733
@masteryhx Hello, I have updated the code according to your comment, PTAL~
Here are the test results:
1. trigger the normal savepoint, the client gets a timeout exception:
2. trigger the detached savepoint, the client returns the trigger id immediately, and we could monitor the status of the savepoint by rest api:
3. trigger the stop-with-savepoint, the client also gets a timeout exception, and the job will be stopped after the savepoint finished:
4. trigger the stop-with-detached-savepoint, the client also returns the trigger id immediately, and we could get the status of the savepoint by rest api through the trigger id, after the savepoint finished, the job will be stopped too:
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1415245692
##########
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:
Hi @xiangforever2014 @masteryhx sorry for jumping in... And I'm wondering if we could use the capital '-D' for short option? Or I suggest we do not leave any short option for this, only using '-detach'.
I agree we could adjust these CLI options in 2.0 .
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1851604601
@masteryhx @Zakelly Following your constructive comments, I have rearranged the commits and rebased to the master branch, please have a look, many thanks~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1862077615
@masteryhx kindly remind~
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx closed pull request #23253: [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api
URL: https://github.com/apache/flink/pull/23253
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #23253:
URL: https://github.com/apache/flink/pull/23253#discussion_r1426080866
##########
flink-clients/src/main/java/org/apache/flink/client/program/ClusterClient.java:
##########
@@ -188,6 +207,21 @@ CompletableFuture<String> triggerSavepoint(
*/
CompletableFuture<Long> triggerCheckpoint(JobID jobId, CheckpointType checkpointType);
+ /**
+ * Triggers a detached 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
+ */
+ CompletableFuture<String> triggerDetachSavepoint(
Review Comment:
Sorry for misleading.
I just talked about naming.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1820273335
@masteryhx Many thanks for your suggestion, and I have followed your advice to simplify the code.
I have tested in my local machine, when triggering a detached savepoint, the CLI will return with a trigger id immediately, and I could use the trigger id to get the status of the savepoint by REST API [/jobs/:jobid/savepoints/:triggerid ](https://nightlies.apache.org/flink/flink-docs-release-1.18/docs/ops/rest_api/#jobs-jobid-savepoints-triggerid).
Triggering a savepoint vs. Triggering a detached savepoint:
1. Triggering a savepoint: from the picture, we could see that the CLI should wait for the savepoint to be finished, and it is very easy to cause a CLI timeout when the state of the job is huge.
2. Triggering a detached savepoint: the CLI returned with the trigger id immediately, and we could get the status of the savepoint by REST API.
- when the savepoint has not finished, we get
- once the savepoint finished, we get
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-32881][checkpoint] Support triggering savepoint in detach mode for CLI and dumping all pending savepoint ids by rest api [flink]
Posted by "xiangforever2014 (via GitHub)" <gi...@apache.org>.
xiangforever2014 commented on PR #23253:
URL: https://github.com/apache/flink/pull/23253#issuecomment-1820158718
@flinkbot run azure
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org