-
Notifications
You must be signed in to change notification settings - Fork 13k
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-33057][Checkpointing] Add options to disable creating job-id subdirectories under the checkpoint directory #23509
Conversation
@masteryhx @Myasuka What do you think about adding this option? |
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.
Thanks for the PR, PTAL my comments.
Could you also add an IT to test switching this option which is important for users to use it out of the box ?
flink-core/src/main/java/org/apache/flink/configuration/CheckpointingOptions.java
Outdated
Show resolved
Hide resolved
flink-core/src/main/java/org/apache/flink/configuration/CheckpointingOptions.java
Show resolved
Hide resolved
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.
Thanks for creating this PR, please take a look at my comments.
...rc/main/java/org/apache/flink/runtime/state/memory/MemoryBackendCheckpointStorageAccess.java
Show resolved
Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/filesystem/FsStateBackend.java
Outdated
Show resolved
Hide resolved
@masteryhx @Myasuka Thanks for your review! I have addressed your comments, would you please take another look? Thanks! |
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.
Thanks for the update.
Current PR LGTM % the conflict.
Could you rebase all commits into master to resolve the conflict ?
BTW, please remember to rename the title with component tag.
…ubdirectories under the checkpoint directory
Already done, thanks @masteryhx |
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.
LGTM.
…ubdirectories under the checkpoint directory(#23509)
What is the purpose of the change
By default, Flink creates subdirectories named by UUID (job id) under checkpoint directory for each job. It's a good means to avoid collision. However, it also bring in some effort to remember/find the right directory when recovering from previous checkpoint, as well as cleaning the old subdirectories. According to previous discussion (Yun Tang's and Stephan Ewen's ), I think it would be useful to add an option to disable creating the UUID subdirectories under the checkpoint directory. For compatibility considerations, we create the subdirectories by default.
Brief change log
Verifying this change
This change is already covered by newly added tests, a parameterized AbstractFileCheckpointStorageAccessTestBase#testCreateCheckpointSubDirs .
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation