You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ha...@apache.org on 2008/06/19 19:19:59 UTC
svn commit: r669574 - in /hadoop/core/trunk: CHANGES.txt
src/hdfs/org/apache/hadoop/dfs/FSDirectory.java
src/test/org/apache/hadoop/dfs/TestDFSRename.java
Author: hairong
Date: Thu Jun 19 10:19:59 2008
New Revision: 669574
URL: http://svn.apache.org/viewvc?rev=669574&view=rev
Log:
HADOOP-3576. Fix NullPointerException when renaming a directory to its subdirectory. Contributed by Tsz Wo (Nicholas), SZE.
Modified:
hadoop/core/trunk/CHANGES.txt
hadoop/core/trunk/src/hdfs/org/apache/hadoop/dfs/FSDirectory.java
hadoop/core/trunk/src/test/org/apache/hadoop/dfs/TestDFSRename.java
Modified: hadoop/core/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/CHANGES.txt?rev=669574&r1=669573&r2=669574&view=diff
==============================================================================
--- hadoop/core/trunk/CHANGES.txt (original)
+++ hadoop/core/trunk/CHANGES.txt Thu Jun 19 10:19:59 2008
@@ -15,6 +15,9 @@
HADOOP-3563. Refactor the distributed upgrade code so that it is
easier to identify datanode and namenode related code. (dhruba)
+ HADOOP-3576. Fix NullPointerException when renaming a directory
+ to its subdirectory. (Tse Wo (Nicholas), SZE via hairong)
+
Release 0.18.0 - Unreleased
INCOMPATIBLE CHANGES
Modified: hadoop/core/trunk/src/hdfs/org/apache/hadoop/dfs/FSDirectory.java
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/src/hdfs/org/apache/hadoop/dfs/FSDirectory.java?rev=669574&r1=669573&r2=669574&view=diff
==============================================================================
--- hadoop/core/trunk/src/hdfs/org/apache/hadoop/dfs/FSDirectory.java (original)
+++ hadoop/core/trunk/src/hdfs/org/apache/hadoop/dfs/FSDirectory.java Thu Jun 19 10:19:59 2008
@@ -317,8 +317,7 @@
/**
* @see #unprotectedRenameTo(String, String, long)
*/
- public boolean renameTo(String src, String dst)
- throws QuotaExceededException {
+ boolean renameTo(String src, String dst) throws QuotaExceededException {
if (NameNode.stateChangeLog.isDebugEnabled()) {
NameNode.stateChangeLog.debug("DIR* FSDirectory.renameTo: "
+src+" to "+dst);
@@ -359,23 +358,6 @@
dst += Path.SEPARATOR + new Path(src).getName();
}
- byte[][] dstComponents = INode.getPathComponents(dst);
- INode[] dstInodes = new INode[dstComponents.length];
- rootDir.getExistingPathINodes(dstComponents, dstInodes);
-
- // check the validity of the destination
- if (dstInodes[dstInodes.length-1] != null) { //check if destination exists
- NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
- +"failed to rename "+src+" to "+dst+
- " because destination exists");
- return false;
- } else if (dstInodes[dstInodes.length-2] == null) { // check if its parent exists
- NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
- +"failed to rename "+src+" to "+dst+
- " because destination's parent does not exists");
- return false;
- }
-
// remove source
INode srcChild = null;
try {
@@ -389,20 +371,37 @@
return false;
}
- // add to the destination
+ // check the validity of the destination
INode dstChild = null;
QuotaExceededException failureByQuota = null;
- try {
- // set the destination's name
+
+ byte[][] dstComponents = INode.getPathComponents(dst);
+ INode[] dstInodes = new INode[dstComponents.length];
+ rootDir.getExistingPathINodes(dstComponents, dstInodes);
+ if (dstInodes[dstInodes.length-1] != null) { //check if destination exists
+ NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
+ +"failed to rename "+src+" to "+dst+
+ " because destination exists");
+ } else if (dstInodes[dstInodes.length-2] == null) { // check if its parent exists
+ NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
+ +"failed to rename "+src+" to "+dst+
+ " because destination's parent does not exists");
+ }
+ else {
+ // add to the destination
srcChild.setLocalName(dstComponents[dstInodes.length-1]);
- // add it to the namespace
- dstChild = addChild(dstInodes, dstInodes.length-1, srcChild, false);
- } catch (QuotaExceededException qe) {
- failureByQuota = qe;
+ try {
+ // add it to the namespace
+ dstChild = addChild(dstInodes, dstInodes.length-1, srcChild, false);
+ } catch (QuotaExceededException qe) {
+ failureByQuota = qe;
+ }
}
if (dstChild != null) {
- NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedRenameTo: "
+ if (NameNode.stateChangeLog.isDebugEnabled()) {
+ NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedRenameTo: "
+src+" is renamed to "+dst);
+ }
// update modification time of dst and the parent of src
srcInodes[srcInodes.length-2].setModificationTime(timestamp);
Modified: hadoop/core/trunk/src/test/org/apache/hadoop/dfs/TestDFSRename.java
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/src/test/org/apache/hadoop/dfs/TestDFSRename.java?rev=669574&r1=669573&r2=669574&view=diff
==============================================================================
--- hadoop/core/trunk/src/test/org/apache/hadoop/dfs/TestDFSRename.java (original)
+++ hadoop/core/trunk/src/test/org/apache/hadoop/dfs/TestDFSRename.java Thu Jun 19 10:19:59 2008
@@ -45,36 +45,50 @@
FileSystem fs = cluster.getFileSystem();
assertTrue(fs.mkdirs(dir));
- Path a = new Path(dir, "a");
- Path aa = new Path(dir, "aa");
- Path b = new Path(dir, "b");
+ { //test lease
+ Path a = new Path(dir, "a");
+ Path aa = new Path(dir, "aa");
+ Path b = new Path(dir, "b");
+
+ DataOutputStream a_out = fs.create(a);
+ a_out.writeBytes("something");
+ a_out.close();
+
+ //should not have any lease
+ assertEquals(0, countLease(cluster));
+
+ DataOutputStream aa_out = fs.create(aa);
+ aa_out.writeBytes("something");
+
+ //should have 1 lease
+ assertEquals(1, countLease(cluster));
+ list(fs, "rename0");
+ fs.rename(a, b);
+ list(fs, "rename1");
+ aa_out.writeBytes(" more");
+ aa_out.close();
+ list(fs, "rename2");
+
+ //should not have any lease
+ assertEquals(0, countLease(cluster));
+ }
- DataOutputStream a_out = fs.create(a);
- a_out.writeBytes("something");
- a_out.close();
-
- //should not have any lease
- assertEquals(0, countLease(cluster));
-
- DataOutputStream aa_out = fs.create(aa);
- aa_out.writeBytes("something");
-
- //should have 1 lease
- assertEquals(1, countLease(cluster));
- list(fs, "rename0");
- fs.rename(a, b);
- list(fs, "rename1");
- aa_out.writeBytes(" more");
- aa_out.close();
- list(fs, "rename2");
+ { // test non-existent destination
+ Path dstPath = new Path("/c/d");
+ assertFalse(fs.exists(dstPath));
+ assertFalse(fs.rename(dir, dstPath));
+ }
- //should not have any lease
- assertEquals(0, countLease(cluster));
+ { // test rename /a/b to /a/b/c
+ Path src = new Path("/a/b");
+ Path dst = new Path("/a/b/c");
- // test non-existent destination
- Path dstPath = new Path("/c/d");
- assertFalse(fs.exists(dstPath));
- assertFalse(fs.rename(dir, dstPath));
+ DataOutputStream a_out = fs.create(new Path(src, "foo"));
+ a_out.writeBytes("something");
+ a_out.close();
+
+ assertFalse(fs.rename(src, dst));
+ }
fs.delete(dir, true);
} finally {