You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2020/03/17 18:22:00 UTC

[spark] branch master updated: [SPARK-30860][CORE] Use FileSystem.mkdirs to avoid umask at rolling event log folder and appStatusFile creation

This is an automated email from the ASF dual-hosted git repository.

dongjoon 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 9f27a54  [SPARK-30860][CORE] Use FileSystem.mkdirs to avoid umask at rolling event log folder and appStatusFile creation
9f27a54 is described below

commit 9f27a5495d0114723701c932f20f32d308a571cc
Author: Adam Binford <ad...@radiantsolutions.com>
AuthorDate: Tue Mar 17 11:20:10 2020 -0700

    [SPARK-30860][CORE] Use FileSystem.mkdirs to avoid umask at rolling event log folder and appStatusFile creation
    
    ### What changes were proposed in this pull request?
    This pull request fixes an issue with rolling event logs. The rolling event log directory is created ignoring the dfs umask setting. This allows the history server to prune old rolling logs when run as the group owner of the event log folder.
    
    ### Why are the changes needed?
    For non-rolling event logs, log files are created ignoring the umask setting by calling setPermission after creating the file. The default umask of 022 currently causes rolling log directories to be created without group write permissions, preventing the history server from pruning logs of applications not run as the same user as the history server. This adds the same behavior for rolling event logs so users don't need to worry about the umask setting causing different behavior.
    
    ### Does this PR introduce any user-facing change?
    No
    
    ### How was this patch tested?
    Manually. The folder is created with the correct 770 permission. The status file is still affected by the umask setting, but that doesn't stop the folder from being deleted by the history server. I'm not sure if that causes any other issues. I'm not sure how to test something involving a Hadoop setting.
    
    Closes #27764 from Kimahriman/bug/rolling-log-permissions.
    
    Authored-by: Adam Binford <ad...@radiantsolutions.com>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../org/apache/spark/deploy/history/EventLogFileWriters.scala  | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/core/src/main/scala/org/apache/spark/deploy/history/EventLogFileWriters.scala b/core/src/main/scala/org/apache/spark/deploy/history/EventLogFileWriters.scala
index 1d58d05..7d44cbd 100644
--- a/core/src/main/scala/org/apache/spark/deploy/history/EventLogFileWriters.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/history/EventLogFileWriters.scala
@@ -166,7 +166,8 @@ object EventLogFileWriter {
   val IN_PROGRESS = ".inprogress"
   val COMPACTED = ".compact"
 
-  val LOG_FILE_PERMISSIONS = new FsPermission(Integer.parseInt("770", 8).toShort)
+  val LOG_FILE_PERMISSIONS = new FsPermission(Integer.parseInt("660", 8).toShort)
+  val LOG_FOLDER_PERMISSIONS = new FsPermission(Integer.parseInt("770", 8).toShort)
 
   def apply(
       appId: String,
@@ -317,7 +318,8 @@ class RollingEventLogFilesWriter(
       throw new IOException(s"Target log directory already exists ($logDirForAppPath)")
     }
 
-    fileSystem.mkdirs(logDirForAppPath, EventLogFileWriter.LOG_FILE_PERMISSIONS)
+    // SPARK-30860: use the class method to avoid the umask causing permission issues
+    FileSystem.mkdirs(fileSystem, logDirForAppPath, EventLogFileWriter.LOG_FOLDER_PERMISSIONS)
     createAppStatusFile(inProgress = true)
     rollEventLogFile()
   }
@@ -361,7 +363,9 @@ class RollingEventLogFilesWriter(
 
   private def createAppStatusFile(inProgress: Boolean): Unit = {
     val appStatusPath = getAppStatusFilePath(logDirForAppPath, appId, appAttemptId, inProgress)
-    val outputStream = fileSystem.create(appStatusPath)
+    // SPARK-30860: use the class method to avoid the umask causing permission issues
+    val outputStream = FileSystem.create(fileSystem, appStatusPath,
+      EventLogFileWriter.LOG_FILE_PERMISSIONS)
     // we intentionally create zero-byte file to minimize the cost
     outputStream.close()
   }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org