Author: szetszwo
Date: Fri Oct 26 18:13:59 2012
New Revision: 1402602
URL: http://svn.apache.org/viewvc?rev=1402602&view=rev
Log:
svn merge -c 1402599 from trunk for HDFS-4112. A few improvements on
INodeDirectory include adding a utility method for casting; avoiding creation
of new empty lists; cleaning up some code and rewriting some javadoc.
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/ (props
changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestINodeFile.java
Propchange: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/
------------------------------------------------------------------------------
Merged /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs:r1402599
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
(original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
Fri Oct 26 18:13:59 2012
@@ -79,6 +79,10 @@ Release 2.0.3-alpha - Unreleased
HDFS-4107. Add utility methods for casting INode to INodeFile and
INodeFileUnderConstruction. (szetszwo)
+ HDFS-4112. A few improvements on INodeDirectory include adding a utility
+ method for casting; avoiding creation of new empty lists; cleaning up
+ some code and rewriting some javadoc. (szetszwo)
+
OPTIMIZATIONS
BUG FIXES
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java:r1402599
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
Fri Oct 26 18:13:59 2012
@@ -696,7 +696,7 @@ public class FSDirectory implements Clos
throw new FileAlreadyExistsException(error);
}
List<INode> children = dstInode.isDirectory() ?
- ((INodeDirectory) dstInode).getChildrenRaw() : null;
+ ((INodeDirectory) dstInode).getChildren() : null;
if (children != null && children.size() != 0) {
error = "rename cannot overwrite non empty destination directory "
+ dst;
@@ -1019,35 +1019,21 @@ public class FSDirectory implements Clos
return true;
}
- /** Return if a directory is empty or not **/
- boolean isDirEmpty(String src) throws UnresolvedLinkException {
- boolean dirNotEmpty = true;
- if (!isDir(src)) {
- return true;
- }
+ /**
+ * @return true if the path is a non-empty directory; otherwise, return
false.
+ */
+ boolean isNonEmptyDirectory(String path) throws UnresolvedLinkException {
readLock();
try {
- INode targetNode = rootDir.getNode(src, false);
- assert targetNode != null : "should be taken care in isDir() above";
- if (((INodeDirectory)targetNode).getChildren().size() != 0) {
- dirNotEmpty = false;
+ final INode inode = rootDir.getNode(path, false);
+ if (inode == null || !inode.isDirectory()) {
+ //not found or not a directory
+ return false;
}
+ return ((INodeDirectory)inode).getChildrenList().size() != 0;
} finally {
readUnlock();
}
- return dirNotEmpty;
- }
-
- boolean isEmpty() {
- try {
- return isDirEmpty("/");
- } catch (UnresolvedLinkException e) {
- if(NameNode.stateChangeLog.isDebugEnabled()) {
- NameNode.stateChangeLog.debug("/ cannot be a symlink");
- }
- assert false : "/ cannot be a symlink";
- return true;
- }
}
/**
@@ -1171,7 +1157,7 @@ public class FSDirectory implements Clos
targetNode, needLocation)}, 0);
}
INodeDirectory dirInode = (INodeDirectory)targetNode;
- List<INode> contents = dirInode.getChildren();
+ List<INode> contents = dirInode.getChildrenList();
int startChild = dirInode.nextChild(startAfter);
int totalNumChildren = contents.size();
int numOfListing = Math.min(totalNumChildren-startChild, this.lsLimit);
@@ -1683,7 +1669,7 @@ public class FSDirectory implements Clos
}
if (maxDirItems != 0) {
INodeDirectory parent = (INodeDirectory)pathComponents[pos-1];
- int count = parent.getChildren().size();
+ int count = parent.getChildrenList().size();
if (count >= maxDirItems) {
throw new MaxDirectoryItemsExceededException(maxDirItems, count);
}
@@ -1832,7 +1818,7 @@ public class FSDirectory implements Clos
* INode. using 'parent' is not currently recommended. */
nodesInPath.add(dir);
- for (INode child : dir.getChildren()) {
+ for (INode child : dir.getChildrenList()) {
if (child.isDirectory()) {
updateCountForINodeWithQuota((INodeDirectory)child,
counts, nodesInPath);
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
Fri Oct 26 18:13:59 2012
@@ -246,10 +246,8 @@ class FSImageFormat {
private int loadDirectory(DataInputStream in) throws IOException {
String parentPath = FSImageSerialization.readString(in);
FSDirectory fsDir = namesystem.dir;
- INode parent = fsDir.rootDir.getNode(parentPath, true);
- if (parent == null || !parent.isDirectory()) {
- throw new IOException("Path " + parentPath + "is not a directory.");
- }
+ final INodeDirectory parent = INodeDirectory.valueOf(
+ fsDir.rootDir.getNode(parentPath, true), parentPath);
int numChildren = in.readInt();
for(int i=0; i<numChildren; i++) {
@@ -259,7 +257,7 @@ class FSImageFormat {
INode newNode = loadINode(in); // read rest of inode
// add to parent
- namesystem.dir.addToParent(localName, (INodeDirectory)parent, newNode,
false);
+ namesystem.dir.addToParent(localName, parent, newNode, false);
}
return numChildren;
}
@@ -532,7 +530,7 @@ class FSImageFormat {
private void saveImage(ByteBuffer currentDirName,
INodeDirectory current,
DataOutputStream out) throws IOException {
- List<INode> children = current.getChildrenRaw();
+ List<INode> children = current.getChildren();
if (children == null || children.isEmpty())
return;
// print prefix (parent directory name)
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Fri Oct 26 18:13:59 2012
@@ -2661,7 +2661,7 @@ public class FSNamesystem implements Nam
if (isInSafeMode()) {
throw new SafeModeException("Cannot delete " + src, safeMode);
}
- if (!recursive && !dir.isDirEmpty(src)) {
+ if (!recursive && dir.isNonEmptyDirectory(src)) {
throw new IOException(src + " is non empty");
}
if (enforcePermission && isPermissionEnabled) {
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
Fri Oct 26 18:13:59 2012
@@ -173,7 +173,7 @@ class FSPermissionChecker {
INodeDirectory d = directories.pop();
check(d, access);
- for(INode child : d.getChildren()) {
+ for(INode child : d.getChildrenList()) {
if (child.isDirectory()) {
directories.push((INodeDirectory)child);
}
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
Fri Oct 26 18:13:59 2012
@@ -17,7 +17,9 @@
*/
package org.apache.hadoop.hdfs.server.namenode;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.apache.hadoop.classification.InterfaceAudience;
@@ -39,7 +41,8 @@ import com.google.common.primitives.Sign
*/
@InterfaceAudience.Private
abstract class INode implements Comparable<byte[]> {
- /*
+ static final List<INode> EMPTY_LIST = Collections.unmodifiableList(new
ArrayList<INode>());
+ /**
* The inode name is in java UTF8 encoding;
* The name in HdfsFileStatus should keep the same encoding as this.
* if this encoding is changed, implicitly getFileInfo and listStatus in
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
Fri Oct 26 18:13:59 2012
@@ -18,6 +18,7 @@
package org.apache.hadoop.hdfs.server.namenode;
import java.io.FileNotFoundException;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -32,6 +33,18 @@ import org.apache.hadoop.hdfs.protocol.U
* Directory INode class.
*/
class INodeDirectory extends INode {
+ /** Cast INode to INodeDirectory. */
+ public static INodeDirectory valueOf(INode inode, String path
+ ) throws IOException {
+ if (inode == null) {
+ throw new IOException("Directory does not exist: " + path);
+ }
+ if (!inode.isDirectory()) {
+ throw new IOException("Path is not a directory: " + path);
+ }
+ return (INodeDirectory)inode;
+ }
+
protected static final int DEFAULT_FILES_PER_DIRECTORY = 5;
final static String ROOT_NAME = "";
@@ -112,11 +125,11 @@ class INodeDirectory extends INode {
}
/**
- * Return the INode of the last component in components, or null if the last
+ * @return the INode of the last component in components, or null if the last
* component does not exist.
*/
- private INode getNode(byte[][] components, boolean resolveLink)
- throws UnresolvedLinkException {
+ private INode getNode(byte[][] components, boolean resolveLink
+ ) throws UnresolvedLinkException {
INode[] inode = new INode[1];
getExistingPathINodes(components, inode, resolveLink);
return inode[0];
@@ -299,10 +312,7 @@ class INodeDirectory extends INode {
<T extends INode> T addNode(String path, T newNode
) throws FileNotFoundException, UnresolvedLinkException {
byte[][] pathComponents = getPathComponents(path);
- if(addToParent(pathComponents, newNode,
- true) == null)
- return null;
- return newNode;
+ return addToParent(pathComponents, newNode, true) == null? null: newNode;
}
/**
@@ -326,10 +336,9 @@ class INodeDirectory extends INode {
return parent;
}
- INodeDirectory getParent(byte[][] pathComponents)
- throws FileNotFoundException, UnresolvedLinkException {
- int pathLen = pathComponents.length;
- if (pathLen < 2) // add root
+ INodeDirectory getParent(byte[][] pathComponents
+ ) throws FileNotFoundException, UnresolvedLinkException {
+ if (pathComponents.length < 2) // add root
return null;
// Gets the parent INode
INode[] inodes = new INode[2];
@@ -355,21 +364,15 @@ class INodeDirectory extends INode {
* @throws FileNotFoundException if parent does not exist or
* is not a directory.
*/
- INodeDirectory addToParent( byte[][] pathComponents,
- INode newNode,
- boolean propagateModTime
- ) throws FileNotFoundException,
- UnresolvedLinkException {
-
- int pathLen = pathComponents.length;
- if (pathLen < 2) // add root
+ INodeDirectory addToParent(byte[][] pathComponents, INode newNode,
+ boolean propagateModTime) throws FileNotFoundException,
UnresolvedLinkException {
+ if (pathComponents.length < 2) { // add root
return null;
- newNode.name = pathComponents[pathLen-1];
+ }
+ newNode.name = pathComponents[pathComponents.length - 1];
// insert into the parent children list
INodeDirectory parent = getParent(pathComponents);
- if(parent.addChild(newNode, propagateModTime) == null)
- return null;
- return parent;
+ return parent.addChild(newNode, propagateModTime) == null? null: parent;
}
@Override
@@ -415,11 +418,15 @@ class INodeDirectory extends INode {
}
/**
+ * @return an empty list if the children list is null;
+ * otherwise, return the children list.
+ * The returned list should not be modified.
*/
- List<INode> getChildren() {
- return children==null ? new ArrayList<INode>() : children;
+ public List<INode> getChildrenList() {
+ return children==null ? EMPTY_LIST : children;
}
- List<INode> getChildrenRaw() {
+ /** @return the children list which is possibly null. */
+ public List<INode> getChildren() {
return children;
}
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestINodeFile.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestINodeFile.java?rev=1402602&r1=1402601&r2=1402602&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestINodeFile.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestINodeFile.java
Fri Oct 26 18:13:59 2012
@@ -234,6 +234,14 @@ public class TestINodeFile {
} catch(FileNotFoundException fnfe) {
assertTrue(fnfe.getMessage().contains("File does not exist"));
}
+
+ //cast to INodeDirectory, should fail
+ try {
+ INodeDirectory.valueOf(from, path);
+ fail();
+ } catch(IOException ioe) {
+ assertTrue(ioe.getMessage().contains("Directory does not exist"));
+ }
}
{//cast from INodeFile
@@ -251,6 +259,14 @@ public class TestINodeFile {
} catch(IOException ioe) {
assertTrue(ioe.getMessage().contains("File is not under
construction"));
}
+
+ //cast to INodeDirectory, should fail
+ try {
+ INodeDirectory.valueOf(from, path);
+ fail();
+ } catch(IOException ioe) {
+ assertTrue(ioe.getMessage().contains("Path is not a directory"));
+ }
}
{//cast from INodeFileUnderConstruction
@@ -265,6 +281,14 @@ public class TestINodeFile {
final INodeFileUnderConstruction u = INodeFileUnderConstruction.valueOf(
from, path);
assertTrue(u == from);
+
+ //cast to INodeDirectory, should fail
+ try {
+ INodeDirectory.valueOf(from, path);
+ fail();
+ } catch(IOException ioe) {
+ assertTrue(ioe.getMessage().contains("Path is not a directory"));
+ }
}
{//cast from INodeDirectory
@@ -285,6 +309,10 @@ public class TestINodeFile {
} catch(FileNotFoundException fnfe) {
assertTrue(fnfe.getMessage().contains("Path is not a file"));
}
+
+ //cast to INodeDirectory, should success
+ final INodeDirectory d = INodeDirectory.valueOf(from, path);
+ assertTrue(d == from);
}
}
}
