You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ss...@apache.org on 2016/08/08 20:28:00 UTC

hive git commit: HIVE-14421. FS.deleteOnExit holds references to _tmp_space.db files. (Siddharth Seth, reviewed by Thejas Nair)

Repository: hive
Updated Branches:
  refs/heads/master 3f16a991b -> 043870139


HIVE-14421. FS.deleteOnExit holds references to _tmp_space.db files. (Siddharth Seth, reviewed by Thejas Nair)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/04387013
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/04387013
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/04387013

Branch: refs/heads/master
Commit: 0438701395161325a429b4fd8211213276aa0fef
Parents: 3f16a99
Author: Siddharth Seth <ss...@apache.org>
Authored: Mon Aug 8 13:27:26 2016 -0700
Committer: Siddharth Seth <ss...@apache.org>
Committed: Mon Aug 8 13:27:26 2016 -0700

----------------------------------------------------------------------
 .../apache/hadoop/hive/common/FileUtils.java    |  3 ++
 .../hive/ql/exec/tez/TezSessionState.java       |  3 +-
 .../hadoop/hive/ql/session/SessionState.java    | 33 ++++++++++++++++----
 3 files changed, 31 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/04387013/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
index 62c5504..3ed2d08 100644
--- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
+++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
@@ -769,6 +769,9 @@ public final class FileUtils {
   /**
    * create temporary file and register it to delete-on-exit hook.
    * File.deleteOnExit is not used for possible memory leakage.
+   *
+   * Make sure to use {@link #deleteTmpFile(File)} after the file is no longer required,
+   * and has been deleted to avoid a memory leak.
    */
   public static File createTempFile(String lScratchDir, String prefix, String suffix) throws IOException {
     File tmpDir = lScratchDir == null ? null : new File(lScratchDir);

http://git-wip-us.apache.org/repos/asf/hive/blob/04387013/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
index 38250f2..e8a1757 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java
@@ -570,9 +570,8 @@ public class TezSessionState {
     fs.mkdirs(tezDir, fsPermission);
     // Make sure the path is normalized (we expect validation to pass since we just created it).
     tezDir = DagUtils.validateTargetDir(tezDir, conf).getPath();
-    // don't keep the directory around on non-clean exit
-    fs.deleteOnExit(tezDir);
 
+    // Directory removal will be handled by cleanup at the SessionState level.
     return tezDir;
   }
 

http://git-wip-us.apache.org/repos/asf/hive/blob/04387013/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
----------------------------------------------------------------------
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
index 0581bab..408c92e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
@@ -659,7 +659,9 @@ public class SessionState {
     conf.set(LOCAL_SESSION_PATH_KEY, localSessionPath.toUri().toString());
     // 7. HDFS temp table space
     hdfsTmpTableSpace = new Path(hdfsSessionPath, TMP_PREFIX);
-    createPath(conf, hdfsTmpTableSpace, scratchDirPermission, false, true);
+    // This is a sub-dir under the hdfsSessionPath. Will be removed along with that dir.
+    // Don't register with deleteOnExit
+    createPath(conf, hdfsTmpTableSpace, scratchDirPermission, false, false);
     conf.set(TMP_TABLE_SPACE_KEY, hdfsTmpTableSpace.toUri().toString());
   }
 
@@ -782,19 +784,38 @@ public class SessionState {
   private void dropSessionPaths(Configuration conf) throws IOException {
     if (hdfsSessionPath != null) {
       if (hdfsSessionPathLockFile != null) {
-        hdfsSessionPathLockFile.close();
+        try {
+          hdfsSessionPathLockFile.close();
+        } catch (IOException e) {
+          LOG.error("Failed while closing remoteFsSessionLockFile", e);
+        }
       }
-      hdfsSessionPath.getFileSystem(conf).delete(hdfsSessionPath, true);
-      LOG.info("Deleted HDFS directory: " + hdfsSessionPath);
+      dropPathAndUnregisterDeleteOnExit(hdfsSessionPath, conf, false);
     }
     if (localSessionPath != null) {
-      FileSystem.getLocal(conf).delete(localSessionPath, true);
-      LOG.info("Deleted local directory: " + localSessionPath);
+      dropPathAndUnregisterDeleteOnExit(localSessionPath, conf, true);
     }
     deleteTmpOutputFile();
     deleteTmpErrOutputFile();
   }
 
+  private void dropPathAndUnregisterDeleteOnExit(Path path, Configuration conf, boolean localFs) {
+    FileSystem fs = null;
+    try {
+      if (localFs) {
+        fs = FileSystem.getLocal(conf);
+      } else {
+        fs = path.getFileSystem(conf);
+      }
+      fs.cancelDeleteOnExit(path);
+      fs.delete(path, true);
+      LOG.info("Deleted directory: {} on fs with scheme {}", path, fs.getScheme());
+    } catch (IOException e) {
+      LOG.error("Failed to delete path at {} on fs with scheme {}", path,
+          (fs == null ? "Unknown-null" : fs.getScheme()), e);
+    }
+  }
+
   /**
    * Setup authentication and authorization plugins for this session.
    */