Repository: hbase
Updated Branches:
  refs/heads/branch-1.2 1ea3a8bc8 -> 2954aeae2


HBASE-15291 FileSystem not closed in secure bulkLoad

Signed-off-by: Ashish Singhi <ashishsin...@apache.org>


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

Branch: refs/heads/branch-1.2
Commit: 2954aeae2d10a35fdde0d619640a563dcc33f79c
Parents: 1ea3a8b
Author: Ashish Singhi <ashishsin...@apache.org>
Authored: Wed Apr 11 14:50:07 2018 +0530
Committer: Ashish Singhi <ashishsin...@apache.org>
Committed: Wed Apr 11 14:51:01 2018 +0530

----------------------------------------------------------------------
 .../security/access/SecureBulkLoadEndpoint.java | 57 ++++++++++++++------
 1 file changed, 40 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/2954aeae/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
index bd88b6c..7496e4e 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
@@ -204,6 +204,15 @@ public class SecureBulkLoadEndpoint extends 
SecureBulkLoadService
       done.run(CleanupBulkLoadResponse.newBuilder().build());
     } catch (IOException e) {
       ResponseConverter.setControllerException(controller, e);
+    } finally {
+      UserGroupInformation ugi = getActiveUser().getUGI();
+      try {
+        if (!UserGroupInformation.getLoginUser().equals(ugi)) {
+          FileSystem.closeAllForUGI(ugi);
+        }
+      } catch (IOException e) {
+        LOG.error("Failed to close FileSystem for: " + ugi, e);
+      }
     }
     done.run(null);
   }
@@ -374,7 +383,7 @@ public class SecureBulkLoadEndpoint extends 
SecureBulkLoadService
       Path p = new Path(srcPath);
       Path stageP = new Path(stagingDir, new Path(Bytes.toString(family), 
p.getName()));
       if (srcFs == null) {
-        srcFs = FileSystem.get(p.toUri(), conf);
+        srcFs = FileSystem.newInstance(p.toUri(), conf);
       }
 
       if(!isFile(p)) {
@@ -401,26 +410,40 @@ public class SecureBulkLoadEndpoint extends 
SecureBulkLoadService
     @Override
     public void doneBulkLoad(byte[] family, String srcPath) throws IOException 
{
       LOG.debug("Bulk Load done for: " + srcPath);
+      closeSrcFs();
+    }
+
+    private void closeSrcFs() throws IOException {
+      if (srcFs != null) {
+        srcFs.close();
+        srcFs = null;
+      }
     }
 
     @Override
     public void failedBulkLoad(final byte[] family, final String srcPath) 
throws IOException {
-      if (!FSHDFSUtils.isSameHdfs(conf, srcFs, fs)) {
-        // files are copied so no need to move them back
-        return;
-      }
-      Path p = new Path(srcPath);
-      Path stageP = new Path(stagingDir,
-          new Path(Bytes.toString(family), p.getName()));
-      LOG.debug("Moving " + stageP + " back to " + p);
-      if(!fs.rename(stageP, p))
-        throw new IOException("Failed to move HFile: " + stageP + " to " + p);
-
-      // restore original permission
-      if (origPermissions.containsKey(srcPath)) {
-        fs.setPermission(p, origPermissions.get(srcPath));
-      } else {
-        LOG.warn("Can't find previous permission for path=" + srcPath);
+      try {
+        Path p = new Path(srcPath);
+        if (srcFs == null) {
+          srcFs = FileSystem.newInstance(p.toUri(), conf);
+        }
+        if (!FSHDFSUtils.isSameHdfs(conf, srcFs, fs)) {
+          // files are copied so no need to move them back
+          return;
+        }
+        Path stageP = new Path(stagingDir, new Path(Bytes.toString(family), 
p.getName()));
+        LOG.debug("Moving " + stageP + " back to " + p);
+        if (!fs.rename(stageP, p))
+          throw new IOException("Failed to move HFile: " + stageP + " to " + 
p);
+
+        // restore original permission
+        if (origPermissions.containsKey(srcPath)) {
+          fs.setPermission(p, origPermissions.get(srcPath));
+        } else {
+          LOG.warn("Can't find previous permission for path=" + srcPath);
+        }
+      } finally {
+        closeSrcFs();
       }
     }
 

Reply via email to