This is an automated email from the ASF dual-hosted git repository.
srowen pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new e9ee2c8787a [SPARK-37618][CORE][FOLLOWUP] Support cleaning up shuffle
blocks from external shuffle service
e9ee2c8787a is described below
commit e9ee2c8787adfc07a7d1882b7749dd41c2990048
Author: Mridul Muralidharan <mridulatgmail.com>
AuthorDate: Sun May 8 08:11:19 2022 -0500
[SPARK-37618][CORE][FOLLOWUP] Support cleaning up shuffle blocks from
external shuffle service
### What changes were proposed in this pull request?
Fix test failure in build.
Depending on the umask of the process running tests (which is typically
inherited from the user's default umask), the group writable bit for the
files/directories could be set or unset. The test was assuming that by default
the umask will be restrictive (and so files/directories wont be group
writable). Since this is not a valid assumption, we use jnr to change the umask
of the process to be more restrictive - so that the test can validate the
behavior change - and reset it back once [...]
### Why are the changes needed?
Fix test failure in build
### Does this PR introduce _any_ user-facing change?
No
Adds jnr as a test scoped dependency, which does not bring in any other new
dependency (asm is already a dep in spark).
```
[INFO] +- com.github.jnr:jnr-posix:jar:3.0.9:test
[INFO] | +- com.github.jnr:jnr-ffi:jar:2.0.1:test
[INFO] | | +- com.github.jnr:jffi:jar:1.2.7:test
[INFO] | | +- com.github.jnr:jffi:jar:native:1.2.7:test
[INFO] | | +- org.ow2.asm:asm:jar:5.0.3:test
[INFO] | | +- org.ow2.asm:asm-commons:jar:5.0.3:test
[INFO] | | +- org.ow2.asm:asm-analysis:jar:5.0.3:test
[INFO] | | +- org.ow2.asm:asm-tree:jar:5.0.3:test
[INFO] | | +- org.ow2.asm:asm-util:jar:5.0.3:test
[INFO] | | \- com.github.jnr:jnr-x86asm:jar:1.0.2:test
[INFO] | \- com.github.jnr:jnr-constants:jar:0.8.6:test
```
### How was this patch tested?
Modification to existing test.
Tested on Linux, skips test when native posix env is not found.
Closes #36473 from mridulm/fix-SPARK-37618-test.
Authored-by: Mridul Muralidharan <mridulatgmail.com>
Signed-off-by: Sean Owen <sro...@gmail.com>
(cherry picked from commit 317407171cb36439c371153cfd45c1482bf5e425)
Signed-off-by: Sean Owen <sro...@gmail.com>
---
LICENSE-binary | 1 +
core/pom.xml | 5 ++
.../spark/storage/DiskBlockManagerSuite.scala | 53 +++++++++++++++-------
pom.xml | 6 +++
4 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/LICENSE-binary b/LICENSE-binary
index 8bbc913262c..40e2e389b22 100644
--- a/LICENSE-binary
+++ b/LICENSE-binary
@@ -546,6 +546,7 @@ jakarta.annotation:jakarta-annotation-api
https://projects.eclipse.org/projects/
jakarta.servlet:jakarta.servlet-api
https://projects.eclipse.org/projects/ee4j.servlet
jakarta.ws.rs:jakarta.ws.rs-api https://github.com/eclipse-ee4j/jaxrs-api
org.glassfish.hk2.external:jakarta.inject
+com.github.jnr:jnr-posix
Public Domain
diff --git a/core/pom.xml b/core/pom.xml
index 68d1089e34c..ac644130a61 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -61,6 +61,11 @@
<groupId>com.twitter</groupId>
<artifactId>chill-java</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.jnr</groupId>
+ <artifactId>jnr-posix</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-asm9-shaded</artifactId>
diff --git
a/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala
b/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala
index 58fe40f9ade..3e4002614ca 100644
--- a/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala
+++ b/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala
@@ -24,6 +24,7 @@ import java.util.HashMap
import com.fasterxml.jackson.core.`type`.TypeReference
import com.fasterxml.jackson.databind.ObjectMapper
+import jnr.posix.{POSIX, POSIXFactory}
import org.apache.commons.io.FileUtils
import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
@@ -141,28 +142,46 @@ class DiskBlockManagerSuite extends SparkFunSuite with
BeforeAndAfterEach with B
assert(attemptId.equals("1"))
}
+ // Use jnr to get and override the current process umask.
+ // Expects the input mask to be an octal number
+ private def getAndSetUmask(posix: POSIX, mask: String): String = {
+ val prev = posix.umask(BigInt(mask, 8).toInt)
+ "0" + "%o".format(prev)
+ }
+
test("SPARK-37618: Sub dirs are group writable when removing from shuffle
service enabled") {
val conf = testConf.clone
conf.set("spark.local.dir", rootDirs)
conf.set("spark.shuffle.service.enabled", "true")
conf.set("spark.shuffle.service.removeShuffle", "false")
- val diskBlockManager = new DiskBlockManager(conf, deleteFilesOnStop =
true, isDriver = false)
- val blockId = new TestBlockId("test")
- val newFile = diskBlockManager.getFile(blockId)
- val parentDir = newFile.getParentFile()
- assert(parentDir.exists && parentDir.isDirectory)
- val permission = Files.getPosixFilePermissions(parentDir.toPath)
- assert(!permission.contains(PosixFilePermission.GROUP_WRITE))
-
- assert(parentDir.delete())
-
- conf.set("spark.shuffle.service.removeShuffle", "true")
- val diskBlockManager2 = new DiskBlockManager(conf, deleteFilesOnStop =
true, isDriver = false)
- val newFile2 = diskBlockManager2.getFile(blockId)
- val parentDir2 = newFile2.getParentFile()
- assert(parentDir2.exists && parentDir2.isDirectory)
- val permission2 = Files.getPosixFilePermissions(parentDir2.toPath)
- assert(permission2.contains(PosixFilePermission.GROUP_WRITE))
+ val posix = POSIXFactory.getPOSIX
+
+ assume(posix.isNative, "Skipping test for SPARK-37618, native posix
support not found")
+
+ val oldUmask = getAndSetUmask(posix, "077")
+ try {
+ val diskBlockManager = new DiskBlockManager(conf, deleteFilesOnStop =
true,
+ isDriver = false)
+ val blockId = new TestBlockId("test")
+ val newFile = diskBlockManager.getFile(blockId)
+ val parentDir = newFile.getParentFile()
+ assert(parentDir.exists && parentDir.isDirectory)
+ val permission = Files.getPosixFilePermissions(parentDir.toPath)
+ assert(!permission.contains(PosixFilePermission.GROUP_WRITE))
+
+ assert(parentDir.delete())
+
+ conf.set("spark.shuffle.service.removeShuffle", "true")
+ val diskBlockManager2 = new DiskBlockManager(conf, deleteFilesOnStop =
true,
+ isDriver = false)
+ val newFile2 = diskBlockManager2.getFile(blockId)
+ val parentDir2 = newFile2.getParentFile()
+ assert(parentDir2.exists && parentDir2.isDirectory)
+ val permission2 = Files.getPosixFilePermissions(parentDir2.toPath)
+ assert(permission2.contains(PosixFilePermission.GROUP_WRITE))
+ } finally {
+ getAndSetUmask(posix, oldUmask)
+ }
}
def writeToFile(file: File, numBytes: Int): Unit = {
diff --git a/pom.xml b/pom.xml
index 86305319429..c91167d8de6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -453,6 +453,12 @@
<artifactId>chill-java</artifactId>
<version>${chill.version}</version>
</dependency>
+ <dependency>
+ <groupId>com.github.jnr</groupId>
+ <artifactId>jnr-posix</artifactId>
+ <version>3.1.15</version>
+ <scope>test</scope>
+ </dependency>
<!-- This artifact is a shaded version of ASM 9.x. The POM that was used
to produce this
is at
https://github.com/apache/geronimo-xbean/tree/trunk/xbean-asm9-shaded
For context on why we shade ASM, see SPARK-782 and SPARK-6152. -->
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
