You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/03/30 03:56:00 UTC

[GitHub] [spark] LuciferYang commented on a diff in pull request #36529: [SPARK-39102][CORE][SQL][DSTREAM] Add checkstyle rules to disabled use of Guava's `Files.createTempDir()`

LuciferYang commented on code in PR #36529:
URL: https://github.com/apache/spark/pull/36529#discussion_r1152702218


##########
common/network-common/src/main/java/org/apache/spark/network/util/JavaUtils.java:
##########
@@ -362,6 +364,60 @@ public static byte[] bufferToArray(ByteBuffer buffer) {
     }
   }
 
+  /**
+   * Create a temporary directory inside `java.io.tmpdir` with default namePrefix "spark".
+   * The directory will be automatically deleted when the VM shuts down.
+   */
+  public static File createTempDir() throws IOException {
+    return createTempDir(System.getProperty("java.io.tmpdir"), "spark");
+  }
+
+  /**
+   * Create a temporary directory inside the given parent directory. The directory will be
+   * automatically deleted when the VM shuts down.
+   */
+  public static File createTempDir(String root, String namePrefix) throws IOException {

Review Comment:
   This pr copying the logic of `o.a.spark.util.Utils#createTempDir` to a more basic module due to some underlying modules cannot call `o.a.spark.util.Utils#createTempDir` and in https://github.com/apache/spark/pull/36611 `o.a.spark.util.Utils#createTempDir` also call this method instead.
   
   Initially, `o.a.spark.util.ShutdownHookManager#registerShutdownDeleteDir` was used in `o.a.spark.util.Utils#createTempDir` to clean up temporary files:
   
   https://github.com/apache/spark/blob/12e7991b5b38302e6496307c7263ad729c82a6cf/core/src/main/scala/org/apache/spark/util/ShutdownHookManager.scala#L72-L78
   
   But it also cannot be accessed in current module, so here it is changed to use `deleteOnExit`. Personally, there is no essential difference between `registerShutdownDeleteDir` and `deleteOnExit`, if I understand incorrectly, please correct me.
   
   
   @sadikovi What is the bad case you found? Running UT require more memory or other scenarios? What is your suggestion?
   
   
   
   
   
   
   
   
   
   
   
   
   
   
   
   



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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org