From 48b9d5fd2a96728b1118be217ca597c4098e99ca Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Thu, 6 Oct 2016 16:33:46 -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 4c5ecb1d22..91d9bce548 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 @@ 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 @@ static HdfsFileStatus setPermission( 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 @@ static HdfsFileStatus setOwner( 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 @@ static HdfsFileStatus setOwner( 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 @@ static HdfsFileStatus setTimes( 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 @@ static boolean setReplication( 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 @@ static HdfsFileStatus setStoragePolicy(FSDirectory fsd, BlockManager bm, 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 @@ static long getPreferredBlockSize(FSDirectory fsd, String src) 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 @@ static long getPreferredBlockSize(FSDirectory fsd, String src) */ 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 @@ static void setQuota(FSDirectory fsd, String src, long nsQuota, long ssQuota, } 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 @@ static boolean unprotectedSetTimes( * @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 @@ static INodeDirectory unprotectedSetQuota( 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 @@ static INodeDirectory unprotectedSetQuota( } static BlockInfo[] unprotectedSetReplication( - FSDirectory fsd, String src, short replication) + FSDirectory fsd, INodesInPath iip, short replication) throws QuotaExceededException, UnresolvedLinkException, SnapshotAccessControlException, UnsupportedActionException { 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() || inode.asFile().isStriped()) { // TODO we do not support replication on stripe layout files yet @@ -438,10 +413,10 @@ static BlockInfo[] unprotectedSetReplication( 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(); @@ -476,8 +451,7 @@ static void unprotectedSetStoragePolicy(FSDirectory fsd, BlockManager bm, } 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"); @@ -485,8 +459,8 @@ static void unprotectedSetStoragePolicy(FSDirectory fsd, BlockManager bm, } 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; @@ -501,14 +475,16 @@ private static void setDirStoragePolicy( 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 9c72a86c1f..09201cfd09 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 @@ -521,10 +521,12 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, } 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: { @@ -589,52 +591,66 @@ fsDir, renameReservedPathsOnUpgrade(deleteOp.path, logVersion), } 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 fb3b375b34..34fb8b6e14 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 @@ -1798,8 +1798,7 @@ LocatedBlocks getBlockLocations(String clientMachine, String srcArg, 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); }