Mailing-List: contact hdfs-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hdfs-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1480146 - in
 /hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs: ./
 src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/
 src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/
Date: Wed, 08 May 2013 01:17:25 -0000
To: hdfs-commits@hadoop.apache.org
From: szetszwo@apache.org
Message-Id: <20130508011725.3F2FD238889B@eris.apache.org>

Author: szetszwo
Date: Wed May  8 01:17:24 2013
New Revision: 1480146

URL: http://svn.apache.org/r1480146
Log:
HDFS-4806. In INodeDirectoryWithSnapshot, use isInLatestSnapshot() to determine if an added/removed child should be recorded in the snapshot diff.  Contributed by Jing Zhao

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/snapshot/INodeDirectoryWithSnapshot.java
    hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.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=1480146&r1=1480145&r2=1480146&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 Wed May  8 01:17:24 2013
@@ -347,3 +347,7 @@ Branch-2802 Snapshot (Unreleased)
 
   HDFS-4802. Disallowing snapshot on / twice should throw SnapshotException
   but not IllegalStateException.  (Jing Zhao via szetszwo)
+
+  HDFS-4806. In INodeDirectoryWithSnapshot, use isInLatestSnapshot() to 
+  determine if an added/removed child should be recorded in the snapshot diff.
+  (Jing Zhao via szetszwo)

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=1480146&r1=1480145&r2=1480146&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 Wed May  8 01:17:24 2013
@@ -550,7 +550,7 @@ public class INodeDirectoryWithSnapshot 
       final INodeMap inodeMap) throws QuotaExceededException {
     ChildrenDiff diff = null;
     Integer undoInfo = null;
-    if (latest != null) {
+    if (isInLatestSnapshot(latest)) {
       diff = diffs.checkAndAddLatestSnapshotDiff(latest, this).diff;
       undoInfo = diff.create(inode);
     }
@@ -566,7 +566,18 @@ public class INodeDirectoryWithSnapshot 
       final INodeMap inodeMap) throws QuotaExceededException {
     ChildrenDiff diff = null;
     UndoInfo<INode> undoInfo = null;
-    if (latest != null) {
+    // For a directory that is not a renamed node, if isInLatestSnapshot returns
+    // false, the directory is not in the latest snapshot, thus we do not need
+    // to record the removed child in any snapshot.
+    // For a directory that was moved/renamed, note that if the directory is in
+    // any of the previous snapshots, we will create a reference node for the 
+    // directory while rename, and isInLatestSnapshot will return true in that
+    // scenario (if all previous snapshots have been deleted, isInLatestSnapshot
+    // still returns false). Thus if isInLatestSnapshot returns false, the 
+    // directory node cannot be in any snapshot (not in current tree, nor in 
+    // previous src tree). Thus we do not need to record the removed child in 
+    // any snapshot.
+    if (isInLatestSnapshot(latest)) {
       diff = diffs.checkAndAddLatestSnapshotDiff(latest, this).diff;
       undoInfo = diff.delete(child);
     }

Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java?rev=1480146&r1=1480145&r2=1480146&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java Wed May  8 01:17:24 2013
@@ -52,6 +52,7 @@ import org.apache.hadoop.hdfs.server.nam
 import org.apache.hadoop.hdfs.server.namenode.FSNamesystem;
 import org.apache.hadoop.hdfs.server.namenode.INode;
 import org.apache.hadoop.hdfs.server.namenode.INodeDirectory;
+import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 import org.apache.hadoop.hdfs.server.namenode.INodeMap;
 import org.apache.hadoop.hdfs.server.namenode.INodeReference;
 import org.apache.hadoop.hdfs.server.namenode.INodeReference.WithCount;
@@ -1674,4 +1675,51 @@ public class TestRenameWithSnapshots {
     
     restartClusterAndCheckImage();
   }
+  
+  /**
+   * This test demonstrates that 
+   * {@link INodeDirectoryWithSnapshot#removeChild(INode, Snapshot, INodeMap)}
+   * and 
+   * {@link INodeDirectoryWithSnapshot#addChild(INode, boolean, Snapshot, INodeMap)}
+   * should use {@link INode#isInLatestSnapshot(Snapshot)} to check if the 
+   * added/removed child should be recorded in snapshots.
+   */
+  @Test
+  public void testRenameAndDeleteSnapshot_5() throws Exception {
+    final Path dir1 = new Path("/dir1");
+    final Path dir2 = new Path("/dir2");
+    final Path dir3 = new Path("/dir3");
+    hdfs.mkdirs(dir1);
+    hdfs.mkdirs(dir2);
+    hdfs.mkdirs(dir3);
+    
+    final Path foo = new Path(dir1, "foo");
+    hdfs.mkdirs(foo);
+    SnapshotTestHelper.createSnapshot(hdfs, dir1, "s1");
+    final Path bar = new Path(foo, "bar");
+    // create file bar, and foo will become an INodeDirectoryWithSnapshot
+    DFSTestUtil.createFile(hdfs, bar, BLOCKSIZE, REPL, SEED);
+    // delete snapshot s1. now foo is not in any snapshot
+    hdfs.deleteSnapshot(dir1, "s1");
+    
+    SnapshotTestHelper.createSnapshot(hdfs, dir2, "s2");
+    // rename /dir1/foo to /dir2/foo
+    final Path foo2 = new Path(dir2, foo.getName());
+    hdfs.rename(foo, foo2);
+    // rename /dir2/foo/bar to /dir3/foo/bar
+    final Path bar2 = new Path(dir2, "foo/bar");
+    final Path bar3 = new Path(dir3, "bar");
+    hdfs.rename(bar2, bar3);
+    
+    // delete /dir2/foo. Since it is not in any snapshot, we will call its 
+    // destroy function. If we do not use isInLatestSnapshot in removeChild and
+    // addChild methods in INodeDirectoryWithSnapshot, the file bar will be 
+    // stored in the deleted list of foo, and will be destroyed.
+    hdfs.delete(foo2, true);
+    
+    // check if /dir3/bar still exists
+    assertTrue(hdfs.exists(bar3));
+    INodeFile barNode = (INodeFile) fsdir.getINode4Write(bar3.toString());
+    assertSame(fsdir.getINode4Write(dir3.toString()), barNode.getParent());
+  }
 }