hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vgumas...@apache.org
Subject svn commit: r1624086 - in /hive/trunk: itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
Date Wed, 10 Sep 2014 18:15:32 GMT
Author: vgumashta
Date: Wed Sep 10 18:15:32 2014
New Revision: 1624086

URL: http://svn.apache.org/r1624086
Log:
HIVE-8022: Recursive root scratch directory creation is not using hdfs umask properly (Vaibhav
Gumashta reviewed by Thejas Nair)

Modified:
    hive/trunk/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
    hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
    hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java

Modified: hive/trunk/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
URL: http://svn.apache.org/viewvc/hive/trunk/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java?rev=1624086&r1=1624085&r2=1624086&view=diff
==============================================================================
--- hive/trunk/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
(original)
+++ hive/trunk/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Wed Sep 10 18:15:32 2014
@@ -46,6 +46,7 @@ import org.junit.Test;
 public class TestJdbcWithMiniHS2 {
   private static MiniHS2 miniHS2 = null;
   private static Path dataFilePath;
+  private static final String tmpDir = System.getProperty("test.tmp.dir");
 
   private Connection hs2Conn = null;
 
@@ -303,7 +304,7 @@ public class TestJdbcWithMiniHS2 {
    * @throws Exception
    */
   @Test
-  public void testScratchDirs() throws Exception {
+  public void testSessionScratchDirs() throws Exception {
     // Stop HiveServer2
     if (miniHS2.isStarted()) {
       miniHS2.stop();
@@ -314,7 +315,7 @@ public class TestJdbcWithMiniHS2 {
     // 1. Test with doAs=false
     conf.setBoolean("hive.server2.enable.doAs", false);
     // Set a custom prefix for hdfs scratch dir path
-    conf.set("hive.exec.scratchdir", "/tmp/hs2");
+    conf.set("hive.exec.scratchdir", tmpDir + "/hs2");
     // Set a scratch dir permission
     String fsPermissionStr = "700";
     conf.set("hive.scratch.dir.permission", fsPermissionStr);
@@ -326,19 +327,21 @@ public class TestJdbcWithMiniHS2 {
     hs2Conn = getConnection(miniHS2.getJdbcURL(), userName, "password");
     // FS
     FileSystem fs = miniHS2.getLocalFS();
+    FsPermission expectedFSPermission = new FsPermission(HiveConf.getVar(conf,
+        HiveConf.ConfVars.SCRATCHDIRPERMISSION));
 
     // Verify scratch dir paths and permission
     // HDFS scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR) + "/" +
userName);
-    verifyScratchDir(conf, fs, scratchDirPath, userName, false);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, false);
 
     // Local scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.LOCALSCRATCHDIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
 
     // Downloaded resources dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.DOWNLOADED_RESOURCES_DIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
 
     // 2. Test with doAs=true
     // Restart HiveServer2 with doAs=true
@@ -356,15 +359,15 @@ public class TestJdbcWithMiniHS2 {
     // Verify scratch dir paths and permission
     // HDFS scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR) + "/" +
userName);
-    verifyScratchDir(conf, fs, scratchDirPath, userName, false);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, false);
 
     // Local scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.LOCALSCRATCHDIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
 
     // Downloaded resources dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.DOWNLOADED_RESOURCES_DIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
 
     // Test for user "trinity"
     userName = "trinity";
@@ -373,22 +376,61 @@ public class TestJdbcWithMiniHS2 {
     // Verify scratch dir paths and permission
     // HDFS scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR) + "/" +
userName);
-    verifyScratchDir(conf, fs, scratchDirPath, userName, false);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, false);
 
     // Local scratch dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.LOCALSCRATCHDIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
 
     // Downloaded resources dir
     scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.DOWNLOADED_RESOURCES_DIR));
-    verifyScratchDir(conf, fs, scratchDirPath, userName, true);
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, true);
+  }
+
+  /**
+   * Tests the creation of the root hdfs scratch dir, which should be writable by all (777).
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testRootScratchDir() throws Exception {
+    // Stop HiveServer2
+    if (miniHS2.isStarted()) {
+      miniHS2.stop();
+    }
+    HiveConf conf = new HiveConf();
+    String userName;
+    Path scratchDirPath;
+    conf.set("hive.exec.scratchdir", tmpDir + "/hs2");
+    // Start an instance of HiveServer2 which uses miniMR
+    miniHS2 = new MiniHS2(conf);
+    Map<String, String> confOverlay = new HashMap<String, String>();
+    miniHS2.start(confOverlay);
+    userName = System.getProperty("user.name");
+    hs2Conn = getConnection(miniHS2.getJdbcURL(), userName, "password");
+    // FS
+    FileSystem fs = miniHS2.getLocalFS();
+    FsPermission expectedFSPermission = new FsPermission("777");
+    // Verify scratch dir paths and permission
+    // HDFS scratch dir
+    scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR));
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, false);
+    // Test with multi-level scratch dir path
+    // Stop HiveServer2
+    if (miniHS2.isStarted()) {
+      miniHS2.stop();
+    }
+    conf.set("hive.exec.scratchdir", tmpDir + "/level1/level2/level3");
+    miniHS2 = new MiniHS2(conf);
+    miniHS2.start(confOverlay);
+    hs2Conn = getConnection(miniHS2.getJdbcURL(), userName, "password");
+    scratchDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR));
+    verifyScratchDir(conf, fs, scratchDirPath, expectedFSPermission, userName, false);
   }
 
   private void verifyScratchDir(HiveConf conf, FileSystem fs, Path scratchDirPath,
-      String userName, boolean isLocal) throws Exception {
+      FsPermission expectedFSPermission, String userName, boolean isLocal) throws Exception
{
     String dirType = isLocal ? "Local" : "DFS";
-    FsPermission expectedFSPermission = new FsPermission(HiveConf.getVar(conf,
-        HiveConf.ConfVars.SCRATCHDIRPERMISSION));
     assertTrue("The expected " + dirType + " scratch dir does not exist for the user: " +
         userName, fs.exists(scratchDirPath));
     if (fs.exists(scratchDirPath) && !isLocal) {

Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java?rev=1624086&r1=1624085&r2=1624086&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java (original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java Wed Sep 10 18:15:32
2014
@@ -92,6 +92,7 @@ import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathFilter;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hive.common.HiveInterruptCallback;
 import org.apache.hadoop.hive.common.HiveInterruptUtils;
 import org.apache.hadoop.hive.common.HiveStatsUtils;
@@ -160,6 +161,7 @@ import org.apache.hadoop.hive.serde2.Ser
 import org.apache.hadoop.hive.serde2.Serializer;
 import org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe;
 import org.apache.hadoop.hive.shims.ShimLoader;
+import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.io.SequenceFile;
 import org.apache.hadoop.io.SequenceFile.CompressionType;
 import org.apache.hadoop.io.Text;
@@ -3423,6 +3425,41 @@ public final class Utilities {
     }
   }
 
+  public static boolean createDirsWithPermission(Configuration conf, Path mkdirPath,
+      FsPermission fsPermission, boolean recursive) throws IOException {
+    String origUmask = null;
+    LOG.debug("Create dirs " + mkdirPath + " with permission " + fsPermission + " recursive
"
+        + recursive);
+    if (recursive) {
+      origUmask = conf.get(FsPermission.UMASK_LABEL);
+      // this umask is required because by default the hdfs mask is 022 resulting in
+      // all parents getting the fsPermission & !(022) permission instead of fsPermission
+      conf.set(FsPermission.UMASK_LABEL, "000");
+    }
+    FileSystem fs = ShimLoader.getHadoopShims().getNonCachedFileSystem(mkdirPath.toUri(),
conf);
+    boolean retval = false;
+    try {
+      retval = fs.mkdirs(mkdirPath, fsPermission);
+      resetUmaskInConf(conf, recursive, origUmask);
+    } catch (IOException ioe) {
+      resetUmaskInConf(conf, recursive, origUmask);
+      throw ioe;
+    } finally {
+      IOUtils.closeStream(fs);
+    }
+    return retval;
+  }
+
+  private static void resetUmaskInConf(Configuration conf, boolean unsetUmask, String origUmask)
{
+    if (unsetUmask) {
+      if (origUmask != null) {
+        conf.set(FsPermission.UMASK_LABEL, origUmask);
+      } else {
+        conf.unset(FsPermission.UMASK_LABEL);
+      }
+    }
+  }
+
   /**
    * Returns true if a plan is both configured for vectorized execution
    * and vectorization is allowed. The plan may be configured for vectorization

Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java?rev=1624086&r1=1624085&r2=1624086&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java (original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java Wed Sep 10
18:15:32 2014
@@ -470,10 +470,7 @@ public class SessionState {
    */
   private void createSessionDirs(String userName) throws IOException {
     HiveConf conf = getConf();
-    // First create the root scratch dir on hdfs (if it doesn't already exist) and make it
writable
-    Path rootHDFSDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR));
-    String rootHDFSDirPermission = "777";
-    createPath(conf, rootHDFSDirPath, rootHDFSDirPermission, false, false);
+    Path rootHDFSDirPath = createRootHDFSDir(conf);
     // Now create session specific dirs
     String scratchDirPermission = HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIRPERMISSION);
     Path path;
@@ -506,6 +503,30 @@ public class SessionState {
   }
 
   /**
+   * Create the root scratch dir on hdfs (if it doesn't already exist) and make it writable
+   * @param conf
+   * @return
+   * @throws IOException
+   */
+  private Path createRootHDFSDir(HiveConf conf) throws IOException {
+    Path rootHDFSDirPath = new Path(HiveConf.getVar(conf, HiveConf.ConfVars.SCRATCHDIR));
+    FsPermission expectedHDFSDirPermission = new FsPermission("777");
+    FileSystem fs = rootHDFSDirPath.getFileSystem(conf);
+    if (!fs.exists(rootHDFSDirPath)) {
+      Utilities.createDirsWithPermission(conf, rootHDFSDirPath, expectedHDFSDirPermission,
true);
+    }
+    FsPermission currentHDFSDirPermission = fs.getFileStatus(rootHDFSDirPath).getPermission();
+    LOG.debug("HDFS root scratch dir: " + rootHDFSDirPath + ", permission: "
+        + currentHDFSDirPermission);
+    // If the root HDFS scratch dir already exists, make sure the permissions are 777.
+    if (!expectedHDFSDirPermission.equals(fs.getFileStatus(rootHDFSDirPath).getPermission()))
{
+      throw new RuntimeException("The root scratch dir: " + rootHDFSDirPath
+          + " on HDFS should be writable. Current permissions are: " + currentHDFSDirPermission);
+    }
+    return rootHDFSDirPath;
+  }
+
+  /**
    * Create a given path if it doesn't exist.
    *
    * @param conf



Mime
View raw message