Skip to content

[SPARK-41943][CORE] Use java api to create files and grant permissions #39448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
smallzhongfeng wants to merge 3 commits into from
Closed

[SPARK-41943][CORE] Use java api to create files and grant permissions #39448

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36a8b1f
[CORE] Use java api to create files and grant permissions
Jan 7, 2023
f2e02e8
fix log
Jan 8, 2023
696b7f5
just trigger GA
Jan 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions core/src/main/scala/org/apache/spark/storage/DiskBlockManager.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.apache.spark.storage

import java.io.{File, IOException}
import java.nio.file.Files
import java.nio.file.attribute.PosixFilePermission
import java.nio.file.attribute.{PosixFilePermission, PosixFilePermissions}
import java.util.UUID

import scala.collection.mutable.HashMap
Expand Down Expand Up @@ -301,9 +301,6 @@ private[spark] class DiskBlockManager(
* Create a directory that is writable by the group.
* Grant the permission 770 "rwxrwx---" to the directory so the shuffle server can
* create subdirs/files within the merge folder.
* TODO: Find out why can't we create a dir using java api with permission 770
Copy link
Contributor Author

@smallzhongfeng smallzhongfeng Jan 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For TODO, maybe we can see https://bugs.openjdk.org/browse/JDK-8220013

Copy link
Member

@HyukjinKwon HyukjinKwon Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind summarizing why we can use this approach while the JDK issue isn't resolved?

Copy link
Contributor Author

@smallzhongfeng smallzhongfeng Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In unix like systems every process has a property called umask which is masked onto the permissions of any file created and inherited by child processes. the default is 0002 or "turn off write for others". So it's most likely that Java is setting the permission you seek and then it's being masked out. At present, this issue has not been resolved, so my current approach is to create a directory first, and then explicitly grant permissions.

* Files.createDirectories(mergeDir.toPath, PosixFilePermissions.asFileAttribute(
* PosixFilePermissions.fromString("rwxrwx---")))
*/
def createDirWithPermission770(dirToCreate: File): Unit = {
var attempts = 0
Expand All @@ -315,16 +312,13 @@ private[spark] class DiskBlockManager(
throw SparkCoreErrors.failToCreateDirectoryError(dirToCreate.getAbsolutePath, maxAttempts)
}
try {
val builder = new ProcessBuilder().command(
"mkdir", "-p", "-m770", dirToCreate.getAbsolutePath)
val proc = builder.start()
val exitCode = proc.waitFor()
dirToCreate.mkdirs()
Files.setPosixFilePermissions(
dirToCreate.toPath, PosixFilePermissions.fromString("rwxrwx---"))
if (dirToCreate.exists()) {
created = dirToCreate
}
logDebug(
s"Created directory at ${dirToCreate.getAbsolutePath} with permission " +
s"770 and exitCode $exitCode")
logDebug(s"Created directory at ${dirToCreate.getAbsolutePath} with permission 770")
} catch {
case e: SecurityException =>
logWarning(s"Failed to create directory ${dirToCreate.getAbsolutePath} " +
Expand Down