diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 30a2f69f6f..ab48d59e0f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -155,6 +155,9 @@ Trunk (Unreleased) HDFS-4122. Cleanup HDFS logs and reduce the size of logged messages. (suresh) + HDFS-4124. Refactor INodeDirectory#getExistingPathINodes() to enable + returningmore than INode array. (Jing Zhao via suresh) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index e298ac2655..e20b99c2a3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -59,6 +59,7 @@ import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState; import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileSnapshot; import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileWithLink; +import org.apache.hadoop.hdfs.server.namenode.INodeDirectory.INodesInPath; import org.apache.hadoop.hdfs.util.ByteArray; import com.google.common.base.Preconditions; @@ -594,8 +595,9 @@ boolean unprotectedRenameTo(String src, String dst, long timestamp) } byte[][] dstComponents = INode.getPathComponents(dst); - INode[] dstInodes = new INode[dstComponents.length]; - rootDir.getExistingPathINodes(dstComponents, dstInodes, false); + INodesInPath dstInodesInPath = rootDir.getExistingPathINodes(dstComponents, + dstComponents.length, false); + INode[] dstInodes = dstInodesInPath.getINodes(); if (dstInodes[dstInodes.length-1] != null) { NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: " +"failed to rename "+src+" to "+dst+ @@ -610,7 +612,7 @@ boolean unprotectedRenameTo(String src, String dst, long timestamp) } // Ensure dst has quota to accommodate rename - verifyQuotaForRename(srcInodes,dstInodes); + verifyQuotaForRename(srcInodes, dstInodes); INode dstChild = null; INode srcChild = null; @@ -714,8 +716,9 @@ boolean unprotectedRenameTo(String src, String dst, long timestamp, throw new IOException(error); } final byte[][] dstComponents = INode.getPathComponents(dst); - final INode[] dstInodes = new INode[dstComponents.length]; - rootDir.getExistingPathINodes(dstComponents, dstInodes, false); + INodesInPath dstInodesInPath = rootDir.getExistingPathINodes(dstComponents, + dstComponents.length, false); + final INode[] dstInodes = dstInodesInPath.getINodes(); INode dstInode = dstInodes[dstInodes.length - 1]; if (dstInodes.length == 1) { error = "rename destination cannot be the root"; @@ -1513,12 +1516,13 @@ boolean mkdirs(String src, PermissionStatus permissions, src = normalizePath(src); String[] names = INode.getPathNames(src); byte[][] components = INode.getPathComponents(names); - INode[] inodes = new INode[components.length]; - final int lastInodeIndex = inodes.length - 1; + final int lastInodeIndex = components.length - 1; writeLock(); try { - rootDir.getExistingPathINodes(components, inodes, false); + INodesInPath inodesInPath = rootDir.getExistingPathINodes(components, + components.length, false); + INode[] inodes = inodesInPath.getINodes(); // find the index of the first null in inodes[] StringBuilder pathbuilder = new StringBuilder(); @@ -1588,16 +1592,14 @@ boolean mkdirs(String src, PermissionStatus permissions, return true; } - /** - */ INode unprotectedMkdir(String src, PermissionStatus permissions, long timestamp) throws QuotaExceededException, UnresolvedLinkException { assert hasWriteLock(); byte[][] components = INode.getPathComponents(src); - INode[] inodes = new INode[components.length]; - - rootDir.getExistingPathINodes(components, inodes, false); + INodesInPath inodesInPath = rootDir.getExistingPathINodes(components, + components.length, false); + INode[] inodes = inodesInPath.getINodes(); unprotectedMkdir(inodes, inodes.length-1, components[inodes.length-1], permissions, timestamp); return inodes[inodes.length-1]; @@ -1626,10 +1628,11 @@ private T addNode(String src, T child, byte[] path = components[components.length-1]; child.setLocalName(path); cacheName(child); - INode[] inodes = new INode[components.length]; writeLock(); try { - rootDir.getExistingPathINodes(components, inodes, false); + INodesInPath inodesInPath = rootDir.getExistingPathINodes(components, + components.length, false); + INode[] inodes = inodesInPath.getINodes(); return addChild(inodes, inodes.length-1, child, childDiskspace); } finally { writeUnlock(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java index 46a9b1b5b4..6562e46bdf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java @@ -135,9 +135,9 @@ private INode getChildINode(byte[] name) { */ private INode getNode(byte[][] components, boolean resolveLink ) throws UnresolvedLinkException { - INode[] inode = new INode[1]; - getExistingPathINodes(components, inode, resolveLink); - return inode[0]; + INodesInPath inodesInPath = getExistingPathINodes(components, 1, + resolveLink); + return inodesInPath.inodes[0]; } /** @@ -185,27 +185,29 @@ INode getNode(String path, boolean resolveLink) * fill the array with [rootINode,c1,c2,null] * * @param components array of path component name - * @param existing array to fill with existing INodes + * @param numOfINodes number of INodes to return * @param resolveLink indicates whether UnresolvedLinkException should * be thrown when the path refers to a symbolic link. - * @return number of existing INodes in the path + * @return the specified number of existing INodes in the path */ - int getExistingPathINodes(byte[][] components, INode[] existing, - boolean resolveLink) throws UnresolvedLinkException { + INodesInPath getExistingPathINodes(byte[][] components, int numOfINodes, + boolean resolveLink) + throws UnresolvedLinkException { assert this.compareTo(components[0]) == 0 : "Incorrect name " + getLocalName() + " expected " + (components[0] == null? null: DFSUtil.bytes2String(components[0])); + INodesInPath existing = new INodesInPath(numOfINodes); INode curNode = this; int count = 0; - int index = existing.length - components.length; + int index = numOfINodes - components.length; if (index > 0) { index = 0; } while (count < components.length && curNode != null) { final boolean lastComp = (count == components.length - 1); if (index >= 0) { - existing[index] = curNode; + existing.inodes[index] = curNode; } if (curNode.isLink() && (!lastComp || (lastComp && resolveLink))) { final String path = constructPath(components, 0, components.length); @@ -230,7 +232,7 @@ int getExistingPathINodes(byte[][] components, INode[] existing, count++; index++; } - return count; + return existing; } /** @@ -251,11 +253,9 @@ int getExistingPathINodes(byte[][] components, INode[] existing, INode[] getExistingPathINodes(String path, boolean resolveLink) throws UnresolvedLinkException { byte[][] components = getPathComponents(path); - INode[] inodes = new INode[components.length]; - - this.getExistingPathINodes(components, inodes, resolveLink); - - return inodes; + INodesInPath inodes = this.getExistingPathINodes(components, + components.length, resolveLink); + return inodes.inodes; } /** @@ -346,9 +346,8 @@ INodeDirectory getParent(byte[][] pathComponents if (pathComponents.length < 2) // add root return null; // Gets the parent INode - INode[] inodes = new INode[2]; - getExistingPathINodes(pathComponents, inodes, false); - INode inode = inodes[0]; + INodesInPath inodes = getExistingPathINodes(pathComponents, 2, false); + INode inode = inodes.inodes[0]; if (inode == null) { throw new FileNotFoundException("Parent path does not exist: "+ DFSUtil.byteArray2String(pathComponents)); @@ -452,4 +451,22 @@ int collectSubtreeBlocksAndClear(List v) { children = null; return total; } + + /** + * Used by + * {@link INodeDirectory#getExistingPathINodes(byte[][], int, boolean)}. + * Containing INodes information resolved from a given path. + */ + static class INodesInPath { + private INode[] inodes; + + public INodesInPath(int number) { + assert (number >= 0); + this.inodes = new INode[number]; + } + + INode[] getINodes() { + return inodes; + } + } }