[FLINK-32974][client] Avoid creating a new temporary directory every time for RestClusterClient #23363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @zentol , could you help to review this PR? |
wanglijie95
force-pushed
the
FLINK-32974
branch
from
925630d
to
b7541ba
Compare
wanglijie95
changed the title
zhuzhurk
reviewed
Sep 20, 2023
| Files.createDirectories( | ||
| Paths.get( | ||
| ConfigurationUtils.parseTempDirectories(config)[0], | ||
| "flink-rest-client-jobgraphs"))); |
There was a problem hiding this comment.
Looks to me there's no need to create an additional folder here.
If the submission succeeds, job graph files will be cleaned up, while leaving this folder remained in the tmp dir.
…time for RestClusterClient
wanglijie95
force-pushed
the
FLINK-32974
branch
from
b7541ba
to
06a7afe
Compare
wanglijie95
commented
Sep 20, 2023
| @@ -359,7 +333,8 @@ public CompletableFuture<JobID> submitJob(@Nonnull JobGraph jobGraph) { | |||
| () -> { | |||
| try { | |||
| final java.nio.file.Path jobGraphFile = | |||
| Files.createTempFile(tempDir, "flink-jobgraph", ".bin"); | |||
| Files.createTempFile( | |||
| "flink-jobgraph-" + jobGraph.getJobID(), ".bin"); | |||
There was a problem hiding this comment.
I have to add the jobid into the job graph file name, in order to check whether the job graph file has been deleted in testJobGraphFileCleanedUpOnJobSubmissionFailure. cc @zhuzhurk
wanglijie95
added a commit
that referenced
this pull request
Sep 21, 2023
…time for RestClusterClient This closes #23363
wanglijie95
added a commit
that referenced
this pull request
Sep 21, 2023
…time for RestClusterClient This closes #23363
RocMarshal
pushed a commit
to RocMarshal/flink
that referenced
this pull request
May 9, 2024
…time for RestClusterClient This closes apache#23363
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
Fix FLINK-32974
After FLINK-32226, when creating a new RestClusterClient, a temporary directory (named
flink-rest-client-jobgraphsxxx) is created, but the directory is never cleaned up, which may cause the inode to be used up.In this PR, we try to solve this problem by using fixed directory
flink-rest-client-jobgraphsinstead of creating a temporary directoryflink-rest-client-jobgraphsxxxevery timeVerifying this change
Add test
RestClusterClientTest#testTempDirCleanedUpOnCloseDoes this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation