This is an automated email from the ASF dual-hosted git repository.
gurwls223 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 531ec8bddc8 [SPARK-44300][CONNECT][BUG-FIX] Fix artifact cleanup to
limit deletion scope to session specific artifacts
531ec8bddc8 is described below
commit 531ec8bddc8dd22ca39486dbdd31e62e989ddc15
Author: vicennial <[email protected]>
AuthorDate: Wed Jul 5 12:06:33 2023 +0900
[SPARK-44300][CONNECT][BUG-FIX] Fix artifact cleanup to limit deletion
scope to session specific artifacts
### What changes were proposed in this pull request?
Modify the directory deletion in
`SparkConnectArtifactManager#cleanUpResources` to target the session-specific
artifact directory instead of the root artifact directory.
### Why are the changes needed?
Currently, when `SparkConnectArtifactManager#cleanUpResources` is called,
it would lead to the deletion of **all** artifacts instead of session-specific
ones. This breaks resource isolation among sessions when the bug is triggered.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
New unit test in `ArtifactManagerSuite` that verifies that the correct
directory is deleted as well as the existence of the root directory.
Closes #41854 from vicennial/SPARK-44300.
Authored-by: vicennial <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
---
.../connect/artifact/SparkConnectArtifactManager.scala | 2 +-
.../sql/connect/artifact/ArtifactManagerSuite.scala | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git
a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/artifact/SparkConnectArtifactManager.scala
b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/artifact/SparkConnectArtifactManager.scala
index 9fd8e367e4a..449ba011c21 100644
---
a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/artifact/SparkConnectArtifactManager.scala
+++
b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/artifact/SparkConnectArtifactManager.scala
@@ -188,7 +188,7 @@ class SparkConnectArtifactManager(sessionHolder:
SessionHolder) extends Logging
blockManager.removeCache(sessionHolder.userId, sessionHolder.sessionId)
// Clean up artifacts folder
- FileUtils.deleteDirectory(artifactRootPath.toFile)
+ FileUtils.deleteDirectory(artifactPath.toFile)
}
private[connect] def uploadArtifactToFs(
diff --git
a/connector/connect/server/src/test/scala/org/apache/spark/sql/connect/artifact/ArtifactManagerSuite.scala
b/connector/connect/server/src/test/scala/org/apache/spark/sql/connect/artifact/ArtifactManagerSuite.scala
index 612bf096b22..345e458cd2f 100644
---
a/connector/connect/server/src/test/scala/org/apache/spark/sql/connect/artifact/ArtifactManagerSuite.scala
+++
b/connector/connect/server/src/test/scala/org/apache/spark/sql/connect/artifact/ArtifactManagerSuite.scala
@@ -274,6 +274,24 @@ class ArtifactManagerSuite extends SharedSparkSession with
ResourceHelper {
assert(result.forall(_.getString(0).contains("Ahri")))
}
}
+
+ test("SPARK-44300: Cleaning up resources only deletes session-specific
resources") {
+ val copyDir = Utils.createTempDir().toPath
+ FileUtils.copyDirectory(artifactPath.toFile, copyDir.toFile)
+ val stagingPath = copyDir.resolve("Hello.class")
+ val remotePath = Paths.get("classes/Hello.class")
+
+ val sessionHolder = SparkConnectService.getOrCreateIsolatedSession("c1",
"session")
+ sessionHolder.addArtifact(remotePath, stagingPath, None)
+
+ val sessionDirectory =
+
SparkConnectArtifactManager.getArtifactDirectoryAndUriForSession(sessionHolder)._1.toFile
+ assert(sessionDirectory.exists())
+
+ sessionHolder.artifactManager.cleanUpResources()
+ assert(!sessionDirectory.exists())
+ assert(SparkConnectArtifactManager.artifactRootPath.toFile.exists())
+ }
}
class ArtifactUriSuite extends SparkFunSuite with LocalSparkContext {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
