From e3a9666d285c62afb0d50abea74d9e2ffe2767a8 Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Thu, 6 Oct 2016 16:31:29 -0500 Subject: [PATCH] HDFS-10955. Pass IIP for FSDirAttr methods. Contributed by Daryn Sharp. --- .../hdfs/server/namenode/FSDirAttrOp.java | 110 +++++++----------- .../hdfs/server/namenode/FSEditLogLoader.java | 62 ++++++---- .../hdfs/server/namenode/FSNamesystem.java | 3 +- 3 files changed, 83 insertions(+), 92 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index b7b88041103..6c3f6abc67e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -50,9 +50,8 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_STORAGE_POLICY_ENABLED_KE public class FSDirAttrOp { static HdfsFileStatus setPermission( - FSDirectory fsd, final String srcArg, FsPermission permission) + FSDirectory fsd, final String src, FsPermission permission) throws IOException { - String src = srcArg; if (FSDirectory.isExactReservedName(src)) { throw new InvalidPathException(src); } @@ -61,13 +60,12 @@ public class FSDirAttrOp { fsd.writeLock(); try { iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); fsd.checkOwner(pc, iip); - unprotectedSetPermission(fsd, src, permission); + unprotectedSetPermission(fsd, iip, permission); } finally { fsd.writeUnlock(); } - fsd.getEditLog().logSetPermissions(src, permission); + fsd.getEditLog().logSetPermissions(iip.getPath(), permission); return fsd.getAuditFileInfo(iip); } @@ -82,7 +80,6 @@ public class FSDirAttrOp { fsd.writeLock(); try { iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); fsd.checkOwner(pc, iip); if (!pc.isSuperUser()) { if (username != null && !pc.getUser().equals(username)) { @@ -92,11 +89,11 @@ public class FSDirAttrOp { throw new AccessControlException("User does not belong to " + group); } } - unprotectedSetOwner(fsd, src, username, group); + unprotectedSetOwner(fsd, iip, username, group); } finally { fsd.writeUnlock(); } - fsd.getEditLog().logSetOwner(src, username, group); + fsd.getEditLog().logSetOwner(iip.getPath(), username, group); return fsd.getAuditFileInfo(iip); } @@ -109,20 +106,18 @@ public class FSDirAttrOp { fsd.writeLock(); try { iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); // Write access is required to set access and modification times if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } final INode inode = iip.getLastINode(); if (inode == null) { - throw new FileNotFoundException("File/Directory " + src + + throw new FileNotFoundException("File/Directory " + iip.getPath() + " does not exist."); } - boolean changed = unprotectedSetTimes(fsd, inode, mtime, atime, true, - iip.getLatestSnapshotId()); + boolean changed = unprotectedSetTimes(fsd, iip, mtime, atime, true); if (changed) { - fsd.getEditLog().logTimes(src, mtime, atime); + fsd.getEditLog().logTimes(iip.getPath(), mtime, atime); } } finally { fsd.writeUnlock(); @@ -139,16 +134,15 @@ public class FSDirAttrOp { fsd.writeLock(); try { final INodesInPath iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } - final BlockInfo[] blocks = unprotectedSetReplication(fsd, src, + final BlockInfo[] blocks = unprotectedSetReplication(fsd, iip, replication); isFile = blocks != null; if (isFile) { - fsd.getEditLog().logSetReplication(src, replication); + fsd.getEditLog().logSetReplication(iip.getPath(), replication); } } finally { fsd.writeUnlock(); @@ -186,15 +180,14 @@ public class FSDirAttrOp { INodesInPath iip; fsd.writeLock(); try { - src = FSDirectory.resolvePath(src, fsd); - iip = fsd.getINodesInPath4Write(src); + iip = fsd.resolvePathForWrite(pc, src); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } unprotectedSetStoragePolicy(fsd, bm, iip, policyId); - fsd.getEditLog().logSetStoragePolicy(src, policyId); + fsd.getEditLog().logSetStoragePolicy(iip.getPath(), policyId); } finally { fsd.writeUnlock(); } @@ -232,11 +225,10 @@ public class FSDirAttrOp { fsd.readLock(); try { final INodesInPath iip = fsd.resolvePath(pc, src, false); - src = iip.getPath(); if (fsd.isPermissionEnabled()) { fsd.checkTraverse(pc, iip); } - return INodeFile.valueOf(iip.getLastINode(), src) + return INodeFile.valueOf(iip.getLastINode(), iip.getPath()) .getPreferredBlockSize(); } finally { fsd.readUnlock(); @@ -250,14 +242,16 @@ public class FSDirAttrOp { */ static void setQuota(FSDirectory fsd, String src, long nsQuota, long ssQuota, StorageType type) throws IOException { + FSPermissionChecker pc = fsd.getPermissionChecker(); if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); pc.checkSuperuserPrivilege(); } fsd.writeLock(); try { - INodeDirectory changed = unprotectedSetQuota(fsd, src, nsQuota, ssQuota, type); + INodesInPath iip = fsd.resolvePathForWrite(pc, src); + INodeDirectory changed = + unprotectedSetQuota(fsd, iip, nsQuota, ssQuota, type); if (changed != null) { final QuotaCounts q = changed.getQuotaCounts(); if (type == null) { @@ -273,58 +267,40 @@ public class FSDirAttrOp { } static void unprotectedSetPermission( - FSDirectory fsd, String src, FsPermission permissions) + FSDirectory fsd, INodesInPath iip, FsPermission permissions) throws FileNotFoundException, UnresolvedLinkException, QuotaExceededException, SnapshotAccessControlException { assert fsd.hasWriteLock(); - final INodesInPath inodesInPath = fsd.getINodesInPath4Write(src, true); - final INode inode = inodesInPath.getLastINode(); - if (inode == null) { - throw new FileNotFoundException("File does not exist: " + src); - } - int snapshotId = inodesInPath.getLatestSnapshotId(); + final INode inode = FSDirectory.resolveLastINode(iip); + int snapshotId = iip.getLatestSnapshotId(); inode.setPermission(permissions, snapshotId); } static void unprotectedSetOwner( - FSDirectory fsd, String src, String username, String groupname) + FSDirectory fsd, INodesInPath iip, String username, String groupname) throws FileNotFoundException, UnresolvedLinkException, QuotaExceededException, SnapshotAccessControlException { assert fsd.hasWriteLock(); - final INodesInPath inodesInPath = fsd.getINodesInPath4Write(src, true); - INode inode = inodesInPath.getLastINode(); - if (inode == null) { - throw new FileNotFoundException("File does not exist: " + src); - } + final INode inode = FSDirectory.resolveLastINode(iip); if (username != null) { - inode = inode.setUser(username, inodesInPath.getLatestSnapshotId()); + inode.setUser(username, iip.getLatestSnapshotId()); } if (groupname != null) { - inode.setGroup(groupname, inodesInPath.getLatestSnapshotId()); + inode.setGroup(groupname, iip.getLatestSnapshotId()); } } static boolean setTimes( - FSDirectory fsd, INode inode, long mtime, long atime, boolean force, - int latestSnapshotId) throws QuotaExceededException { + FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) + throws QuotaExceededException { fsd.writeLock(); try { - return unprotectedSetTimes(fsd, inode, mtime, atime, force, - latestSnapshotId); + return unprotectedSetTimes(fsd, iip, mtime, atime, force); } finally { fsd.writeUnlock(); } } - static boolean unprotectedSetTimes( - FSDirectory fsd, String src, long mtime, long atime, boolean force) - throws UnresolvedLinkException, QuotaExceededException { - assert fsd.hasWriteLock(); - final INodesInPath i = fsd.getINodesInPath(src, true); - return unprotectedSetTimes(fsd, i.getLastINode(), mtime, atime, - force, i.getLatestSnapshotId()); - } - /** * See {@link org.apache.hadoop.hdfs.protocol.ClientProtocol#setQuota(String, * long, long, StorageType)} @@ -339,7 +315,8 @@ public class FSDirAttrOp { * @throws SnapshotAccessControlException if path is in RO snapshot */ static INodeDirectory unprotectedSetQuota( - FSDirectory fsd, String src, long nsQuota, long ssQuota, StorageType type) + FSDirectory fsd, INodesInPath iip, long nsQuota, + long ssQuota, StorageType type) throws FileNotFoundException, PathIsNotDirectoryException, QuotaExceededException, UnresolvedLinkException, SnapshotAccessControlException, UnsupportedActionException { @@ -363,9 +340,8 @@ public class FSDirAttrOp { nsQuota); } - String srcs = FSDirectory.normalizePath(src); - final INodesInPath iip = fsd.getINodesInPath4Write(srcs, true); - INodeDirectory dirNode = INodeDirectory.valueOf(iip.getLastINode(), srcs); + INodeDirectory dirNode = + INodeDirectory.valueOf(iip.getLastINode(), iip.getPath()); if (dirNode.isRoot() && nsQuota == HdfsConstants.QUOTA_RESET) { throw new IllegalArgumentException("Cannot clear namespace quota on root."); } else { // a directory inode @@ -401,13 +377,12 @@ public class FSDirAttrOp { } static BlockInfo[] unprotectedSetReplication( - FSDirectory fsd, String src, short replication) + FSDirectory fsd, INodesInPath iip, short replication) throws QuotaExceededException, UnresolvedLinkException, SnapshotAccessControlException { assert fsd.hasWriteLock(); final BlockManager bm = fsd.getBlockManager(); - final INodesInPath iip = fsd.getINodesInPath4Write(src, true); final INode inode = iip.getLastINode(); if (inode == null || !inode.isFile()) { return null; @@ -437,10 +412,10 @@ public class FSDirAttrOp { if (oldBR != -1) { if (oldBR > targetReplication) { FSDirectory.LOG.info("Decreasing replication from {} to {} for {}", - oldBR, targetReplication, src); + oldBR, targetReplication, iip.getPath()); } else { FSDirectory.LOG.info("Increasing replication from {} to {} for {}", - oldBR, targetReplication, src); + oldBR, targetReplication, iip.getPath()); } } return file.getBlocks(); @@ -475,8 +450,7 @@ public class FSDirAttrOp { } inode.asFile().setStoragePolicyID(policyId, snapshotId); } else if (inode.isDirectory()) { - setDirStoragePolicy(fsd, inode.asDirectory(), policyId, - snapshotId); + setDirStoragePolicy(fsd, iip, policyId); } else { throw new FileNotFoundException(iip.getPath() + " is not a file or directory"); @@ -484,8 +458,8 @@ public class FSDirAttrOp { } private static void setDirStoragePolicy( - FSDirectory fsd, INodeDirectory inode, byte policyId, - int latestSnapshotId) throws IOException { + FSDirectory fsd, INodesInPath iip, byte policyId) throws IOException { + INode inode = FSDirectory.resolveLastINode(iip); List existingXAttrs = XAttrStorage.readINodeXAttrs(inode); XAttr xAttr = BlockStoragePolicySuite.buildXAttr(policyId); List newXAttrs = null; @@ -500,14 +474,16 @@ public class FSDirAttrOp { Arrays.asList(xAttr), EnumSet.of(XAttrSetFlag.CREATE, XAttrSetFlag.REPLACE)); } - XAttrStorage.updateINodeXAttrs(inode, newXAttrs, latestSnapshotId); + XAttrStorage.updateINodeXAttrs(inode, newXAttrs, iip.getLatestSnapshotId()); } - private static boolean unprotectedSetTimes( - FSDirectory fsd, INode inode, long mtime, long atime, boolean force, - int latest) throws QuotaExceededException { + static boolean unprotectedSetTimes( + FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) + throws QuotaExceededException { assert fsd.hasWriteLock(); boolean status = false; + INode inode = iip.getLastINode(); + int latest = iip.getLatestSnapshotId(); if (mtime != -1) { inode = inode.setModificationTime(mtime, latest); status = true; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java index 4192e3d80c9..84c29bfbb39 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java @@ -508,10 +508,12 @@ public class FSEditLogLoader { } case OP_SET_REPLICATION: { SetReplicationOp setReplicationOp = (SetReplicationOp)op; + String src = renameReservedPathsOnUpgrade( + setReplicationOp.path, logVersion); + INodesInPath iip = fsDir.getINodesInPath4Write(src); short replication = fsNamesys.getBlockManager().adjustReplication( setReplicationOp.replication); - FSDirAttrOp.unprotectedSetReplication(fsDir, renameReservedPathsOnUpgrade( - setReplicationOp.path, logVersion), replication); + FSDirAttrOp.unprotectedSetReplication(fsDir, iip, replication); break; } case OP_CONCAT_DELETE: { @@ -576,52 +578,66 @@ public class FSEditLogLoader { } case OP_SET_PERMISSIONS: { SetPermissionsOp setPermissionsOp = (SetPermissionsOp)op; - FSDirAttrOp.unprotectedSetPermission(fsDir, renameReservedPathsOnUpgrade( - setPermissionsOp.src, logVersion), setPermissionsOp.permissions); + final String src = + renameReservedPathsOnUpgrade(setPermissionsOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetPermission(fsDir, iip, + setPermissionsOp.permissions); break; } case OP_SET_OWNER: { SetOwnerOp setOwnerOp = (SetOwnerOp)op; - FSDirAttrOp.unprotectedSetOwner( - fsDir, renameReservedPathsOnUpgrade(setOwnerOp.src, logVersion), + final String src = renameReservedPathsOnUpgrade( + setOwnerOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetOwner(fsDir, iip, setOwnerOp.username, setOwnerOp.groupname); break; } case OP_SET_NS_QUOTA: { SetNSQuotaOp setNSQuotaOp = (SetNSQuotaOp)op; - FSDirAttrOp.unprotectedSetQuota( - fsDir, renameReservedPathsOnUpgrade(setNSQuotaOp.src, logVersion), + final String src = renameReservedPathsOnUpgrade( + setNSQuotaOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetQuota(fsDir, iip, setNSQuotaOp.nsQuota, HdfsConstants.QUOTA_DONT_SET, null); break; } case OP_CLEAR_NS_QUOTA: { ClearNSQuotaOp clearNSQuotaOp = (ClearNSQuotaOp)op; - FSDirAttrOp.unprotectedSetQuota( - fsDir, renameReservedPathsOnUpgrade(clearNSQuotaOp.src, logVersion), + final String src = renameReservedPathsOnUpgrade( + clearNSQuotaOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetQuota(fsDir, iip, HdfsConstants.QUOTA_RESET, HdfsConstants.QUOTA_DONT_SET, null); break; } - - case OP_SET_QUOTA: + case OP_SET_QUOTA: { SetQuotaOp setQuotaOp = (SetQuotaOp) op; - FSDirAttrOp.unprotectedSetQuota(fsDir, - renameReservedPathsOnUpgrade(setQuotaOp.src, logVersion), + final String src = renameReservedPathsOnUpgrade( + setQuotaOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetQuota(fsDir, iip, setQuotaOp.nsQuota, setQuotaOp.dsQuota, null); break; - - case OP_SET_QUOTA_BY_STORAGETYPE: - FSEditLogOp.SetQuotaByStorageTypeOp setQuotaByStorageTypeOp = + } + case OP_SET_QUOTA_BY_STORAGETYPE: { + FSEditLogOp.SetQuotaByStorageTypeOp setQuotaByStorageTypeOp = (FSEditLogOp.SetQuotaByStorageTypeOp) op; - FSDirAttrOp.unprotectedSetQuota(fsDir, - renameReservedPathsOnUpgrade(setQuotaByStorageTypeOp.src, logVersion), + final String src = renameReservedPathsOnUpgrade( + setQuotaByStorageTypeOp.src, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetQuota(fsDir, iip, HdfsConstants.QUOTA_DONT_SET, setQuotaByStorageTypeOp.dsQuota, setQuotaByStorageTypeOp.type); - break; - + break; + } case OP_TIMES: { TimesOp timesOp = (TimesOp)op; - FSDirAttrOp.unprotectedSetTimes( - fsDir, renameReservedPathsOnUpgrade(timesOp.path, logVersion), + final String src = renameReservedPathsOnUpgrade( + timesOp.path, logVersion); + final INodesInPath iip = fsDir.getINodesInPath4Write(src); + FSDirAttrOp.unprotectedSetTimes(fsDir, iip, timesOp.mtime, timesOp.atime, true); break; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 54c862932b6..a0340f4ff0c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -1784,8 +1784,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, boolean updateAccessTime = inode != null && now > inode.getAccessTime() + dir.getAccessTimePrecision(); if (!isInSafeMode() && updateAccessTime) { - boolean changed = FSDirAttrOp.setTimes(dir, - inode, -1, now, false, iip.getLatestSnapshotId()); + boolean changed = FSDirAttrOp.setTimes(dir, iip, -1, now, false); if (changed) { getEditLog().logTimes(src, -1, now); }