-
Notifications
You must be signed in to change notification settings - Fork 13.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-20288][docs] Correct documentation about savepoint self-contained #14175
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit aa47d7f (Mon Nov 23 13:37:18 UTC 2020) ✅no warnings Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
@klion26 , could you please take a view on this? |
@Myasuka thanks for your contribution, maybe this is duplicated with FLINK-19381/PR ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a suggestion about how to phrase the section. Please let me know what you think.
@@ -94,7 +94,7 @@ When triggering a savepoint, a new savepoint directory is created where the data | |||
<strong>Attention:</strong> The target directory has to be a location accessible by both the JobManager(s) and TaskManager(s) e.g. a location on a distributed file-system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change this to "...location on a distributed file-system or Object Store." ?
</div> | ||
|
||
Note that if you use the `MemoryStateBackend`, metadata *and* savepoint state will be stored in the `_metadata` file. Since it is self-contained, you may move the file and restore from any location. | ||
Note that if you use the `MemoryStateBackend`, metadata *and* savepoint state will be stored in the `_metadata` file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about changing this to:
Savepoints can generally be moved by moving (or copying) the entire savepoint directory to a different location, and Flink will be able to restore from the moved savepoint.
<div class="alert alert-warning">An exception to that is if *entropy injection* is activated: In that case the savepoint directory will not contain all savepoint data files, because the injected path entropy spreads the files over many directories. Lacking a common savepoint root directory, the savepoints will contain absolute path references, which prevent moving the directory.</div>
<div class="alert alert-warning">Unlike savepoints, checkpoints cannot generally be moved to a different location, because checkpoints may include some absolute path references.</div>
If you use the `MemoryStateBackend`, metadata *and* savepoint state will be stored in the `_metadata` file, so don't be confused by the absence of additional data files.
I would drop the reference to a closed JIRA issue here, users should not feel like they need to look into the bug tracker to understand a feature.
What is the purpose of the change
Correct documentation about savepoint self-contained
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation