From c459f054865533c2722ce6db71b4dd1813484012 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Wed, 11 Apr 2018 14:46:56 +0530 Subject: [PATCH] HBASE-15291 FileSystem not closed in secure bulkLoad --- .../security/access/SecureBulkLoadEndpoint.java | 57 +++++++++++++++------- 1 file changed, 40 insertions(+), 17 deletions(-) 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 bd88b6c9e3..7496e4eda4 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(); } } -- 2.15.1.windows.2