hadoop-hdfs-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From szets...@apache.org
Subject svn commit: r1443825 [1/2] - in /hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs: ./ src/main/java/org/apache/hadoop/hdfs/server/namenode/ src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/ src/test/java/org/apache/hadoop/h...
Date Fri, 08 Feb 2013 02:18:56 GMT
Author: szetszwo
Date: Fri Feb  8 02:18:55 2013
New Revision: 1443825

URL: http://svn.apache.org/r1443825
Log:
HDFS-4446. Support file snapshots with diff lists.

Removed:
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionSnapshot.java
Modified:
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectoryWithQuota.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFileUnderConstruction.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeSymlink.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotFSImageFormat.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImageWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotTestHelper.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestINodeFileUnderConstructionWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt Fri Feb  8 02:18:55 2013
@@ -149,3 +149,5 @@ Branch-2802 Snapshot (Unreleased)
 
   HDFS-4414. Add support for getting snapshot diff from DistributedFileSystem.
   (Jing Zhao via suresh)
+
+  HDFS-4446. Support file snapshots with diff lists.  (szetszwo)

Modified: hadoop/common/branches/HDFS-2802/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/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java Fri Feb  8 02:18:55 2013
@@ -60,6 +60,7 @@ import org.apache.hadoop.hdfs.server.blo
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState;
 import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo;
 import org.apache.hadoop.hdfs.server.namenode.INodeDirectory.INodesInPath;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.FileWithSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectorySnapshottable;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotAccessControlException;
@@ -1123,28 +1124,35 @@ public class FSDirectory implements Clos
       BlocksMapUpdateInfo collectedBlocks, long mtime) {
     assert hasWriteLock();
 
-    // Remove the node from the namespace
-    final INode targetNode = removeLastINode(inodesInPath);
+    // check if target node exists
+    INode targetNode = inodesInPath.getLastINode();
     if (targetNode == null) {
       return 0;
     }
-    // set the parent's modification time
-    final INode[] inodes = inodesInPath.getINodes();
+
+    // check latest snapshot
     final Snapshot latestSnapshot = inodesInPath.getLatestSnapshot();
-    final INodeDirectory parent = (INodeDirectory)inodes[inodes.length - 2];
-    parent.updateModificationTime(mtime, latestSnapshot);
+    final INode snapshotCopy = ((INodeDirectory)inodesInPath.getINode(-2))
+        .getChild(targetNode.getLocalNameBytes(), latestSnapshot);
+    if (snapshotCopy == targetNode) {
+      // it is also in a snapshot, record modification before delete it
+      targetNode = targetNode.recordModification(latestSnapshot);
+    }
+
+    // Remove the node from the namespace
+    final INode removed = removeLastINode(inodesInPath);
+    Preconditions.checkState(removed == targetNode);
 
-    final INode snapshotCopy = parent.getChild(targetNode.getLocalNameBytes(),
-        latestSnapshot);
-    // if snapshotCopy == targetNode, it means that the file is also stored in
-    // a snapshot so that the block should not be removed.
-    final int filesRemoved = snapshotCopy == targetNode? 0
-        : targetNode.destroySubtreeAndCollectBlocks(null, collectedBlocks);
+    // set the parent's modification time
+    targetNode.getParent().updateModificationTime(mtime, latestSnapshot);
+
+    final int inodesRemoved = targetNode.destroySubtreeAndCollectBlocks(
+        null, collectedBlocks);
     if (NameNode.stateChangeLog.isDebugEnabled()) {
       NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedDelete: "
           + targetNode.getFullPathName() + " is removed");
     }
-    return filesRemoved;
+    return inodesRemoved;
   }
   
   /**
@@ -1184,7 +1192,7 @@ public class FSDirectory implements Clos
   /**
    * Replaces the specified INodeFile with the specified one.
    */
-  public void replaceINodeFile(String path, INodeFile oldnode,
+  void replaceINodeFile(String path, INodeFile oldnode,
       INodeFile newnode, Snapshot latest) throws IOException {
     writeLock();
     try {
@@ -1194,17 +1202,27 @@ public class FSDirectory implements Clos
     }
   }
 
+  /** Replace an INodeFile and record modification for the latest snapshot. */
   void unprotectedReplaceINodeFile(final String path, final INodeFile oldnode,
       final INodeFile newnode, final Snapshot latest) {
     Preconditions.checkState(hasWriteLock());
 
-    final INodeDirectory parent = oldnode.getParent();
+    INodeDirectory parent = oldnode.getParent();
     final INode removed = parent.removeChild(oldnode, latest);
     Preconditions.checkState(removed == oldnode,
         "removed != oldnode=%s, removed=%s", oldnode, removed);
 
-    oldnode.setParent(null);
-    parent.addChild(newnode, true, latest);
+    //cleanup the removed object
+    parent = removed.getParent(); //parent could be replaced.
+    removed.clearReferences();
+    if (removed instanceof FileWithSnapshot) {
+      final FileWithSnapshot withSnapshot = (FileWithSnapshot)removed;
+      if (withSnapshot.isEverythingDeleted()) {
+        withSnapshot.removeSelf();
+      }
+    }
+
+    parent.addChild(newnode, false, latest);
 
     /* Currently oldnode and newnode are assumed to contain the same
      * blocks. Otherwise, blocks need to be removed from the blocksMap.
@@ -1244,8 +1262,7 @@ public class FSDirectory implements Clos
       }
 
       INodeDirectory dirInode = (INodeDirectory)targetNode;
-      final ReadOnlyList<INode> contents = dirInode.getChildrenList(
-          inodesInPath.getPathSnapshot());
+      final ReadOnlyList<INode> contents = dirInode.getChildrenList(snapshot);
       int startChild = INodeDirectory.nextChild(contents, startAfter);
       int totalNumChildren = contents.size();
       int numOfListing = Math.min(totalNumChildren-startChild, this.lsLimit);
@@ -2141,8 +2158,8 @@ public class FSDirectory implements Clos
      long blocksize = 0;
      if (node instanceof INodeFile) {
        INodeFile fileNode = (INodeFile)node;
-       size = fileNode.computeFileSize(true);
-       replication = fileNode.getFileReplication();
+       size = fileNode.computeFileSize(true, snapshot);
+       replication = fileNode.getFileReplication(snapshot);
        blocksize = fileNode.getPreferredBlockSize();
      }
      return new HdfsFileStatus(
@@ -2171,11 +2188,11 @@ public class FSDirectory implements Clos
       LocatedBlocks loc = null;
       if (node instanceof INodeFile) {
         INodeFile fileNode = (INodeFile)node;
-        size = fileNode.computeFileSize(true);
-        replication = fileNode.getFileReplication();
+        size = fileNode.computeFileSize(true, snapshot);
+        replication = fileNode.getFileReplication(snapshot);
         blocksize = fileNode.getPreferredBlockSize();
         loc = getFSNamesystem().getBlockManager().createLocatedBlocks(
-            fileNode.getBlocks(), fileNode.computeFileSize(false),
+            fileNode.getBlocks(), fileNode.computeFileSize(false, snapshot),
             fileNode.isUnderConstruction(), 0L, size, false);
         if (loc==null) {
           loc = new LocatedBlocks();

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageSerialization.java Fri Feb  8 02:18:55 2013
@@ -36,9 +36,8 @@ import org.apache.hadoop.hdfs.server.com
 import org.apache.hadoop.hdfs.server.namenode.snapshot.FileWithSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectorySnapshottable;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot;
-import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileSnapshot;
-import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileUnderConstructionSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileUnderConstructionWithSnapshot;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileWithSnapshot;
 import org.apache.hadoop.io.LongWritable;
 import org.apache.hadoop.io.ShortWritable;
 import org.apache.hadoop.io.Text;
@@ -229,16 +228,10 @@ public class FSImageSerialization {
       out.writeInt(0); // # of blocks
       out.writeBoolean(false);
     }
-    if (node instanceof INodeFileSnapshot
-        || node instanceof INodeFileUnderConstructionSnapshot) {
-      out.writeLong(node.computeFileSize(true));
-      if (node instanceof INodeFileUnderConstructionSnapshot) {
-        out.writeBoolean(true);
-        writeString(((INodeFileUnderConstruction) node).getClientName(), out);
-        writeString(((INodeFileUnderConstruction) node).getClientMachine(), out);
-      } else {
-        out.writeBoolean(false);
-      }
+//  TODO: fix snapshot fsimage
+    if (node instanceof INodeFileWithSnapshot) {
+      out.writeLong(node.computeFileSize(true, null));
+      out.writeBoolean(false);
     } else {
       out.writeLong(-1);
       out.writeBoolean(node instanceof FileWithSnapshot);

Modified: hadoop/common/branches/HDFS-2802/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/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Fri Feb  8 02:18:55 2013
@@ -1376,8 +1376,8 @@ public class FSNamesystem implements Nam
           dir.setTimes(src, inode, -1, now, false, iip.getLatestSnapshot());
         }
         return blockManager.createLocatedBlocks(inode.getBlocks(),
-            inode.computeFileSize(false), inode.isUnderConstruction(),
-            offset, length, needBlockToken);
+            inode.computeFileSize(false, iip.getPathSnapshot()),
+            inode.isUnderConstruction(), offset, length, needBlockToken);
       } finally {
         if (attempt == 0) {
           readUnlock();
@@ -3306,8 +3306,7 @@ public class FSNamesystem implements Nam
         dir.replaceINodeFile(src, pendingFile, pendingFileWithSnaphsot, null);
         pendingFile = pendingFileWithSnaphsot;
       }
-      pendingFile = (INodeFileUnderConstruction) pendingFile
-          .recordModification(latestSnapshot);
+      pendingFile = pendingFile.recordModification(latestSnapshot);
     }
 
     // The file is no longer pending.

Modified: hadoop/common/branches/HDFS-2802/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/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java Fri Feb  8 02:18:55 2013
@@ -37,15 +37,12 @@ import org.apache.hadoop.hdfs.server.blo
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectorySnapshottable;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot;
-import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileSnapshot;
-import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileUnderConstructionSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileWithSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.diff.Diff;
 import org.apache.hadoop.util.StringUtils;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
 import com.google.common.primitives.SignedBytes;
 
 /**
@@ -181,20 +178,6 @@ public abstract class INode implements D
   public long getId() {
     return this.id;
   }
-  
-  /**
-   * Create a copy of this inode for snapshot.
-   * 
-   * @return a pair of inodes, where the left inode is the current inode and
-   *         the right inode is the snapshot copy. The current inode usually is
-   *         the same object of this inode. However, in some cases, the inode
-   *         may be replaced with a new inode for maintaining snapshot data.
-   *         Then, the current inode is the new inode.
-   */
-  public Pair<? extends INode, ? extends INode> createSnapshotCopy() {
-    throw new UnsupportedOperationException(getClass().getSimpleName()
-        + " does not support createSnapshotCopy().");
-  }
 
   /**
    * Check whether this is the root inode.
@@ -293,11 +276,7 @@ public abstract class INode implements D
    *         However, in some cases, this inode may be replaced with a new inode
    *         for maintaining snapshots. The current inode is then the new inode.
    */
-  INode recordModification(final Snapshot latest) {
-    Preconditions.checkState(!isDirectory(),
-        "this is an INodeDirectory, this=%s", this);
-    return parent.saveChild2Snapshot(this, latest);
-  }
+  abstract INode recordModification(final Snapshot latest);
 
   /**
    * Check whether it's a file.
@@ -323,7 +302,7 @@ public abstract class INode implements D
    * @param snapshot the snapshot to be deleted; null means the current state.
    * @param collectedBlocks blocks collected from the descents for further block
    *                        deletion/update will be added to the given map.
-   * @return the number of deleted files in the subtree.
+   * @return the number of deleted inodes in the subtree.
    */
   abstract int destroySubtreeAndCollectBlocks(Snapshot snapshot,
       BlocksMapUpdateInfo collectedBlocks);
@@ -410,7 +389,7 @@ public abstract class INode implements D
 
   @Override
   public String toString() {
-    return name == null? "<name==null>": getFullPathName();
+    return getLocalName();
   }
 
   @VisibleForTesting
@@ -442,7 +421,12 @@ public abstract class INode implements D
   public void setParent(INodeDirectory parent) {
     this.parent = parent;
   }
-  
+
+  /** Clear references to other objects. */
+  public void clearReferences() {
+    setParent(null);
+  }
+
   /**
    * @param snapshot
    *          if it is not null, get the result from the given snapshot;
@@ -454,16 +438,17 @@ public abstract class INode implements D
   }
 
   /** The same as getModificationTime(null). */
-  public long getModificationTime() {
+  public final long getModificationTime() {
     return getModificationTime(null);
   }
 
   /** Update modification time if it is larger than the current value. */
-  public void updateModificationTime(long mtime, Snapshot latest) {
+  public final INode updateModificationTime(long mtime, Snapshot latest) {
     assert isDirectory();
-    if (mtime > modificationTime) {
-      setModificationTime(mtime, latest);
+    if (mtime <= modificationTime) {
+      return this;
     }
+    return setModificationTime(mtime, latest);
   }
 
   void cloneModificationTime(INode that) {
@@ -473,7 +458,7 @@ public abstract class INode implements D
   /**
    * Always set the last modification time of inode.
    */
-  public INode setModificationTime(long modtime, Snapshot latest) {
+  public final INode setModificationTime(long modtime, Snapshot latest) {
     final INode nodeToUpdate = recordModification(latest);
     nodeToUpdate.modificationTime = modtime;
     return nodeToUpdate;
@@ -639,19 +624,20 @@ public abstract class INode implements D
         dir = new INodeDirectory(id, permissions, modificationTime);
       }
       return snapshottable ? new INodeDirectorySnapshottable(dir)
-          : (withSnapshot ? INodeDirectoryWithSnapshot.newInstance(dir, null)
+          : (withSnapshot ? new INodeDirectoryWithSnapshot(dir)
               : dir);
     }
     // file
     INodeFile fileNode = new INodeFile(id, permissions, blocks, replication,
         modificationTime, atime, preferredBlockSize);
-    if (computeFileSize >= 0) {
-      return underConstruction ? new INodeFileUnderConstructionSnapshot(
-          fileNode, computeFileSize, clientName, clientMachine)
-          : new INodeFileSnapshot(fileNode, computeFileSize); 
-    } else {
-      return withLink ? new INodeFileWithSnapshot(fileNode) : fileNode;
-    }
+//    TODO: fix image for file diff.
+//    if (computeFileSize >= 0) {
+//      return underConstruction ? new INodeFileUnderConstructionSnapshot(
+//          fileNode, computeFileSize, clientName, clientMachine)
+//          : new INodeFileWithSnapshot(fileNode, computeFileSize); 
+//    } else {
+      return withLink ? new INodeFileWithSnapshot(fileNode, null) : fileNode;
+//    }
   }
 
   /**

Modified: hadoop/common/branches/HDFS-2802/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/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java Fri Feb  8 02:18:55 2013
@@ -32,6 +32,7 @@ import org.apache.hadoop.hdfs.protocol.H
 import org.apache.hadoop.hdfs.protocol.UnresolvedPathException;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectorySnapshottable;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileUnderConstructionWithSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileWithSnapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotAccessControlException;
@@ -140,8 +141,7 @@ public class INodeDirectory extends INod
     assertChildrenNonNull();
 
     if (latest != null) {
-      final INodeDirectoryWithSnapshot dir = replaceSelf4INodeDirectoryWithSnapshot(latest);
-      return dir.removeChild(child, latest);
+      return recordModification(latest).removeChild(child, latest);
     }
 
     final int i = searchChildren(child.getLocalNameBytes());
@@ -163,76 +163,70 @@ public class INodeDirectory extends INod
       replaceSelf(q);
       return q;
     } else {
-      final INodeDirectoryWithSnapshot s
-          = INodeDirectoryWithSnapshot.newInstance(this, null);
+      final INodeDirectoryWithSnapshot s = new INodeDirectoryWithSnapshot(this);
       s.setQuota(nsQuota, dsQuota, null);
-      replaceSelf(s);
-      s.saveSelf2Snapshot(latest, this);
-      return s;
+      return replaceSelf(s).saveSelf2Snapshot(latest, this);
     }
   }
   /** Replace itself with an {@link INodeDirectorySnapshottable}. */
   public INodeDirectorySnapshottable replaceSelf4INodeDirectorySnapshottable(
       Snapshot latest) {
+    Preconditions.checkState(!(this instanceof INodeDirectorySnapshottable),
+        "this is already an INodeDirectorySnapshottable, this=%s", this);
     final INodeDirectorySnapshottable s = new INodeDirectorySnapshottable(this);
-    replaceSelf(s);
-    s.saveSelf2Snapshot(latest, this);
+    replaceSelf(s).saveSelf2Snapshot(latest, this);
     return s;
   }
 
   /** Replace itself with an {@link INodeDirectoryWithSnapshot}. */
-  public INodeDirectoryWithSnapshot replaceSelf4INodeDirectoryWithSnapshot(
-      Snapshot latest) {
+  public INodeDirectoryWithSnapshot replaceSelf4INodeDirectoryWithSnapshot() {
     Preconditions.checkState(!(this instanceof INodeDirectoryWithSnapshot),
         "this is already an INodeDirectoryWithSnapshot, this=%s", this);
-
-    final INodeDirectoryWithSnapshot withSnapshot
-        = INodeDirectoryWithSnapshot.newInstance(this, latest);
-    replaceSelf(withSnapshot);
-    return withSnapshot;
+    return replaceSelf(new INodeDirectoryWithSnapshot(this));
   }
 
   /** Replace itself with {@link INodeDirectory}. */
   public INodeDirectory replaceSelf4INodeDirectory() {
     Preconditions.checkState(getClass() != INodeDirectory.class,
         "the class is already INodeDirectory, this=%s", this);
-
-    final INodeDirectory newNode = new INodeDirectory(this, true);
-    replaceSelf(newNode);
-    return newNode;
+    return replaceSelf(new INodeDirectory(this, true));
   }
 
   /** Replace itself with the given directory. */
-  private final void replaceSelf(INodeDirectory newDir) {
+  private final <N extends INodeDirectory> N replaceSelf(final N newDir) {
     final INodeDirectory parent = getParent();
     Preconditions.checkArgument(parent != null, "parent is null, this=%s", this);
+    return parent.replaceChild(newDir);
+  }
 
-    final int i = parent.searchChildrenForExistingINode(newDir);
-    final INode oldDir = parent.children.set(i, newDir);
-    oldDir.setParent(null);
+  private final <N extends INode> N replaceChild(final N newChild) {
+    assertChildrenNonNull();
+    final int i = searchChildrenForExistingINode(newChild);
+    final INode oldChild = children.set(i, newChild);
+    oldChild.clearReferences();
+    return newChild;
   }
 
   /** Replace a child {@link INodeFile} with an {@link INodeFileWithSnapshot}. */
-  INodeFileWithSnapshot replaceChild4INodeFileWithSnapshot(final INodeFile child) {
-    assertChildrenNonNull();
+  INodeFileWithSnapshot replaceChild4INodeFileWithSnapshot(
+      final INodeFile child) {
     Preconditions.checkArgument(!(child instanceof INodeFileWithSnapshot),
-        "Child file is already an INodeFileWithLink, child=" + child);
+        "Child file is already an INodeFileWithSnapshot, child=" + child);
+    return replaceChild(new INodeFileWithSnapshot(child, null));
+  }
 
-    final INodeFileWithSnapshot newChild = new INodeFileWithSnapshot(child);
-    final int i = searchChildrenForExistingINode(newChild);
-    children.set(i, newChild);
-    return newChild;
+  /** Replace a child {@link INodeFile} with an {@link INodeFileUnderConstructionWithSnapshot}. */
+  INodeFileUnderConstructionWithSnapshot replaceChild4INodeFileUcWithSnapshot(
+      final INodeFileUnderConstruction child) {
+    Preconditions.checkArgument(!(child instanceof INodeFileUnderConstructionWithSnapshot),
+        "Child file is already an INodeFileUnderConstructionWithSnapshot, child=" + child);
+    return replaceChild(new INodeFileUnderConstructionWithSnapshot(child));
   }
 
   @Override
   public INodeDirectory recordModification(Snapshot latest) {
-    if (latest == null) {
-      return this;
-    }
-    final INodeDirectoryWithSnapshot withSnapshot
-        = replaceSelf4INodeDirectoryWithSnapshot(latest);
-    withSnapshot.saveSelf2Snapshot(latest, this);
-    return withSnapshot;
+    return latest == null? this
+        : replaceSelf4INodeDirectoryWithSnapshot().recordModification(latest);
   }
 
   /**
@@ -240,12 +234,13 @@ public class INodeDirectory extends INod
    * 
    * @return the child inode, which may be replaced.
    */
-  public INode saveChild2Snapshot(INode child, Snapshot latest) {
+  public INode saveChild2Snapshot(final INode child, final Snapshot latest,
+      final INode snapshotCopy) {
     if (latest == null) {
       return child;
     }
-    return replaceSelf4INodeDirectoryWithSnapshot(latest)
-        .saveChild2Snapshot(child, latest);
+    return replaceSelf4INodeDirectoryWithSnapshot()
+        .saveChild2Snapshot(child, latest, snapshotCopy);
   }
 
   /**
@@ -471,8 +466,7 @@ public class INodeDirectory extends INod
   public boolean addChild(final INode node, final boolean setModTime,
       final Snapshot latest) {
     if (latest != null) {
-      final INodeDirectoryWithSnapshot dir = replaceSelf4INodeDirectoryWithSnapshot(latest);
-      return dir.addChild(node, setModTime, latest);
+      return recordModification(latest).addChild(node, setModTime, latest);
     }
 
     if (children == null) {
@@ -581,15 +575,28 @@ public class INodeDirectory extends INod
   }
 
   @Override
-  public int destroySubtreeAndCollectBlocks(final Snapshot snapshot,
+  public void clearReferences() {
+    super.clearReferences();
+    setChildren(null);
+  }
+
+  public int destroySubtreeAndCollectBlocksRecursively(final Snapshot snapshot,
       final BlocksMapUpdateInfo collectedBlocks) {
     int total = 0;
     for (INode child : getChildrenList(snapshot)) {
       total += child.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks);
     }
+    return total;
+  }
+
+  @Override
+  public int destroySubtreeAndCollectBlocks(final Snapshot snapshot,
+      final BlocksMapUpdateInfo collectedBlocks) {
+    int total = destroySubtreeAndCollectBlocksRecursively(
+        snapshot, collectedBlocks);
     if (snapshot == null) {
-      parent = null;
-      children = null;
+      total++; //count this dir only if this object is destroyed  
+      clearReferences();
     }
     return total;
   }
@@ -691,9 +698,12 @@ public class INodeDirectory extends INod
       return inodes;
     }
     
-    /** @return the i-th inode. */
+    /**
+     * @return the i-th inode if i >= 0;
+     *         otherwise, i < 0, return the (length + i)-th inode.
+     */
     public INode getINode(int i) {
-      return inodes[i];
+      return inodes[i >= 0? i: inodes.length + i];
     }
     
     /** @return the last inode. */

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectoryWithQuota.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectoryWithQuota.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectoryWithQuota.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectoryWithQuota.java Fri Feb  8 02:18:55 2013
@@ -43,7 +43,7 @@ public class INodeDirectoryWithQuota ext
    * @param dsQuota Diskspace quota to be assigned to this indoe
    * @param other The other inode from which all other properties are copied
    */
-  protected INodeDirectoryWithQuota(INodeDirectory other, boolean adopt,
+  public INodeDirectoryWithQuota(INodeDirectory other, boolean adopt,
       long nsQuota, long dsQuota) {
     super(other, adopt);
     INode.DirCounts counts = new INode.DirCounts();

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java Fri Feb  8 02:18:55 2013
@@ -116,23 +116,12 @@ public class INodeFile extends INode imp
     this.blocks = blklist;
   }
   
-  protected INodeFile(INodeFile that) {
+  public INodeFile(INodeFile that) {
     super(that);
     this.header = that.header;
     this.blocks = that.blocks;
   }
 
-  @Override
-  INodeFile recordModification(final Snapshot latest) {
-    //TODO: change it to use diff list
-    return (INodeFile)super.recordModification(latest);
-  }
-
-  @Override
-  public Pair<? extends INodeFile, ? extends INodeFile> createSnapshotCopy() {
-    return parent.replaceChild4INodeFileWithSnapshot(this).createSnapshotCopy();
-  }
-
   /** @return true unconditionally. */
   @Override
   public final boolean isFile() {
@@ -150,24 +139,36 @@ public class INodeFile extends INode imp
         clientName, clientMachine, clientNode); 
   }
 
+  @Override
+  public INodeFile recordModification(final Snapshot latest) {
+    return latest == null? this
+        : parent.replaceChild4INodeFileWithSnapshot(this)
+            .recordModification(latest);
+  }
+
   /**
    * Set the {@link FsPermission} of this {@link INodeFile}.
    * Since this is a file,
    * the {@link FsAction#EXECUTE} action, if any, is ignored.
    */
   @Override
-  INode setPermission(FsPermission permission, Snapshot latest) {
+  final INode setPermission(FsPermission permission, Snapshot latest) {
     return super.setPermission(permission.applyUMask(UMASK), latest);
   }
 
   /** @return the replication factor of the file. */
-  public final short getFileReplication() {
+  public short getFileReplication(Snapshot snapshot) {
     return HeaderFormat.getReplication(header);
   }
 
+  /** The same as getFileReplication(null). */
+  public final short getFileReplication() {
+    return getFileReplication(null);
+  }
+
   @Override
   public short getBlockReplication() {
-    return getFileReplication();
+    return getFileReplication(null);
   }
 
   public void setFileReplication(short replication, Snapshot latest) {
@@ -242,7 +243,6 @@ public class INodeFile extends INode imp
       return 0;
     }
 
-    parent = null;
     if (blocks != null && collectedBlocks != null) {
       for (BlockInfo blk : blocks) {
         collectedBlocks.addDeleteBlock(blk);
@@ -250,6 +250,7 @@ public class INodeFile extends INode imp
       }
     }
     setBlocks(null);
+    clearReferences();
     return 1;
   }
   
@@ -262,16 +263,22 @@ public class INodeFile extends INode imp
 
   @Override
   long[] computeContentSummary(long[] summary) {
-    summary[0] += computeFileSize(true);
+    summary[0] += computeFileSize(true, null);
     summary[1]++;
     summary[3] += diskspaceConsumed();
     return summary;
   }
 
+  /** The same as computeFileSize(includesBlockInfoUnderConstruction, null). */
+  public long computeFileSize(boolean includesBlockInfoUnderConstruction) {
+    return computeFileSize(includesBlockInfoUnderConstruction, null);
+  }
+
   /** Compute file size.
    * May or may not include BlockInfoUnderConstruction.
    */
-  public long computeFileSize(boolean includesBlockInfoUnderConstruction) {
+  public long computeFileSize(boolean includesBlockInfoUnderConstruction,
+      Snapshot snapshot) {
     if (blocks == null || blocks.length == 0) {
       return 0;
     }
@@ -343,7 +350,7 @@ public class INodeFile extends INode imp
   public void dumpTreeRecursively(PrintWriter out, StringBuilder prefix,
       final Snapshot snapshot) {
     super.dumpTreeRecursively(out, prefix, snapshot);
-    out.print(", fileSize=" + computeFileSize(true));
+    out.print(", fileSize=" + computeFileSize(true, snapshot));
     // only compare the first block
     out.print(", blocks=" + (blocks == null? null: blocks[0]));
     if (this instanceof FileWithSnapshot) {

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFileUnderConstruction.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFileUnderConstruction.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFileUnderConstruction.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFileUnderConstruction.java Fri Feb  8 02:18:55 2013
@@ -29,6 +29,7 @@ import org.apache.hadoop.hdfs.server.blo
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor;
 import org.apache.hadoop.hdfs.server.blockmanagement.MutableBlockCollection;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
 
 import com.google.common.base.Preconditions;
 
@@ -80,7 +81,7 @@ public class INodeFileUnderConstruction 
     this.clientNode = clientNode;
   }
   
-  protected INodeFileUnderConstruction(final INodeFile that,
+  public INodeFileUnderConstruction(final INodeFile that,
       final String clientName,
       final String clientMachine,
       final DatanodeDescriptor clientNode) {
@@ -127,6 +128,13 @@ public class INodeFileUnderConstruction 
         getBlocks(), getFileReplication(), getPreferredBlockSize());
   }
   
+  @Override
+  public INodeFileUnderConstruction recordModification(final Snapshot latest) {
+    return latest == null? this
+        : parent.replaceChild4INodeFileUcWithSnapshot(this)
+            .recordModification(latest);
+  }
+
   /** Assert all blocks are complete. */
   protected void assertAllBlocksComplete() {
     final BlockInfo[] blocks = getBlocks();

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeSymlink.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeSymlink.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeSymlink.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeSymlink.java Fri Feb  8 02:18:55 2013
@@ -46,8 +46,8 @@ public class INodeSymlink extends INode 
   }
 
   @Override
-  public Pair<INodeSymlink, INodeSymlink> createSnapshotCopy() {
-    return new Pair<INodeSymlink, INodeSymlink>(this, new INodeSymlink(this));
+  INode recordModification(Snapshot latest) {
+    return parent.saveChild2Snapshot(this, latest, new INodeSymlink(this));
   }
 
   /** @return true unconditionally. */

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java Fri Feb  8 02:18:55 2013
@@ -85,33 +85,38 @@ abstract class AbstractINodeDiff<N exten
   }
 
   /** Copy the INode state to the snapshot if it is not done already. */
-  void checkAndInitINode(N snapshotCopy) {
+  void checkAndInitINode(N currentINode, N snapshotCopy) {
     if (snapshotINode == null) {
       if (snapshotCopy == null) {
-        @SuppressWarnings("unchecked")
-        final N right = (N)getCurrentINode().createSnapshotCopy().right;
-        snapshotCopy = right;
+        snapshotCopy = createSnapshotCopyOfCurrentINode(currentINode);
       }
       snapshotINode = snapshotCopy;
     }
   }
 
-  /** @return the current inode. */
-  abstract N getCurrentINode();
+  /** @return a snapshot copy of the current inode. */
+  abstract N createSnapshotCopyOfCurrentINode(N currentINode);
 
   /** @return the inode corresponding to the snapshot. */
   N getSnapshotINode() {
-    // get from this diff, then the posterior diff and then the current inode
+    // get from this diff, then the posterior diff
+    // and then null for the current inode
     for(AbstractINodeDiff<N, D> d = this; ; d = d.posteriorDiff) {
       if (d.snapshotINode != null) {
         return d.snapshotINode;
       } else if (d.posteriorDiff == null) {
-        return getCurrentINode();
+        return null;
       }
     }
   }
 
   /** Combine the posterior diff and collect blocks for deletion. */
-  abstract void combinePosteriorAndCollectBlocks(final D posterior,
-      final BlocksMapUpdateInfo collectedBlocks);
+  abstract void combinePosteriorAndCollectBlocks(final N currentINode,
+      final D posterior, final BlocksMapUpdateInfo collectedBlocks);
+
+  @Override
+  public String toString() {
+    return getClass().getSimpleName() + ": " + snapshot + " (post="
+        + (posteriorDiff == null? null: posteriorDiff.snapshot) + ")";
+  }
 }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java Fri Feb  8 02:18:55 2013
@@ -37,6 +37,12 @@ abstract class AbstractINodeDiffList<N e
   /** Diff list sorted by snapshot IDs, i.e. in chronological order. */
   private final List<D> diffs = new ArrayList<D>();
 
+  AbstractINodeDiffList(final List<D> diffs) {
+    if (diffs != null) {
+      this.diffs.addAll(diffs);
+    }
+  }
+
   /** @return this list as a unmodifiable {@link List}. */
   final List<D> asList() {
     return Collections.unmodifiableList(diffs);
@@ -46,7 +52,7 @@ abstract class AbstractINodeDiffList<N e
   abstract N getCurrentINode();
   
   /** Add a {@link AbstractINodeDiff} for the given snapshot and inode. */
-  abstract D addSnapshotDiff(Snapshot snapshot, N inode, boolean isSnapshotCreation); 
+  abstract D addSnapshotDiff(Snapshot snapshot); 
 
   /**
    * Delete the snapshot with the given name. The synchronization of the diff
@@ -67,14 +73,19 @@ abstract class AbstractINodeDiffList<N e
       return null;
     } else {
       final D removed = diffs.remove(snapshotIndex);
-      if (snapshotIndex > 0) {
+      if (snapshotIndex == 0) {
+        if (removed.snapshotINode != null) {
+          removed.snapshotINode.clearReferences();
+        }
+      } else {
         // combine the to-be-removed diff with its previous diff
         final AbstractINodeDiff<N, D> previous = diffs.get(snapshotIndex - 1);
         if (previous.snapshotINode == null) {
-          // TODO: add a new testcase for this
           previous.snapshotINode = removed.snapshotINode;
+        } else if (removed.snapshotINode != null) {
+          removed.snapshotINode.clearReferences();
         }
-        previous.combinePosteriorAndCollectBlocks(removed, collectedBlocks);
+        previous.combinePosteriorAndCollectBlocks(getCurrentINode(), removed, collectedBlocks);
         previous.setPosterior(removed.getPosterior());
       }
       removed.setPosterior(null);
@@ -83,8 +94,8 @@ abstract class AbstractINodeDiffList<N e
   }
 
   /** Append the diff at the end of the list. */
-  final D append(D diff) {
-    final AbstractINodeDiff<N, D> last = getLast();
+  final D addLast(D diff) {
+    final D last = getLast();
     diffs.add(diff);
     if (last != null) {
       last.setPosterior(diff);
@@ -92,11 +103,13 @@ abstract class AbstractINodeDiffList<N e
     return diff;
   }
   
-  /** Insert the diff to the beginning of the list. */
-  final void insert(D diff) {
+  /** Add the diff to the beginning of the list. */
+  final void addFirst(D diff) {
+    final D first = diffs.isEmpty()? null: diffs.get(0);
     diffs.add(0, diff);
+    diff.setPosterior(first);
   }
-  
+
   /** @return the last diff. */
   final D getLast() {
     final int n = diffs.size();
@@ -131,7 +144,12 @@ abstract class AbstractINodeDiffList<N e
       return j < diffs.size()? diffs.get(j): null;
     }
   }
-  
+
+  N getSnapshotINode(Snapshot snapshot) {
+    final D diff = getDiff(snapshot);
+    return diff == null? null: diff.getSnapshotINode();
+  }
+
   /**
    * Check if the latest snapshot diff exists.  If not, add it.
    * @return the latest snapshot diff, which is never null.
@@ -139,7 +157,15 @@ abstract class AbstractINodeDiffList<N e
   final D checkAndAddLatestSnapshotDiff(Snapshot latest) {
     final D last = getLast();
     return last != null && last.snapshot.equals(latest)? last
-        : addSnapshotDiff(latest, getCurrentINode(), false);
+        : addSnapshotDiff(latest);
+  }
+
+  /** Save the snapshot copy to the latest snapshot. */
+  void saveSelf2Snapshot(Snapshot latest, N snapshotCopy) {
+    if (latest != null) {
+      checkAndAddLatestSnapshotDiff(latest).checkAndInitINode(
+          getCurrentINode(), snapshotCopy);
+    }
   }
   
   @Override
@@ -149,6 +175,6 @@ abstract class AbstractINodeDiffList<N e
 
   @Override
   public String toString() {
-    return "diffs=" + diffs;
+    return getClass().getSimpleName() + ": " + diffs;
   }
 }
\ No newline at end of file

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshot.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshot.java Fri Feb  8 02:18:55 2013
@@ -19,11 +19,9 @@ package org.apache.hadoop.hdfs.server.na
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo;
-import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapINodeUpdateEntry;
 import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo;
-
-import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 
 /**
  * {@link INodeFile} with a link to the next element.
@@ -32,6 +30,38 @@ import com.google.common.base.Preconditi
  */
 @InterfaceAudience.Private
 public interface FileWithSnapshot {
+  /**
+   * The difference of an {@link INodeFile} between two snapshots.
+   */
+  static class FileDiff extends AbstractINodeDiff<INodeFile, FileDiff> {
+    /** The file size at snapshot creation time. */
+    final long fileSize;
+
+    FileDiff(Snapshot snapshot, INodeFile file) {
+      super(snapshot, null, null);
+      fileSize = file.computeFileSize(true, null);
+    }
+
+    @Override
+    INodeFile createSnapshotCopyOfCurrentINode(INodeFile currentINode) {
+      final INodeFile copy = new INodeFile(currentINode);
+      copy.setBlocks(null);
+      return copy;
+    }
+
+    @Override
+    void combinePosteriorAndCollectBlocks(INodeFile currentINode,
+        FileDiff posterior, BlocksMapUpdateInfo collectedBlocks) {
+      Util.collectBlocksAndClear((FileWithSnapshot)currentINode, collectedBlocks);
+    }
+    
+    @Override
+    public String toString() {
+      return super.toString() + " fileSize=" + fileSize + ", rep="
+          + (snapshotINode == null? "?": snapshotINode.getFileReplication());
+    }
+  }
+
   /** @return the {@link INodeFile} view of this object. */
   public INodeFile asINodeFile();
   
@@ -49,10 +79,21 @@ public interface FileWithSnapshot {
   
   /** Remove self from the circular list */
   public void removeSelf();
+
+  /** Is the current file deleted? */
+  public boolean isCurrentFileDeleted();
+
+  /** Are the current file and all snapshot copies deleted? */
+  public boolean isEverythingDeleted();
+
+  /** @return the max file replication in the inode and its snapshot copies. */
+  public short getMaxFileReplication();
   
-  /** Utility methods for the classes which implement the interface. */
-  static class Util {
+  /** @return the max file size in the inode and its snapshot copies. */
+  public long computeMaxFileSize();
 
+  /** Utility methods for the classes which implement the interface. */
+  public static class Util {
     /** @return The previous node in the circular linked list */
     static FileWithSnapshot getPrevious(FileWithSnapshot file) {
       FileWithSnapshot previous = file.getNext();
@@ -76,17 +117,32 @@ public interface FileWithSnapshot {
       }
     }
 
+    /** @return the max file replication of the file in the diff list. */
+    static <N extends INodeFile, D extends AbstractINodeDiff<N, D>>
+        short getMaxFileReplication(short max,
+              final AbstractINodeDiffList<N, D> diffs) {
+      for(AbstractINodeDiff<N, D> d : diffs) {
+        if (d.snapshotINode != null) {
+          final short replication = d.snapshotINode.getFileReplication();
+          if (replication > max) {
+            max = replication;
+          }
+        }
+      }
+      return max;
+    }
+
     /**
      * @return the max file replication of the elements
      *         in the circular linked list.
      */
     static short getBlockReplication(final FileWithSnapshot file) {
-      short max = file.asINodeFile().getFileReplication();
+      short max = file.getMaxFileReplication();
       // i may be null since next will be set to null when the INode is deleted
       for(FileWithSnapshot i = file.getNext();
           i != file && i != null;
           i = i.getNext()) {
-        final short replication = i.asINodeFile().getFileReplication();
+        final short replication = i.getMaxFileReplication();
         if (replication > max) {
           max = replication;
         }
@@ -95,49 +151,54 @@ public interface FileWithSnapshot {
     }
 
     /**
-     * Remove the current inode from the circular linked list.
      * If some blocks at the end of the block list no longer belongs to
-     * any other inode, collect them and update the block list.
+     * any inode, collect them and update the block list.
      */
-    static int collectSubtreeBlocksAndClear(final FileWithSnapshot file,
+    static void collectBlocksAndClear(final FileWithSnapshot file,
         final BlocksMapUpdateInfo info) {
       final FileWithSnapshot next = file.getNext();
-      Preconditions.checkState(next != file, "this is the only remaining inode.");
 
-      // There are other inode(s) using the blocks.
-      // Compute max file size excluding this and find the last inode.
-      long max = next.asINodeFile().computeFileSize(true);
-      short maxReplication = next.asINodeFile().getFileReplication();
-      FileWithSnapshot last = next;
-      for(FileWithSnapshot i = next.getNext(); i != file; i = i.getNext()) {
-        final long size = i.asINodeFile().computeFileSize(true);
-        if (size > max) {
-          max = size;
-        }
-        final short rep = i.asINodeFile().getFileReplication();
-        if (rep > maxReplication) {
-          maxReplication = rep;
-        }
-        last = i;
-      }
-
-      collectBlocksBeyondMaxAndClear(file, max, info);
-      
-      // remove this from the circular linked list.
-      last.setNext(next);
-      // Set the replication of the current INode to the max of all the other
-      // linked INodes, so that in case the current INode is retrieved from the
-      // blocksMap before it is removed or updated, the correct replication
-      // number can be retrieved.
-      file.asINodeFile().setFileReplication(maxReplication, null);
-      file.setNext(null);
-      // clear parent
-      file.asINodeFile().setParent(null);
-      return 1;
+      // find max file size, max replication and the last inode.
+      long maxFileSize = file.computeMaxFileSize();
+      short maxReplication = file.getMaxFileReplication();
+      FileWithSnapshot last = null;
+      if (next != null && next != file) {
+        for(FileWithSnapshot i = next; i != file; i = i.getNext()) {
+          final long size = i.computeMaxFileSize();
+          if (size > maxFileSize) {
+            maxFileSize = size;
+          }
+          final short rep = i.getMaxFileReplication();
+          if (rep > maxReplication) {
+            maxReplication = rep;
+          }
+          last = i;
+        }
+      }
+
+      collectBlocksBeyondMax(file, maxFileSize, info);
+
+      if (file.isEverythingDeleted()) {
+        // Set the replication of the current INode to the max of all the other
+        // linked INodes, so that in case the current INode is retrieved from the
+        // blocksMap before it is removed or updated, the correct replication
+        // number can be retrieved.
+        if (maxReplication > 0) {
+          file.asINodeFile().setFileReplication(maxReplication, null);
+        }
+
+        // remove the file from the circular linked list.
+        if (last != null) {
+          last.setNext(next);
+        }
+        file.setNext(null);
+
+        file.asINodeFile().setBlocks(null);
+      }
     }
 
-    static void collectBlocksBeyondMaxAndClear(final FileWithSnapshot file,
-            final long max, final BlocksMapUpdateInfo info) {
+    private static void collectBlocksBeyondMax(final FileWithSnapshot file,
+        final long max, final BlocksMapUpdateInfo collectedBlocks) {
       final BlockInfo[] oldBlocks = file.asINodeFile().getBlocks();
       if (oldBlocks != null) {
         //find the minimum n such that the size of the first n blocks > max
@@ -146,13 +207,13 @@ public interface FileWithSnapshot {
           size += oldBlocks[n].getNumBytes();
         }
 
-        // Replace the INode for all the remaining blocks in blocksMap
+        // collect update blocks
         final FileWithSnapshot next = file.getNext();
-        final BlocksMapINodeUpdateEntry entry = new BlocksMapINodeUpdateEntry(
-            file.asINodeFile(), next.asINodeFile());
-        if (info != null) {
+        if (next != null && next != file && file.isEverythingDeleted() && collectedBlocks != null) {
+          final BlocksMapINodeUpdateEntry entry = new BlocksMapINodeUpdateEntry(
+              file.asINodeFile(), next.asINodeFile());
           for (int i = 0; i < n; i++) {
-            info.addUpdateBlock(oldBlocks[i], entry);
+            collectedBlocks.addUpdateBlock(oldBlocks[i], entry);
           }
         }
         
@@ -166,19 +227,31 @@ public interface FileWithSnapshot {
             newBlocks = new BlockInfo[n];
             System.arraycopy(oldBlocks, 0, newBlocks, 0, n);
           }
-          for(FileWithSnapshot i = next; i != file; i = i.getNext()) {
+          
+          // set new blocks
+          file.asINodeFile().setBlocks(newBlocks);
+          for(FileWithSnapshot i = next; i != null && i != file; i = i.getNext()) {
             i.asINodeFile().setBlocks(newBlocks);
           }
 
           // collect the blocks beyond max.  
-          if (info != null) {
+          if (collectedBlocks != null) {
             for(; n < oldBlocks.length; n++) {
-              info.addDeleteBlock(oldBlocks[n]);
+              collectedBlocks.addDeleteBlock(oldBlocks[n]);
             }
           }
         }
-        file.asINodeFile().setBlocks(null);
       }
     }
+    
+    static String circularListString(final FileWithSnapshot file) {
+      final StringBuilder b = new StringBuilder("* -> ")
+          .append(file.asINodeFile().getObjectString());
+      FileWithSnapshot n = file.getNext();
+      for(; n != null && n != file; n = n.getNext()) {
+        b.append(" -> ").append(n.asINodeFile().getObjectString());
+      }
+      return b.append(n == null? " -> null": " -> *").toString();
+    }
   }
 }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java Fri Feb  8 02:18:55 2013
@@ -264,13 +264,13 @@ public class INodeDirectorySnapshottable
           + "snapshot with the same name \"" + name + "\".");
     }
 
-    getDiffs().addSnapshotDiff(s, this, true);
+    final DirectoryDiff d = getDiffs().addSnapshotDiff(s);
+    d.snapshotINode = s.getRoot();
     snapshotsByNames.add(-i - 1, s);
 
     //set modification time
-    final long timestamp = Time.now();
-    s.getRoot().updateModificationTime(timestamp, null);
-    updateModificationTime(timestamp, null);
+    updateModificationTime(Time.now(), null);
+    s.getRoot().setModificationTime(getModificationTime(), null);
     return s;
   }
   
@@ -381,7 +381,7 @@ public class INodeDirectorySnapshottable
           "latest == null but getLastSnapshot() != null, this=%s", this);
       replaceSelf4INodeDirectory();
     } else {
-      replaceSelf4INodeDirectoryWithSnapshot(latest).recordModification(latest);
+      replaceSelf4INodeDirectoryWithSnapshot().recordModification(latest);
     }
   }
 

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java Fri Feb  8 02:18:55 2013
@@ -174,7 +174,7 @@ public class INodeDirectoryWithSnapshot 
   /**
    * The difference of an {@link INodeDirectory} between two snapshots.
    */
-  class DirectoryDiff extends AbstractINodeDiff<INodeDirectory, DirectoryDiff> {
+  static class DirectoryDiff extends AbstractINodeDiff<INodeDirectory, DirectoryDiff> {
     /** The size of the children list at snapshot creation time. */
     private final int childrenSize;
     /** The children list diff. */
@@ -206,13 +206,18 @@ public class INodeDirectoryWithSnapshot 
     }
 
     @Override
-    INodeDirectory getCurrentINode() {
-      return INodeDirectoryWithSnapshot.this;
+    INodeDirectory createSnapshotCopyOfCurrentINode(INodeDirectory currentDir) {
+      final INodeDirectory copy = currentDir instanceof INodeDirectoryWithQuota?
+          new INodeDirectoryWithQuota(currentDir, false,
+              currentDir.getNsQuota(), currentDir.getDsQuota())
+        : new INodeDirectory(currentDir, false);
+      copy.setChildren(null);
+      return copy;
     }
 
     @Override
-    void combinePosteriorAndCollectBlocks(final DirectoryDiff posterior,
-        final BlocksMapUpdateInfo collectedBlocks) {
+    void combinePosteriorAndCollectBlocks(final INodeDirectory currentDir,
+        final DirectoryDiff posterior, final BlocksMapUpdateInfo collectedBlocks) {
       diff.combinePosterior(posterior.diff, new Diff.Processor<INode>() {
         /** Collect blocks for deleted files. */
         @Override
@@ -230,7 +235,7 @@ public class INodeDirectoryWithSnapshot 
      *         Since the snapshot is read-only, the logical view of the list is
      *         never changed although the internal data structure may mutate.
      */
-    ReadOnlyList<INode> getChildrenList() {
+    ReadOnlyList<INode> getChildrenList(final INodeDirectory currentDir) {
       return new ReadOnlyList<INode>() {
         private List<INode> children = null;
 
@@ -241,7 +246,7 @@ public class INodeDirectoryWithSnapshot 
               combined.combinePosterior(d.diff, null);
             }
             children = combined.apply2Current(ReadOnlyList.Util.asList(
-                getCurrentINode().getChildrenList(null)));
+                currentDir.getChildrenList(null)));
           }
           return children;
         }
@@ -269,7 +274,7 @@ public class INodeDirectoryWithSnapshot 
     }
 
     /** @return the child with the given name. */
-    INode getChild(byte[] name, boolean checkPosterior) {
+    INode getChild(byte[] name, boolean checkPosterior, INodeDirectory currentDir) {
       for(DirectoryDiff d = this; ; d = d.getPosterior()) {
         final Container<INode> returned = d.diff.accessPrevious(name);
         if (returned != null) {
@@ -280,17 +285,14 @@ public class INodeDirectoryWithSnapshot 
           return null;
         } else if (d.getPosterior() == null) {
           // no more posterior diff, get from current inode.
-          return getCurrentINode().getChild(name, null);
+          return currentDir.getChild(name, null);
         }
       }
     }
     
     @Override
     public String toString() {
-      final DirectoryDiff posterior = getPosterior();
-      return "\n  " + snapshot + " (-> "
-          + (posterior == null? null: posterior.snapshot)
-          + ") childrenSize=" + childrenSize + ", " + diff;
+      return super.toString() + " childrenSize=" + childrenSize + ", " + diff;
     }
     
     /** Serialize fields to out */
@@ -323,35 +325,21 @@ public class INodeDirectoryWithSnapshot 
   /** A list of directory diffs. */
   class DirectoryDiffList extends
       AbstractINodeDiffList<INodeDirectory, DirectoryDiff> {
+    DirectoryDiffList(List<DirectoryDiff> diffs) {
+      super(diffs);
+    }
+
     @Override
     INodeDirectoryWithSnapshot getCurrentINode() {
       return INodeDirectoryWithSnapshot.this;
     }
 
     @Override
-    DirectoryDiff addSnapshotDiff(Snapshot snapshot, INodeDirectory dir,
-        boolean isSnapshotCreation) {
-      final DirectoryDiff d = new DirectoryDiff(snapshot, dir); 
-      if (isSnapshotCreation) {
-        //for snapshot creation, snapshotINode is the same as the snapshot root
-        d.snapshotINode = snapshot.getRoot();
-      }
-      return append(d);
+    DirectoryDiff addSnapshotDiff(Snapshot snapshot) {
+      return addLast(new DirectoryDiff(snapshot, getCurrentINode()));
     }
   }
 
-  /** Create an {@link INodeDirectoryWithSnapshot} with the given snapshot.*/
-  public static INodeDirectoryWithSnapshot newInstance(INodeDirectory dir,
-      Snapshot latest) {
-    final INodeDirectoryWithSnapshot withSnapshot
-        = new INodeDirectoryWithSnapshot(dir, true, null);
-    if (latest != null) {
-      // add a diff for the latest snapshot
-      withSnapshot.diffs.addSnapshotDiff(latest, dir, false);
-    }
-    return withSnapshot;
-  }
-  
   /**
    * Compute the difference between Snapshots.
    * 
@@ -429,10 +417,15 @@ public class INodeDirectoryWithSnapshot 
   /** Diff list sorted by snapshot IDs, i.e. in chronological order. */
   private final DirectoryDiffList diffs;
 
+  public INodeDirectoryWithSnapshot(INodeDirectory that) {
+    this(that, true, that instanceof INodeDirectoryWithSnapshot?
+        ((INodeDirectoryWithSnapshot)that).getDiffs(): null);
+  }
+
   INodeDirectoryWithSnapshot(INodeDirectory that, boolean adopt,
       DirectoryDiffList diffs) {
     super(that, adopt, that.getNsQuota(), that.getDsQuota());
-    this.diffs = diffs != null? diffs: new DirectoryDiffList();
+    this.diffs = new DirectoryDiffList(diffs == null? null: diffs.asList());
   }
 
   /** @return the last snapshot. */
@@ -446,26 +439,20 @@ public class INodeDirectoryWithSnapshot 
   }
 
   @Override
-  public Pair<INodeDirectoryWithSnapshot, INodeDirectory> createSnapshotCopy() {
-    return new Pair<INodeDirectoryWithSnapshot, INodeDirectory>(this,
-        new INodeDirectory(this, false));
-  }
-
-  @Override
   public INodeDirectoryWithSnapshot recordModification(Snapshot latest) {
-    saveSelf2Snapshot(latest, null);
-    return this;
+    return saveSelf2Snapshot(latest, null);
   }
 
   /** Save the snapshot copy to the latest snapshot. */
-  public void saveSelf2Snapshot(Snapshot latest, INodeDirectory snapshotCopy) {
-    if (latest != null) {
-      diffs.checkAndAddLatestSnapshotDiff(latest).checkAndInitINode(snapshotCopy);
-    }
+  public INodeDirectoryWithSnapshot saveSelf2Snapshot(
+      final Snapshot latest, final INodeDirectory snapshotCopy) {
+    diffs.saveSelf2Snapshot(latest, snapshotCopy);
+    return this;
   }
 
   @Override
-  public INode saveChild2Snapshot(INode child, Snapshot latest) {
+  public INode saveChild2Snapshot(final INode child, final Snapshot latest,
+      final INode snapshotCopy) {
     Preconditions.checkArgument(!child.isDirectory(),
         "child is a directory, child=%s", child);
     if (latest == null) {
@@ -473,23 +460,13 @@ public class INodeDirectoryWithSnapshot 
     }
 
     final DirectoryDiff diff = diffs.checkAndAddLatestSnapshotDiff(latest);
-    if (diff.getChild(child.getLocalNameBytes(), false) != null) {
+    if (diff.getChild(child.getLocalNameBytes(), false, this) != null) {
       // it was already saved in the latest snapshot earlier.  
       return child;
     }
 
-    final Pair<? extends INode, ? extends INode> p = child.createSnapshotCopy();
-    if (p.left != p.right) {
-      final UndoInfo<INode> undoIndo = diff.diff.modify(p.right, p.left);
-      if (undoIndo.getTrashedElement() != null && p.left instanceof FileWithSnapshot) {
-        // also should remove oldinode from the circular list
-        FileWithSnapshot newNodeWithLink = (FileWithSnapshot) p.left;
-        FileWithSnapshot oldNodeWithLink = (FileWithSnapshot) p.right;
-        newNodeWithLink.setNext(oldNodeWithLink.getNext());
-        oldNodeWithLink.setNext(null);
-      }
-    }
-    return p.left;
+    diff.diff.modify(snapshotCopy, child);
+    return child;
   }
 
   @Override
@@ -534,53 +511,49 @@ public class INodeDirectoryWithSnapshot 
   @Override
   public ReadOnlyList<INode> getChildrenList(Snapshot snapshot) {
     final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getChildrenList(): super.getChildrenList(null);
+    return diff != null? diff.getChildrenList(this): super.getChildrenList(null);
   }
 
   @Override
   public INode getChild(byte[] name, Snapshot snapshot) {
     final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getChild(name, true): super.getChild(name, null);
+    return diff != null? diff.getChild(name, true, this): super.getChild(name, null);
   }
 
   @Override
   public String getUserName(Snapshot snapshot) {
-    final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getSnapshotINode().getUserName()
-        : super.getUserName(null);
+    final INodeDirectory inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getUserName(): super.getUserName(null);
   }
 
   @Override
   public String getGroupName(Snapshot snapshot) {
-    final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getSnapshotINode().getGroupName()
-        : super.getGroupName(null);
+    final INodeDirectory inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getGroupName(): super.getGroupName(null);
   }
 
   @Override
   public FsPermission getFsPermission(Snapshot snapshot) {
-    final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getSnapshotINode().getFsPermission()
-        : super.getFsPermission(null);
+    final INodeDirectory inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getFsPermission(): super.getFsPermission(null);
   }
 
   @Override
   public long getAccessTime(Snapshot snapshot) {
-    final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getSnapshotINode().getAccessTime()
-        : super.getAccessTime(null);
+    final INodeDirectory inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getAccessTime(): super.getAccessTime(null);
   }
 
   @Override
   public long getModificationTime(Snapshot snapshot) {
-    final DirectoryDiff diff = diffs.getDiff(snapshot);
-    return diff != null? diff.getSnapshotINode().getModificationTime()
+    final INodeDirectory inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getModificationTime()
         : super.getModificationTime(null);
   }
-  
+
   @Override
-  public String toString() {
-    return super.toString() + ", " + diffs;
+  public String toDetailString() {
+    return super.toDetailString() + ", " + diffs;
   }
   
   /**
@@ -607,9 +580,14 @@ public class INodeDirectoryWithSnapshot 
   @Override
   public int destroySubtreeAndCollectBlocks(final Snapshot snapshot,
       final BlocksMapUpdateInfo collectedBlocks) {
-    final int n = super.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks);
+    int n = destroySubtreeAndCollectBlocksRecursively(
+        snapshot, collectedBlocks);
     if (snapshot != null) {
-      getDiffs().deleteSnapshotDiff(snapshot, collectedBlocks);
+      final DirectoryDiff removed = getDiffs().deleteSnapshotDiff(snapshot,
+          collectedBlocks);
+      if (removed != null) {
+        n++; //count this dir only if a snapshot diff is removed.
+      }
     }
     return n;
   }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java Fri Feb  8 02:18:55 2013
@@ -17,10 +17,14 @@
  */
 package org.apache.hadoop.hdfs.server.namenode.snapshot;
 
+import java.util.List;
+
 import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor;
 import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 import org.apache.hadoop.hdfs.server.namenode.INodeFileUnderConstruction;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeFileWithSnapshot.FileDiffList;
 
 /**
  * Represent an {@link INodeFileUnderConstruction} that is snapshotted.
@@ -30,14 +34,49 @@ import org.apache.hadoop.hdfs.server.nam
 @InterfaceAudience.Private
 public class INodeFileUnderConstructionWithSnapshot
     extends INodeFileUnderConstruction implements FileWithSnapshot {
+  /**
+   * The difference of an {@link INodeFileUnderConstruction} between two snapshots.
+   */
+  static class FileUcDiff extends FileDiff {
+    private FileUcDiff(Snapshot snapshot, INodeFile file) {
+      super(snapshot, file);
+    }
+
+    @Override
+    INodeFileUnderConstruction createSnapshotCopyOfCurrentINode(INodeFile file) {
+      final INodeFileUnderConstruction uc = (INodeFileUnderConstruction)file;
+      final INodeFileUnderConstruction copy = new INodeFileUnderConstruction(
+          uc, uc.getClientName(), uc.getClientMachine(), uc.getClientNode());
+      copy.setBlocks(null);
+      return copy;
+    }
+  }
+
+  /**
+   * A list of file diffs.
+   */
+  static class FileUcDiffList extends FileDiffList {
+    private FileUcDiffList(INodeFile currentINode, final List<FileDiff> diffs) {
+      super(currentINode, diffs);
+    }
+
+    @Override
+    FileDiff addSnapshotDiff(Snapshot snapshot) {
+      return addLast(new FileUcDiff(snapshot, getCurrentINode()));
+    }
+  }
+
+  private final FileUcDiffList diffs;
   private FileWithSnapshot next;
 
   INodeFileUnderConstructionWithSnapshot(final INodeFile f,
       final String clientName,
       final String clientMachine,
-      final DatanodeDescriptor clientNode) {
+      final DatanodeDescriptor clientNode,
+      final FileDiffList diffs) {
     super(f, clientName, clientMachine, clientNode);
-    next = this;
+    this.diffs = new FileUcDiffList(this, diffs == null? null: diffs.asList());
+    setNext(this);
   }
 
   /**
@@ -47,14 +86,14 @@ public class INodeFileUnderConstructionW
    * @param f The given {@link INodeFileUnderConstruction} instance
    */
   public INodeFileUnderConstructionWithSnapshot(INodeFileUnderConstruction f) {
-    this(f, f.getClientName(), f.getClientMachine(), f.getClientNode());
+    this(f, f.getClientName(), f.getClientMachine(), f.getClientNode(), null);
   }
   
   @Override
   protected INodeFileWithSnapshot toINodeFile(final long mtime) {
     assertAllBlocksComplete();
     final long atime = getModificationTime();
-    final INodeFileWithSnapshot f = new INodeFileWithSnapshot(this);
+    final INodeFileWithSnapshot f = new INodeFileWithSnapshot(this, diffs);
     f.setModificationTime(mtime, null);
     f.setAccessTime(atime, null);
     // link f with this
@@ -63,11 +102,24 @@ public class INodeFileUnderConstructionW
   }
 
   @Override
-  public Pair<? extends INodeFileUnderConstruction,
-      INodeFileUnderConstructionSnapshot> createSnapshotCopy() {
-    return new Pair<INodeFileUnderConstructionWithSnapshot,
-        INodeFileUnderConstructionSnapshot>(
-            this, new INodeFileUnderConstructionSnapshot(this));
+  public boolean isCurrentFileDeleted() {
+    return getParent() == null;
+  }
+
+  @Override
+  public boolean isEverythingDeleted() {
+    return isCurrentFileDeleted() && diffs.asList().isEmpty();
+  }
+
+  @Override
+  public INodeFileUnderConstructionWithSnapshot recordModification(
+      final Snapshot latest) {
+    // if this object is NOT the latest snapshot copy, this object is created
+    // after the latest snapshot, then do NOT record modification.
+    if (this == getParent().getChild(getLocalNameBytes(), latest)) {
+      diffs.saveSelf2Snapshot(latest, null);
+    }
+    return this;
   }
 
   @Override
@@ -112,21 +164,91 @@ public class INodeFileUnderConstructionW
   }
 
   @Override
+  public short getFileReplication(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getFileReplication()
+        : super.getFileReplication(null);
+  }
+
+  @Override
+  public short getMaxFileReplication() {
+    final short max = isCurrentFileDeleted()? 0: getFileReplication();
+    return Util.getMaxFileReplication(max, diffs);
+  }
+
+  @Override
   public short getBlockReplication() {
     return Util.getBlockReplication(this);
   }
 
   @Override
+  public long computeFileSize(boolean includesBlockInfoUnderConstruction,
+      Snapshot snapshot) {
+    final FileDiff diff = diffs.getDiff(snapshot);
+    return diff != null? diff.fileSize
+        : super.computeFileSize(includesBlockInfoUnderConstruction, null);
+  }
+
+  @Override
+  public long computeMaxFileSize() {
+    if (isCurrentFileDeleted()) {
+      final FileDiff last = diffs.getLast();
+      return last == null? 0: last.fileSize;
+    } else { 
+      return super.computeFileSize(true, null);
+    }
+  }
+
+  @Override
   public int destroySubtreeAndCollectBlocks(final Snapshot snapshot,
       final BlocksMapUpdateInfo collectedBlocks) {
-    if (snapshot != null) {
-      return 0;
-    }
-    if (next == null || next == this) {
-      // this is the only remaining inode.
-      return super.destroySubtreeAndCollectBlocks(null, collectedBlocks);
+    if (snapshot == null) {
+      clearReferences();
     } else {
-      return Util.collectSubtreeBlocksAndClear(this, collectedBlocks);
+      if (diffs.deleteSnapshotDiff(snapshot, collectedBlocks) == null) {
+        //snapshot diff not found and nothing is deleted.
+        return 0;
+      }
     }
+
+    Util.collectBlocksAndClear(this, collectedBlocks);
+    return 1;
+  }
+
+  @Override
+  public String getUserName(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getUserName(): super.getUserName(null);
+  }
+
+  @Override
+  public String getGroupName(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getGroupName(): super.getGroupName(null);
+  }
+
+  @Override
+  public FsPermission getFsPermission(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getFsPermission(): super.getFsPermission(null);
+  }
+
+  @Override
+  public long getAccessTime(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getAccessTime(): super.getAccessTime(null);
+  }
+
+  @Override
+  public long getModificationTime(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getModificationTime()
+        : super.getModificationTime(null);
+  }
+
+  @Override
+  public String toDetailString() {
+    return super.toDetailString()
+        + (isCurrentFileDeleted()? " (DELETED), ": ", ") + diffs;
   }
 }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java Fri Feb  8 02:18:55 2013
@@ -17,7 +17,10 @@
  */
 package org.apache.hadoop.hdfs.server.namenode.snapshot;
 
+import java.util.List;
+
 import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor;
 import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 
@@ -28,10 +31,34 @@ import org.apache.hadoop.hdfs.server.nam
 @InterfaceAudience.Private
 public class INodeFileWithSnapshot extends INodeFile
     implements FileWithSnapshot {
+  /**
+   * A list of file diffs.
+   */
+  static class FileDiffList extends AbstractINodeDiffList<INodeFile, FileDiff> {
+    final INodeFile currentINode;
+
+    FileDiffList(INodeFile currentINode, List<FileDiff> diffs) {
+      super(diffs);
+      this.currentINode = currentINode;
+    }
+
+    @Override
+    INodeFile getCurrentINode() {
+      return currentINode;
+    }
+
+    @Override
+    FileDiff addSnapshotDiff(Snapshot snapshot) {
+      return addLast(new FileDiff(snapshot, getCurrentINode()));
+    }
+  }
+
+  private final FileDiffList diffs;
   private FileWithSnapshot next;
 
-  public INodeFileWithSnapshot(INodeFile f) {
+  public INodeFileWithSnapshot(INodeFile f, FileDiffList diffs) {
     super(f);
+    this.diffs = new FileDiffList(this, diffs == null? null: diffs.asList());
     setNext(this);
   }
 
@@ -42,15 +69,29 @@ public class INodeFileWithSnapshot exten
       final DatanodeDescriptor clientNode) {
     final INodeFileUnderConstructionWithSnapshot f
         = new INodeFileUnderConstructionWithSnapshot(this,
-            clientName, clientMachine, clientNode);
+            clientName, clientMachine, clientNode, diffs);
     this.insertBefore(f);
     return f;
   }
 
   @Override
-  public Pair<INodeFileWithSnapshot, INodeFileSnapshot> createSnapshotCopy() {
-    return new Pair<INodeFileWithSnapshot, INodeFileSnapshot>(this,
-        new INodeFileSnapshot(this));
+  public boolean isCurrentFileDeleted() {
+    return getParent() == null;
+  }
+
+  @Override
+  public boolean isEverythingDeleted() {
+    return isCurrentFileDeleted() && diffs.asList().isEmpty();
+  }
+
+  @Override
+  public INodeFileWithSnapshot recordModification(final Snapshot latest) {
+    // if this object is NOT the latest snapshot copy, this object is created
+    // after the latest snapshot, then do NOT record modification.
+    if (this == getParent().getChild(getLocalNameBytes(), latest)) {
+      diffs.saveSelf2Snapshot(latest, null);
+    }
+    return this;
   }
 
   @Override
@@ -95,21 +136,91 @@ public class INodeFileWithSnapshot exten
   }
 
   @Override
+  public short getFileReplication(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getFileReplication()
+        : super.getFileReplication(null);
+  }
+
+  @Override
+  public short getMaxFileReplication() {
+    final short max = isCurrentFileDeleted()? 0: getFileReplication();
+    return Util.getMaxFileReplication(max, diffs);
+  }
+
+  @Override
   public short getBlockReplication() {
     return Util.getBlockReplication(this);
   }
 
   @Override
+  public long computeFileSize(boolean includesBlockInfoUnderConstruction,
+      Snapshot snapshot) {
+    final FileDiff diff = diffs.getDiff(snapshot);
+    return diff != null? diff.fileSize
+        : super.computeFileSize(includesBlockInfoUnderConstruction, null);
+  }
+
+  @Override
+  public long computeMaxFileSize() {
+    if (isCurrentFileDeleted()) {
+      final FileDiff last = diffs.getLast();
+      return last == null? 0: last.fileSize;
+    } else { 
+      return super.computeFileSize(true, null);
+    }
+  }
+
+  @Override
   public int destroySubtreeAndCollectBlocks(final Snapshot snapshot,
       final BlocksMapUpdateInfo collectedBlocks) {
-    if (snapshot != null) {
-      return 0;
-    }
-    if (next == null || next == this) {
-      // this is the only remaining inode.
-      return super.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks);
+    if (snapshot == null) {
+      clearReferences();
     } else {
-      return Util.collectSubtreeBlocksAndClear(this, collectedBlocks);
+      if (diffs.deleteSnapshotDiff(snapshot, collectedBlocks) == null) {
+        //snapshot diff not found and nothing is deleted.
+        return 0;
+      }
     }
+
+    Util.collectBlocksAndClear(this, collectedBlocks);
+    return 1;
+  }
+
+  @Override
+  public String getUserName(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getUserName(): super.getUserName(null);
+  }
+
+  @Override
+  public String getGroupName(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getGroupName(): super.getGroupName(null);
+  }
+
+  @Override
+  public FsPermission getFsPermission(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getFsPermission(): super.getFsPermission(null);
+  }
+
+  @Override
+  public long getAccessTime(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getAccessTime(): super.getAccessTime(null);
+  }
+
+  @Override
+  public long getModificationTime(Snapshot snapshot) {
+    final INodeFile inode = diffs.getSnapshotINode(snapshot);
+    return inode != null? inode.getModificationTime()
+        : super.getModificationTime(null);
+  }
+
+  @Override
+  public String toDetailString() {
+    return super.toDetailString()
+        + (isCurrentFileDeleted()? "(DELETED), ": ", ") + diffs;
   }
 }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java?rev=1443825&r1=1443824&r2=1443825&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java Fri Feb  8 02:18:55 2013
@@ -23,6 +23,7 @@ import java.util.Comparator;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.server.namenode.FSImageSerialization;
 import org.apache.hadoop.hdfs.server.namenode.INode;
@@ -89,17 +90,16 @@ public class Snapshot implements Compara
   private final Root root;
 
   Snapshot(int id, String name, INodeDirectorySnapshottable dir) {
+    this(id, DFSUtil.string2Bytes(name), dir, dir);
+  }
+
+  Snapshot(int id, byte[] name, INodeDirectory dir,
+      INodeDirectorySnapshottable parent) {
     this.id = id;
     this.root = new Root(dir);
 
     this.root.setLocalName(name);
-    this.root.setParent(dir);
-  }
-  
-  /** Constructor used when loading fsimage */
-  Snapshot(int id, INodeDirectory root) {
-    this.id = id;
-    this.root = new Root(root);
+    this.root.setParent(parent);
   }
   
   /** @return the root directory of the snapshot. */



Mime
View raw message