You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/01/29 12:56:29 UTC

[GitHub] [flink] tillrohrmann commented on a change in pull request #14777: [FLINK-21164][rest] Delete temporary jars

tillrohrmann commented on a change in pull request #14777:
URL: https://github.com/apache/flink/pull/14777#discussion_r566799528



##########
File path: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java
##########
@@ -103,6 +103,11 @@ public JarRunHandler(
                         executor)
                 .handle(
                         (jobIds, throwable) -> {
+                            try {
+                                program.deleteExtractedLibraries();
+                            } catch (Exception e) {
+                                log.debug("Error while deleting jars extracted from user-jar.");

Review comment:
       ```suggestion
                                   log.debug("Error while deleting jars extracted from user-jar.", e);
   ```

##########
File path: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarPlanHandler.java
##########
@@ -100,8 +101,19 @@ public JarPlanHandler(
 
         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.");

Review comment:
       ```suggestion
                               log.debug("Error while deleting jars extracted from user-jar.", e);
   ```

##########
File path: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java
##########
@@ -103,6 +103,11 @@ public JarRunHandler(
                         executor)
                 .handle(
                         (jobIds, throwable) -> {
+                            try {
+                                program.deleteExtractedLibraries();

Review comment:
       Isn't `JarListHandler` also susceptible to the problem?

##########
File path: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java
##########
@@ -103,6 +103,11 @@ public JarRunHandler(
                         executor)
                 .handle(
                         (jobIds, throwable) -> {
+                            try {
+                                program.deleteExtractedLibraries();

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




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org