Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FLINK-21164][rest] Delete temporary jars #14777

Merged
merged 5 commits into from
Jan 29, 2021
Merged

[FLINK-21164][rest] Delete temporary jars #14777

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8eded2f
[FLINK-21164][rest] Delete temporary jars
zentol Jan 27, 2021
b1c458c
Update flink-runtime-web/src/main/java/org/apache/flink/runtime/webmo…
zentol Jan 29, 2021
e5bef52
Update flink-runtime-web/src/main/java/org/apache/flink/runtime/webmo…
zentol Jan 29, 2021
ce3c983
Add PackagedProgram#close()
zentol Jan 29, 2021
c833d99
migrate CliFrontend to PackagedProgram#close
zentol Jan 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions ...untime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarPlanHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.flink.runtime.webmonitor.handlers;

import org.apache.flink.api.common.time.Time;
import org.apache.flink.client.program.PackagedProgram;
import org.apache.flink.configuration.Configuration;
import org.apache.flink.runtime.jobgraph.JobGraph;
import org.apache.flink.runtime.jobgraph.jsonplan.JsonPlanGenerator;
Expand Down Expand Up @@ -100,8 +101,19 @@ protected CompletableFuture<JobPlanInfo> handleRequest(

return CompletableFuture.supplyAsync(
() -> {
final JobGraph jobGraph = context.toJobGraph(configuration, true);
return planGenerator.apply(jobGraph);
final PackagedProgram packagedProgram =
context.toPackagedProgram(configuration);
try {
final JobGraph jobGraph =
context.toJobGraph(packagedProgram, configuration, true);
return planGenerator.apply(jobGraph);
} finally {
try {
packagedProgram.deleteExtractedLibraries();
} catch (Exception e) {
log.debug("Error while deleting jars extracted from user-jar.");
zentol marked this conversation as resolved.
Show resolved Hide resolved
}
}
},
executor);
}
Expand Down
5 changes: 5 additions & 0 deletions ...runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ protected CompletableFuture<JarRunResponseBody> handleRequest(
executor)
.handle(
(jobIds, throwable) -> {
try {
program.deleteExtractedLibraries();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't JarListHandler also susceptible to the problem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also wondering whether the ApplicationClusterEntryPoint really cleans up a PackagedProgram after using. Maybe this is something to investigate as a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the JarListHandler has the same problem. I did not know it actually creates a PackagedProgram.

As for the ApplicationClusterEntryPoint, the files should be cleaned up when the entry point shuts down because we call File#deleteOnExit in PackagedProgram#createTempFile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now whether they are actually deleted depends a bit on when the JVM executes the delete calls. If the ClassLoader is still around at that time, then the deletion may in fact fail.

Explicitly closing the ClassLoader in the PackagedProgram (which unfortunately leaks into the JobGraph...) is a separate issue (see FLINK-9844).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty. Thanks for the update.

} catch (Exception e) {
log.debug("Error while deleting jars extracted from user-jar.");
zentol marked this conversation as resolved.
Show resolved Hide resolved
}
if (throwable != null) {
throw new CompletionException(
new RestHandlerException(
Expand Down
6 changes: 4 additions & 2 deletions ...web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/utils/JarHandlerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,11 @@ public void applyToConfiguration(final Configuration configuration) {
URL::toString);
}

public JobGraph toJobGraph(Configuration configuration, boolean suppressOutput) {
public JobGraph toJobGraph(
PackagedProgram packagedProgram,
Configuration configuration,
boolean suppressOutput) {
try {
final PackagedProgram packagedProgram = toPackagedProgram(configuration);
return PackagedProgramUtils.createJobGraph(
packagedProgram, configuration, parallelism, jobId, suppressOutput);
} catch (final ProgramInvocationException e) {
Expand Down