HIVE-13661: [Refactor] Move common FS operations out of shim layer (A…

…shutosh Chauhan via Sergey Shelukhin) Signed-off-by: Ashutosh Chauhan <hashutosh@apache.org>
apache · May 3, 2016 · e1b0383 · e1b0383
1 parent e9a7218
commit e1b0383
Show file tree

Hide file tree

Showing 10 changed files with 164 additions and 277 deletions.
diff --git a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
@@ -36,10 +36,11 @@
 import org.apache.hadoop.fs.LocalFileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathFilter;
+import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.io.HdfsUtils;
 import org.apache.hadoop.hive.shims.HadoopShims;
-import org.apache.hadoop.hive.shims.HadoopShims.HdfsFileStatus;
 import org.apache.hadoop.hive.shims.ShimLoader;
 import org.apache.hadoop.hive.shims.Utils;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -526,11 +527,9 @@ public static boolean mkdir(FileSystem fs, Path f, boolean inheritPerms, Configu
       if (!success) {
         return false;
       } else {
-        HadoopShims shim = ShimLoader.getHadoopShims();
-        HdfsFileStatus fullFileStatus = shim.getFullFileStatus(conf, fs, lastExistingParent);
         try {
           //set on the entire subtree
-          shim.setFullFileStatus(conf, fullFileStatus, fs, firstNonExistentParent);
+          HdfsUtils.setFullFileStatus(conf, new HdfsUtils.HadoopFileStatus(conf, fs, lastExistingParent), fs, firstNonExistentParent);
         } catch (Exception e) {
           LOG.warn("Error setting permissions of " + firstNonExistentParent, e);
         }
@@ -566,9 +565,8 @@ public static boolean copy(FileSystem srcFS, Path src,
 
     boolean inheritPerms = conf.getBoolVar(HiveConf.ConfVars.HIVE_WAREHOUSE_SUBDIR_INHERIT_PERMS);
     if (copied && inheritPerms) {
-      HdfsFileStatus fullFileStatus = shims.getFullFileStatus(conf, dstFS, dst);
       try {
-        shims.setFullFileStatus(conf, fullFileStatus, dstFS, dst);
+        HdfsUtils.setFullFileStatus(conf, new HdfsUtils.HadoopFileStatus(conf, dstFS, dst.getParent()), dstFS, dst);
       } catch (Exception e) {
         LOG.warn("Error setting permissions or group of " + dst, e);
       }
@@ -620,12 +618,11 @@ public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf) thr
    */
   public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf, boolean forceDelete)
       throws IOException {
-    LOG.info("deleting  " + f);
-    HadoopShims hadoopShim = ShimLoader.getHadoopShims();
+    LOG.debug("deleting  " + f);
 
     boolean result = false;
     try {
-      result = hadoopShim.moveToAppropriateTrash(fs, f, conf);
+      result = Trash.moveToAppropriateTrash(fs, f, conf);
       if (result) {
         LOG.info("Moved to trash: " + f);
         return true;
@@ -687,10 +684,8 @@ public static boolean renameWithPerms(FileSystem fs, Path sourcePath,
     } else {
       //rename the directory
       if (fs.rename(sourcePath, destPath)) {
-        HadoopShims shims = ShimLoader.getHadoopShims();
-        HdfsFileStatus fullFileStatus = shims.getFullFileStatus(conf, fs, destPath.getParent());
         try {
-          shims.setFullFileStatus(conf, fullFileStatus, fs, destPath);
+          HdfsUtils.setFullFileStatus(conf, new HdfsUtils.HadoopFileStatus(conf, fs, destPath.getParent()), fs, destPath);
         } catch (Exception e) {
           LOG.warn("Error setting permissions or group of " + destPath, e);
         }

diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreFsImpl.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreFsImpl.java
@@ -25,10 +25,8 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.hive.metastore.api.MetaException;
-import org.apache.hadoop.hive.shims.HadoopShims;
-import org.apache.hadoop.hive.shims.ShimLoader;
 
 public class HiveMetaStoreFsImpl implements MetaStoreFS {
 
@@ -38,19 +36,18 @@ public class HiveMetaStoreFsImpl implements MetaStoreFS {
   @Override
   public boolean deleteDir(FileSystem fs, Path f, boolean recursive,
       boolean ifPurge, Configuration conf) throws MetaException {
-    LOG.info("deleting  " + f);
-    HadoopShims hadoopShim = ShimLoader.getHadoopShims();
+    LOG.debug("deleting  " + f);
 
     try {
       if (ifPurge) {
         LOG.info("Not moving "+ f +" to trash");
-      } else if (hadoopShim.moveToAppropriateTrash(fs, f, conf)) {
+      } else if (Trash.moveToAppropriateTrash(fs, f, conf)) {
         LOG.info("Moved to trash: " + f);
         return true;
       }
 
       if (fs.delete(f, true)) {
-        LOG.info("Deleted the diretory " + f);
+        LOG.debug("Deleted the diretory " + f);
         return true;
       }
 

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
@@ -61,6 +61,7 @@
 import org.apache.hadoop.hive.common.type.HiveDecimal;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hadoop.hive.io.HdfsUtils;
 import org.apache.hadoop.hive.metastore.MetaStoreUtils;
 import org.apache.hadoop.hive.metastore.PartitionDropOptions;
 import org.apache.hadoop.hive.metastore.TableType;
@@ -216,9 +217,6 @@
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoUtils;
-import org.apache.hadoop.hive.shims.HadoopShims;
-import org.apache.hadoop.hive.shims.HadoopShims.HdfsFileStatus;
-import org.apache.hadoop.hive.shims.ShimLoader;
 import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.mapreduce.MRJobConfig;
 import org.apache.hadoop.tools.HadoopArchives;
@@ -2394,7 +2392,7 @@ public int showColumns(Hive db, ShowColumnsDesc showCols)
 
   /**
    * Write a list of the user defined functions to a file.
-   * @param db 
+   * @param db
    *
    * @param showFuncs
    *          are the functions we're interested in.
@@ -2447,7 +2445,7 @@ private int showFunctions(Hive db, ShowFunctionsDesc showFuncs) throws HiveExcep
 
   /**
    * Write a list of the current locks to a file.
-   * @param db 
+   * @param db
    *
    * @param showLocks
    *          the locks we're interested in.
@@ -2725,7 +2723,7 @@ private int showTxns(Hive db, ShowTxnsDesc desc) throws HiveException {
 
    /**
    * Lock the table/partition specified
-   * @param db 
+   * @param db
    *
    * @param lockTbl
    *          the table/partition to be locked along with the mode
@@ -2771,7 +2769,7 @@ private int unlockDatabase(Hive db, UnlockDatabaseDesc unlockDb) throws HiveExce
 
   /**
    * Unlock the table/partition specified
-   * @param db 
+   * @param db
    *
    * @param unlockTbl
    *          the table/partition to be unlocked
@@ -2787,7 +2785,7 @@ private int unlockTable(Hive db, UnlockTableDesc unlockTbl) throws HiveException
 
   /**
    * Shows a description of a function.
-   * @param db 
+   * @param db
    *
    * @param descFunc
    *          is the function we are describing
@@ -4190,15 +4188,13 @@ private int truncateTable(Hive db, TruncateTableDesc truncateTableDesc) throws H
 
     try {
       // this is not transactional
-      HadoopShims shim = ShimLoader.getHadoopShims();
       for (Path location : getLocations(db, table, partSpec)) {
         FileSystem fs = location.getFileSystem(conf);
-
-        HdfsFileStatus fullFileStatus = shim.getFullFileStatus(conf, fs, location);
+        HdfsUtils.HadoopFileStatus status = new HdfsUtils.HadoopFileStatus(conf, fs, location);
         fs.delete(location, true);
         fs.mkdirs(location);
         try {
-          shim.setFullFileStatus(conf, fullFileStatus, fs, location);
+          HdfsUtils.setFullFileStatus(conf, status, fs, location);
         } catch (Exception e) {
           LOG.warn("Error setting permissions of " + location, e);
         }

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java
@@ -33,6 +33,7 @@
 import org.apache.hadoop.hive.common.FileUtils;
 import org.apache.hadoop.hive.common.HiveStatsUtils;
 import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.io.HdfsUtils;
 import org.apache.hadoop.hive.metastore.MetaStoreUtils;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;
 import org.apache.hadoop.hive.metastore.api.InvalidOperationException;
@@ -65,8 +66,6 @@
 import org.apache.hadoop.hive.ql.plan.MoveWork;
 import org.apache.hadoop.hive.ql.plan.api.StageType;
 import org.apache.hadoop.hive.ql.session.SessionState;
-import org.apache.hadoop.hive.shims.HadoopShims;
-import org.apache.hadoop.hive.shims.ShimLoader;
 import org.apache.hadoop.util.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -179,11 +178,9 @@ private Path createTargetPath(Path targetPath, FileSystem fs) throws IOException
         actualPath = actualPath.getParent();
       }
       fs.mkdirs(mkDirPath);
-      HadoopShims shims = ShimLoader.getHadoopShims();
       if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_WAREHOUSE_SUBDIR_INHERIT_PERMS)) {
         try {
-          HadoopShims.HdfsFileStatus status = shims.getFullFileStatus(conf, fs, actualPath);
-          shims.setFullFileStatus(conf, status, fs, actualPath);
+          HdfsUtils.setFullFileStatus(conf, new HdfsUtils.HadoopFileStatus(conf, fs, actualPath), fs, mkDirPath);
         } catch (Exception e) {
           LOG.warn("Error setting permissions or group of " + actualPath, e);
         }

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
@@ -65,6 +65,7 @@
 import org.apache.hadoop.hive.common.classification.InterfaceStability.Unstable;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hadoop.hive.io.HdfsUtils;
 import org.apache.hadoop.hive.metastore.HiveMetaException;
 import org.apache.hadoop.hive.metastore.HiveMetaHook;
 import org.apache.hadoop.hive.metastore.HiveMetaHookLoader;
@@ -2611,9 +2612,9 @@ private static void copyFiles(final HiveConf conf, final FileSystem destFs,
       FileStatus[] srcs, final FileSystem srcFs, final Path destf, final boolean isSrcLocal, final List<Path> newFiles)
           throws HiveException {
 
-    final HadoopShims.HdfsFileStatus fullDestStatus;
+    final HdfsUtils.HadoopFileStatus fullDestStatus;
     try {
-      fullDestStatus = ShimLoader.getHadoopShims().getFullFileStatus(conf, destFs, destf);
+      fullDestStatus = new HdfsUtils.HadoopFileStatus(conf, destFs, destf);
     } catch (IOException e1) {
       throw new HiveException(e1);
     }
@@ -2674,7 +2675,7 @@ public ObjectPair<Path, Path> call() throws Exception {
             }
 
             if (inheritPerms) {
-              ShimLoader.getHadoopShims().setFullFileStatus(conf, fullDestStatus, destFs, destPath);
+              HdfsUtils.setFullFileStatus(conf, fullDestStatus, destFs, destPath);
             }
             if (null != newFiles) {
               newFiles.add(destPath);
@@ -2697,17 +2698,6 @@ public ObjectPair<Path, Path> call() throws Exception {
     }
   }
 
-  private static boolean destExists(List<List<Path[]>> result, Path proposed) {
-    for (List<Path[]> sdpairs : result) {
-      for (Path[] sdpair : sdpairs) {
-        if (sdpair[1].equals(proposed)) {
-          return true;
-        }
-      }
-    }
-    return false;
-  }
-
   private static boolean isSubDir(Path srcf, Path destf, FileSystem srcFs, FileSystem destFs, boolean isSrcLocal) {
     if (srcf == null) {
       LOG.debug("The source path is null for isSubDir method.");
@@ -2795,8 +2785,7 @@ public static boolean moveFile(HiveConf conf, Path srcf, final Path destf,
     //needed for perm inheritance.
     boolean inheritPerms = HiveConf.getBoolVar(conf,
         HiveConf.ConfVars.HIVE_WAREHOUSE_SUBDIR_INHERIT_PERMS);
-    HadoopShims shims = ShimLoader.getHadoopShims();
-    HadoopShims.HdfsFileStatus destStatus = null;
+    HdfsUtils.HadoopFileStatus destStatus = null;
 
     // If source path is a subdirectory of the destination path:
     //   ex: INSERT OVERWRITE DIRECTORY 'target/warehouse/dest4.out' SELECT src.value WHERE src.key >= 300;
@@ -2808,7 +2797,7 @@ public static boolean moveFile(HiveConf conf, Path srcf, final Path destf,
     try {
       if (inheritPerms || replace) {
         try{
-          destStatus = shims.getFullFileStatus(conf, destFs, destf);
+          destStatus = new HdfsUtils.HadoopFileStatus(conf, destFs, destf);
           //if destf is an existing directory:
           //if replace is true, delete followed by rename(mv) is equivalent to replace
           //if replace is false, rename (mv) actually move the src under dest dir
@@ -2821,7 +2810,7 @@ public static boolean moveFile(HiveConf conf, Path srcf, final Path destf,
         } catch (FileNotFoundException ignore) {
           //if dest dir does not exist, any re
           if (inheritPerms) {
-            destStatus = shims.getFullFileStatus(conf, destFs, destf.getParent());
+            destStatus = new HdfsUtils.HadoopFileStatus(conf, destFs, destf.getParent());
           }
         }
       }
@@ -2888,7 +2877,7 @@ public Boolean call() throws Exception {
 
     if (success && inheritPerms) {
       try {
-        ShimLoader.getHadoopShims().setFullFileStatus(conf, destStatus, destFs, destf);
+        HdfsUtils.setFullFileStatus(conf, destStatus, destFs, destf);
       } catch (IOException e) {
         LOG.warn("Error setting permission of file " + destf + ": "+ e.getMessage(), e);
       }

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
@@ -445,7 +445,7 @@ public HadoopShims.HdfsEncryptionShim getHdfsEncryptionShim() throws HiveExcepti
         if ("hdfs".equals(fs.getUri().getScheme())) {
           hdfsEncryptionShim = ShimLoader.getHadoopShims().createHdfsEncryptionShim(fs, sessionConf);
         } else {
-          LOG.info("Could not get hdfsEncryptionShim, it is only applicable to hdfs filesystem.");
+          LOG.debug("Could not get hdfsEncryptionShim, it is only applicable to hdfs filesystem.");
         }
       } catch (Exception e) {
         throw new HiveException(e);