HDFS-10955. Pass IIP for FSDirAttr methods. Contributed by Daryn Sharp.

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, 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 @@ 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();
@@ -476,8 +451,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");
@@ -485,8 +459,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<XAttr> existingXAttrs = XAttrStorage.readINodeXAttrs(inode);
     XAttr xAttr = BlockStoragePolicySuite.buildXAttr(policyId);
     List<XAttr> newXAttrs = null;
@@ -501,14 +475,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;

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java

@@ -521,10 +521,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: {
@@ -589,52 +591,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;
     }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -1798,8 +1798,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);
           }