HDFS-1138. Prevent erroneous updation of modification time of a directory

when fsimage loads. (Dmytro Molkov via dhruba)


git-svn-id: https://svn.apache.org/repos/asf/hadoop/hdfs/trunk@943306 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -878,6 +878,9 @@ Release 0.21.0 - Unreleased
     HDFS-1141. Closing a file is successful only if the client still has a
     valid lease. (Todd Lipcon via dhruba)
 
+    HDFS-1138. Prevent erroneous updation of modification time of a directory
+    when fsimage loads. (Dmytro Molkov via dhruba)
+
 Release 0.20.3 - Unreleased
 
   IMPROVEMENTS

src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java

@@ -239,7 +239,8 @@ class FSDirectory implements Closeable {
                               long atime,
                               long nsQuota,
                               long dsQuota,
-                              long preferredBlockSize) 
+                              long preferredBlockSize,
+                              boolean propagateModTime) 
                               throws UnresolvedLinkException {
     // NOTE: This does not update space counts for parents
     // create new inode
@@ -264,7 +265,8 @@ class FSDirectory implements Closeable {
     INodeDirectory newParent = null;
     synchronized (rootDir) {
       try {
-        newParent = rootDir.addToParent(src, newNode, parentINode, false);
+        newParent = rootDir.addToParent(src, newNode, parentINode,
+                                        false, propagateModTime);
       } catch (FileNotFoundException e) {
         return null;
       }
@@ -1507,7 +1509,7 @@ class FSDirectory implements Closeable {
       throw new NullPointerException("Panic: parent does not exist");
     }
     T addedNode = ((INodeDirectory)pathComponents[pos-1]).addChild(
-        child, inheritPermission);
+        child, inheritPermission, true);
     if (addedNode == null) {
       updateCount(pathComponents, pos, -counts.getNsCount(), 
           -childDiskspace, true);

src/java/org/apache/hadoop/hdfs/server/namenode/FSImage.java

@@ -1157,9 +1157,10 @@ public class FSImage extends Storage {
           parentPath = getParent(path);
         }
         // add new inode
+        // without propagating modification time to parent
         parentINode = fsDir.addToParent(path, parentINode, permissions,
                                         blocks, symlink, replication, modificationTime, 
-                                        atime, nsQuota, dsQuota, blockSize);
+                                        atime, nsQuota, dsQuota, blockSize, false);
       }
       
       // load datanode info

src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java

@@ -260,10 +260,14 @@ class INodeDirectory extends INode {
    * 
    * @param node INode to insert
    * @param inheritPermission inherit permission from parent?
+   * @param setModTime set modification time for the parent node
+   *                   not needed when replaying the addition and 
+   *                   the parent already has the proper mod time
    * @return  null if the child with this name already exists; 
    *          node, otherwise
    */
-  <T extends INode> T addChild(final T node, boolean inheritPermission) {
+  <T extends INode> T addChild(final T node, boolean inheritPermission,
+                                              boolean setModTime) {
     if (inheritPermission) {
       FsPermission p = getFsPermission();
       //make sure the  permission has wx for the user
@@ -283,7 +287,8 @@ class INodeDirectory extends INode {
     node.parent = this;
     children.add(-low - 1, node);
     // update modification time of the parent directory
-    setModificationTime(node.getModificationTime());
+    if (setModTime)
+      setModificationTime(node.getModificationTime());
     if (node.getGroupName() == null) {
       node.setGroup(getGroupName());
     }
@@ -312,7 +317,7 @@ class INodeDirectory extends INode {
    */
   <T extends INode> T addNode(String path, T newNode, boolean inheritPermission
       ) throws FileNotFoundException, UnresolvedLinkException  {
-    if(addToParent(path, newNode, null, inheritPermission) == null)
+    if(addToParent(path, newNode, null, inheritPermission, true) == null)
       return null;
     return newNode;
   }
@@ -333,6 +338,16 @@ class INodeDirectory extends INode {
                                       boolean inheritPermission
                                     ) throws FileNotFoundException, 
                                              UnresolvedLinkException {
+    return addToParent(path, newNode, parent, inheritPermission, true);
+  }
+  <T extends INode> INodeDirectory addToParent(
+                                      String path,
+                                      T newNode,
+                                      INodeDirectory parent,
+                                      boolean inheritPermission,
+                                      boolean propagateModTime
+                                    ) throws FileNotFoundException, 
+                                             UnresolvedLinkException {
     byte[][] pathComponents = getPathComponents(path);
     assert pathComponents != null : "Incorrect path " + path;
     int pathLen = pathComponents.length;
@@ -353,7 +368,7 @@ class INodeDirectory extends INode {
     }
     // insert into the parent children list
     newNode.name = pathComponents[pathLen-1];
-    if(parent.addChild(newNode, inheritPermission) == null)
+    if(parent.addChild(newNode, inheritPermission, propagateModTime) == null)
       return null;
     return parent;
   }

src/test/hdfs/org/apache/hadoop/hdfs/TestRestartDFS.java

@@ -69,6 +69,26 @@ public class TestRestartDFS extends TestCase {
       assertEquals(rootstatus.getOwner() + "_XXX", newrootstatus.getOwner());
       assertEquals(rootstatus.getGroup(), newrootstatus.getGroup());
 
+      final FileStatus newdirstatus = fs.getFileStatus(dirpath);
+      assertEquals(dirstatus.getOwner(), newdirstatus.getOwner());
+      assertEquals(dirstatus.getGroup() + "_XXX", newdirstatus.getGroup());
+      rootmtime = fs.getFileStatus(rootpath).getModificationTime();
+    } finally {
+      if (cluster != null) { cluster.shutdown(); }
+    }
+    try {
+      // This is a second restart to check that after the first restart
+      // the image written in parallel to both places did not get corrupted
+      cluster = new MiniDFSCluster(conf, 4, false, null);
+      FileSystem fs = cluster.getFileSystem();
+      assertTrue("Filesystem corrupted after restart.",
+                 files.checkFiles(fs, dir));
+
+      final FileStatus newrootstatus = fs.getFileStatus(rootpath);
+      assertEquals(rootmtime, newrootstatus.getModificationTime());
+      assertEquals(rootstatus.getOwner() + "_XXX", newrootstatus.getOwner());
+      assertEquals(rootstatus.getGroup(), newrootstatus.getGroup());
+
       final FileStatus newdirstatus = fs.getFileStatus(dirpath);
       assertEquals(dirstatus.getOwner(), newdirstatus.getOwner());
       assertEquals(dirstatus.getGroup() + "_XXX", newdirstatus.getGroup());