[ 
https://issues.apache.org/jira/browse/HIVE-26887?focusedWorklogId=837442&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837442
 ]

ASF GitHub Bot logged work on HIVE-26887:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 06/Jan/23 09:31
            Start Date: 06/Jan/23 09:31
    Worklog Time Spent: 10m 
      Work Description: skysiders commented on PR #3894:
URL: https://github.com/apache/hive/pull/3894#issuecomment-1373394928

   Hi @cnauroth ,I thought about this question again, and I think that compared 
with FileSystem.mkdirs(fs,path,perms) mentioned by you, my patch may be a 
little faster. The mkdirs(fs,path,perms) function does two things, first 
creating a file with default permissions and then assigning permissions to that 
file. What I do can be divided into three steps, first create the file, then 
determine if the file permissions are correct, if not set permissions. I think 
the reason for the speed here is that the permission judgment is faster than 
setting the permission directly, only if the file itself has the same 
permission and the permission to set, it can be about 4 times faster. According 
to what you said, this patch should modify all mkdirs(path,perms) into 
mkdirs(fs,path,perms), but as I just said, I think it will be a little slower 
than my current patch, but this kind of file creation is also rigorous.I‘m 
looking forward to your reply.




Issue Time Tracking
-------------------

    Worklog Id:     (was: 837442)
    Time Spent: 40m  (was: 0.5h)

> Make sure dirPath has the correct permissions
> ---------------------------------------------
>
>                 Key: HIVE-26887
>                 URL: https://issues.apache.org/jira/browse/HIVE-26887
>             Project: Hive
>          Issue Type: Improvement
>            Reporter: Zhang Dongsheng
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> In the QueryResultsCache function of class QueryResultsCache, there is the 
> following code segment
> {code:java}
>   private QueryResultsCache(HiveConf configuration) throws IOException {
>     ......
>     FileSystem fs = cacheDirPath.getFileSystem(conf);
>     FsPermission fsPermission = new FsPermission("700");
>     fs.mkdirs(cacheDirPath, fsPermission);
>     ......
> }
> {code}
> It can be seen that the function will use the mkdirs to create cacheDirPath, 
> and the parameters passed in include the path variable cacheDirPath and a 
> permission 700. But we haven't confirmed whether the permission is correctly 
> assigned to the file.
> The above question is raised because there are two mkdir functions of hadoop, 
> {code:java}
> mkdirs(Path f, FsPermission permission)
> {code}
>  and 
> {code:java}
> mkdirs(FileSystem fs, Path dir, FsPermission permission)
> {code}
> and the first one is used here. The permissions of this function will be 
> affected by the underlying umask. Although 700 here will hardly be affected 
> by umask, but I think from a rigorous point of view, we should have one more 
> permission check and permission grant here.
> And I find same issue in other three methods here.
> In class Context
> {code:java}
> private Path getScratchDir(String scheme, String authority,
>       boolean mkdir, String scratchDir) {
>           ......
>           FileSystem fs = dirPath.getFileSystem(conf);
>           dirPath = new Path(fs.makeQualified(dirPath).toString());
>           FsPermission fsPermission = new FsPermission(scratchDirPermission);
>           if (!fs.mkdirs(dirPath, fsPermission)) {
>             throw new RuntimeException("Cannot make directory: "
>                 + dirPath.toString());
>           ......
>   }
> {code}
> In class SessionState
> {code:java}
>   static void createPath(HiveConf conf, Path path, String permission, boolean 
> isLocal,
>       boolean isCleanUp) throws IOException {
>     FsPermission fsPermission = new FsPermission(permission);
>     FileSystem fs;
>     ......
>     if (!fs.mkdirs(path, fsPermission)) {
>       throw new IOException("Failed to create directory " + path + " on fs " 
> + fs.getUri());
>     }
>     ......
>   }
> {code}
> and in class TezSessionState
> {code:java}
> private Path createTezDir(String sessionId, String suffix) throws IOException 
> {
>     ......
>     Path tezDir = new Path(hdfsScratchDir, TEZ_DIR);
>     FileSystem fs = tezDir.getFileSystem(conf);
>     FsPermission fsPermission = new FsPermission(HiveConf.getVar(conf, 
> HiveConf.ConfVars.SCRATCHDIRPERMISSION));
>     fs.mkdirs(tezDir, fsPermission);
>     ......
>   }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to