[GitHub] [spark] LuciferYang commented on a diff in pull request #39226: [SPARK-41694][CORE] Add new config to clean up `spark.ui.store.path` directory when `SparkContext.stop()`

[GitBox <gi...@apache.org>]

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


##########
core/src/main/scala/org/apache/spark/status/AppStatusStore.scala:
##########
@@ -733,6 +734,15 @@ private[spark] class AppStatusStore(
 
   def close(): Unit = {
     store.close()
+    cleanUpStorePath()
+  }
+
+  private def cleanUpStorePath(): Unit = {
+    storePath.foreach { p =>
+      if (p.exists()) {
+        p.listFiles().foreach(Utils.deleteRecursively)
+      }
+    }

Review Comment:
   The current code uses a strategy similar to `DiskBlockManager` to generate a temporary directory for RocksDB (without exposing `spark.ui.store.path` to users).
   
   However, I think it is reasonable to expose `spark.ui.store.path` to users: when multiple long running Spark Apps(client mode) are running on same machine  and RockDB is used to store the Live UI, the default use of `/tmp`  to store data may cause `no space`, because `/tmp` directory is generally limited (about 5~7G space)
   
   WDYT @gengliangwang @mridulm ? If `spark.ui.store.path` needs to be exposed to users, I need to revert the last commit.
   
   



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