From b2ebf505248b3f63143dee52839807c13cc7acca Mon Sep 17 00:00:00 2001 From: Mridul Muralidharan Date: Fri, 6 May 2022 12:02:07 -0500 Subject: [PATCH 1/7] Fix RC test failure in SPARK-37618 --- core/pom.xml | 5 ++ .../spark/storage/DiskBlockManagerSuite.scala | 49 ++++++++++++------- pom.xml | 7 +++ 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 7d4bab556596..80578417b05e 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -61,6 +61,11 @@ com.twitter chill-java + + com.github.jnr + jnr-posix + test + org.apache.xbean xbean-asm9-shaded 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 58fe40f9adeb..d0f1ef760b7e 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.POSIXFactory import org.apache.commons.io.FileUtils import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} @@ -141,28 +142,42 @@ 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(mask: String): String = { + val posix = POSIXFactory.getPOSIX + 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 oldUmask = getAndSetUmask("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(oldUmask) + } } def writeToFile(file: File, numBytes: Int): Unit = { diff --git a/pom.xml b/pom.xml index 6081f700b628..b57963789778 100644 --- a/pom.xml +++ b/pom.xml @@ -136,6 +136,7 @@ 9.4.46.v20220331 4.0.3 0.10.0 + 3.0.9 2.5.0 2.0.8 From ed1e809bb8a8602692b5d4eb66b58d6b2f4da1e1 Mon Sep 17 00:00:00 2001 From: Mridul Muralidharan Date: Fri, 6 May 2022 12:11:25 -0500 Subject: [PATCH 2/7] Handle lack of native posix support --- .../spark/storage/DiskBlockManagerSuite.scala | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) 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 d0f1ef760b7e..c272a096598d 100644 --- a/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala +++ b/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala @@ -21,13 +21,11 @@ import java.io.{File, FileWriter} import java.nio.file.{Files, Paths} import java.nio.file.attribute.{PosixFilePermission, PosixFilePermissions} import java.util.HashMap - import com.fasterxml.jackson.core.`type`.TypeReference import com.fasterxml.jackson.databind.ObjectMapper -import jnr.posix.POSIXFactory +import jnr.posix.{POSIX, POSIXFactory} import org.apache.commons.io.FileUtils import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} - import org.apache.spark.{SparkConf, SparkFunSuite} import org.apache.spark.internal.config import org.apache.spark.util.Utils @@ -144,8 +142,7 @@ class DiskBlockManagerSuite extends SparkFunSuite with BeforeAndAfterEach with B // Use jnr to get and override the current process umask. // Expects the input mask to be an octal number - private def getAndSetUmask(mask: String): String = { - val posix = POSIXFactory.getPOSIX + private def getAndSetUmask(posix: POSIX, mask: String): String = { val prev = posix.umask(BigInt(mask, 8).toInt) "0" + "%o".format(prev) } @@ -155,28 +152,35 @@ class DiskBlockManagerSuite extends SparkFunSuite with BeforeAndAfterEach with B conf.set("spark.local.dir", rootDirs) conf.set("spark.shuffle.service.enabled", "true") conf.set("spark.shuffle.service.removeShuffle", "false") + val posix = POSIXFactory.getPOSIX - val oldUmask = getAndSetUmask("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(oldUmask) + if (posix.isNative) { + 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) + } + } else { + logInfo("Skipping test for SPARK-37618, native posix support not found") } } From a4940e1d89a7221b109d1364826bc11dfda90c1a Mon Sep 17 00:00:00 2001 From: Mridul Muralidharan Date: Fri, 6 May 2022 12:29:10 -0500 Subject: [PATCH 3/7] Fix intellij removing import whitespaces --- .../scala/org/apache/spark/storage/DiskBlockManagerSuite.scala | 2 ++ 1 file changed, 2 insertions(+) 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 c272a096598d..95a27f737ae6 100644 --- a/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala +++ b/core/src/test/scala/org/apache/spark/storage/DiskBlockManagerSuite.scala @@ -21,11 +21,13 @@ import java.io.{File, FileWriter} import java.nio.file.{Files, Paths} import java.nio.file.attribute.{PosixFilePermission, PosixFilePermissions} 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} + import org.apache.spark.{SparkConf, SparkFunSuite} import org.apache.spark.internal.config import org.apache.spark.util.Utils From afb7346ed697210473aca3d83143d474e5c41405 Mon Sep 17 00:00:00 2001 From: Mridul Muralidharan Date: Fri, 6 May 2022 13:08:56 -0500 Subject: [PATCH 4/7] Add to license --- LICENSE-binary | 1 + 1 file changed, 1 insertion(+) diff --git a/LICENSE-binary b/LICENSE-binary index a090e5205cc1..e8ad55c93545 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 From c3608d0d9210f35fd18cb71e42cf142e8dfb516f Mon Sep 17 00:00:00 2001 From: Mridul Muralidharan Date: Fri, 6 May 2022 19:23:11 -0500 Subject: [PATCH 5/7] Ensure latest version of jnr - picked old by mistake --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b57963789778..51db6d02ade5 100644 --- a/pom.xml +++ b/pom.xml @@ -136,7 +136,7 @@ 9.4.46.v20220331 4.0.3 0.10.0 - 3.0.9 + 3.1.15 2.5.0 2.0.8