You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-commits@hadoop.apache.org by sz...@apache.org on 2013/04/24 02:28:08 UTC
svn commit: r1471214 - 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/util/
src/test/java/org/apache/hadoop/hdfs/server/namenode/ src/t...
Author: szetszwo
Date: Wed Apr 24 00:28:07 2013
New Revision: 1471214
URL: http://svn.apache.org/r1471214
Log:
HDFS-4735. DisallowSnapshot throws IllegalStateException for nested snapshottable directories. 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/FSNamesystem.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/util/Diff.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/snapshot/TestDisallowModifyROSnapshot.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/TestSnapshotBlocksMap.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=1471214&r1=1471213&r2=1471214&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 Apr 24 00:28:07 2013
@@ -269,3 +269,6 @@ Branch-2802 Snapshot (Unreleased)
HDFS-4719. Remove AbstractINodeDiff.Factory and move its methods to
AbstractINodeDiffList. (Arpit Agarwal via szetszwo)
+
+ HDFS-4735. DisallowSnapshot throws IllegalStateException for nested
+ snapshottable directories. (Jing Zhao via szetszwo)
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=1471214&r1=1471213&r2=1471214&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 Wed Apr 24 00:28:07 2013
@@ -5829,8 +5829,6 @@ public class FSNamesystem implements Nam
writeUnlock();
}
getEditLog().logSync();
-
- //TODO: need to update metrics in corresponding SnapshotManager method
if (auditLog.isInfoEnabled() && isExternalInvocation()) {
logAuditEvent(true, "allowSnapshot", path, null, null);
@@ -5855,8 +5853,6 @@ public class FSNamesystem implements Nam
writeUnlock();
}
getEditLog().logSync();
-
- //TODO: need to update metrics in corresponding SnapshotManager method
if (auditLog.isInfoEnabled() && isExternalInvocation()) {
logAuditEvent(true, "disallowSnapshot", path, null, null);
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=1471214&r1=1471213&r2=1471214&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 Wed Apr 24 00:28:07 2013
@@ -173,8 +173,6 @@ public class INodeDirectory extends INod
/** Replace itself with an {@link INodeDirectoryWithSnapshot}. */
public INodeDirectoryWithSnapshot replaceSelf4INodeDirectoryWithSnapshot() {
- Preconditions.checkState(!(this instanceof INodeDirectoryWithSnapshot),
- "this is already an INodeDirectoryWithSnapshot, this=%s", this);
return replaceSelf(new INodeDirectoryWithSnapshot(this));
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java Wed Apr 24 00:28:07 2013
@@ -271,7 +271,7 @@ public class Diff<K, E extends Diff.Elem
// Case 1.1.3 and 2.3.3: element is already in c-list,
previous = created.set(c, newElement);
- //TODO: fix a bug that previous != oldElement.Set it to oldElement for now
+ // For previous != oldElement, set it to oldElement
previous = oldElement;
} else {
d = search(deleted, oldElement.getKey());
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImageWithSnapshot.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/TestFSImageWithSnapshot.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImageWithSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSImageWithSnapshot.java Wed Apr 24 00:28:07 2013
@@ -173,7 +173,7 @@ public class TestFSImageWithSnapshot {
* 6. Dump the FSDirectory again and compare the two dumped string.
* </pre>
*/
- @Test (timeout=60000)
+ @Test
public void testSaveLoadImage() throws Exception {
int s = 0;
// make changes to the namesystem
@@ -213,8 +213,9 @@ public class TestFSImageWithSnapshot {
hdfs.rename(sub2file2, sub1_sub2file2);
hdfs.rename(sub1file1, sub2file1);
- // TODO: fix case hdfs.rename(sub1file1, sub1file2);
-
+ checkImage(s);
+
+ hdfs.rename(sub2file1, sub2file2);
checkImage(s);
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestDisallowModifyROSnapshot.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/TestDisallowModifyROSnapshot.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestDisallowModifyROSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestDisallowModifyROSnapshot.java Wed Apr 24 00:28:07 2013
@@ -148,8 +148,6 @@ public class TestDisallowModifyROSnapsho
public void testCreateSymlink() throws Exception {
@SuppressWarnings("deprecation")
DFSClient dfsclient = new DFSClient(conf);
- // TODO: if link is objInSnapshot, ParentNotDirectoryException got thrown
- // first by verifyParentDir()
dfsclient.createSymlink(sub2.toString(), "/TestSnapshot/sub1/.snapshot",
false);
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.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/TestNestedSnapshots.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java Wed Apr 24 00:28:07 2013
@@ -18,6 +18,7 @@
package org.apache.hadoop.hdfs.server.namenode.snapshot;
import static org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectorySnapshottable.SNAPSHOT_LIMIT;
+import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.Random;
@@ -34,11 +35,13 @@ import org.apache.hadoop.hdfs.Distribute
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.hdfs.protocol.HdfsConstants;
import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
+import org.apache.hadoop.hdfs.server.namenode.FSDirectory;
+import org.apache.hadoop.hdfs.server.namenode.INode;
import org.apache.hadoop.hdfs.server.namenode.INodeDirectory;
import org.apache.hadoop.ipc.RemoteException;
-import org.junit.AfterClass;
+import org.junit.After;
import org.junit.Assert;
-import org.junit.BeforeClass;
+import org.junit.Before;
import org.junit.Test;
/** Testing nested snapshots. */
@@ -57,16 +60,16 @@ public class TestNestedSnapshots {
private static MiniDFSCluster cluster;
private static DistributedFileSystem hdfs;
- @BeforeClass
- public static void setUp() throws Exception {
+ @Before
+ public void setUp() throws Exception {
cluster = new MiniDFSCluster.Builder(conf).numDataNodes(REPLICATION)
.build();
cluster.waitActive();
hdfs = cluster.getFileSystem();
}
- @AfterClass
- public static void tearDown() throws Exception {
+ @After
+ public void tearDown() throws Exception {
if (cluster != null) {
cluster.shutdown();
}
@@ -279,4 +282,32 @@ public class TestNestedSnapshots {
}
}
}
+
+ /**
+ * When we have nested snapshottable directories and if we try to reset the
+ * snapshottable descendant back to an regular directory, we need to replace
+ * the snapshottable descendant with an INodeDirectoryWithSnapshot
+ */
+ @Test
+ public void testDisallowNestedSnapshottableDir() throws Exception {
+ final Path dir = new Path("/dir");
+ final Path sub = new Path(dir, "sub");
+ hdfs.mkdirs(sub);
+
+ SnapshotTestHelper.createSnapshot(hdfs, dir, "s1");
+ final Path file = new Path(sub, "file");
+ DFSTestUtil.createFile(hdfs, file, BLOCKSIZE, REPLICATION, SEED);
+
+ FSDirectory fsdir = cluster.getNamesystem().getFSDirectory();
+ INode subNode = fsdir.getINode(sub.toString());
+ assertTrue(subNode instanceof INodeDirectoryWithSnapshot);
+
+ hdfs.allowSnapshot(sub);
+ subNode = fsdir.getINode(sub.toString());
+ assertTrue(subNode instanceof INodeDirectorySnapshottable);
+
+ hdfs.disallowSnapshot(sub);
+ subNode = fsdir.getINode(sub.toString());
+ assertTrue(subNode instanceof INodeDirectoryWithSnapshot);
+ }
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.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/TestSnapshot.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java Wed Apr 24 00:28:07 2013
@@ -411,7 +411,6 @@ public class TestSnapshot {
* the owner, and the other indicates the group
*/
private String[] genRandomOwner() {
- // TODO
String[] userGroup = new String[]{"dr.who", "unknown"};
return userGroup;
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.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/TestSnapshotBlocksMap.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java Wed Apr 24 00:28:07 2013
@@ -24,7 +24,6 @@ import static org.junit.Assert.assertNul
import static org.junit.Assert.fail;
import java.io.IOException;
-import java.util.Arrays;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
@@ -46,9 +45,6 @@ import org.junit.Test;
* Test cases for snapshot-related information in blocksMap.
*/
public class TestSnapshotBlocksMap {
- // TODO: fix concat for snapshot
- private static final boolean runConcatTest = false;
-
private static final long seed = 0;
private static final short REPLICATION = 3;
private static final int BLOCKSIZE = 1024;
@@ -208,36 +204,5 @@ public class TestSnapshotBlocksMap {
} catch (IOException e) {
assertExceptionContains("File does not exist: " + s1f0, e);
}
-
- // concat file1, file3 and file5 to file4
- if (runConcatTest) {
- final INodeFile f1 = assertBlockCollection(file1.toString(), 2, fsdir,
- blockmanager);
- final BlockInfo[] f1blocks = f1.getBlocks();
- final INodeFile f3 = assertBlockCollection(file3.toString(), 5, fsdir,
- blockmanager);
- final BlockInfo[] f3blocks = f3.getBlocks();
- final INodeFile f5 = assertBlockCollection(file5.toString(), 7, fsdir,
- blockmanager);
- final BlockInfo[] f5blocks = f5.getBlocks();
- assertBlockCollection(file4.toString(), 1, fsdir, blockmanager);
-
- hdfs.concat(file4, new Path[]{file1, file3, file5});
-
- final INodeFile f4 = assertBlockCollection(file4.toString(), 15, fsdir,
- blockmanager);
- final BlockInfo[] blocks4 = f4.getBlocks();
- for(BlockInfo[] blocks : Arrays.asList(f1blocks, f3blocks, blocks4, f5blocks)) {
- for(BlockInfo b : blocks) {
- assertBlockCollection(blockmanager, f4, b);
- }
- }
- assertAllNull(f1, file1, snapshots);
- assertAllNull(f3, file3, snapshots);
- assertAllNull(f5, file5, snapshots);
- }
}
-
- // TODO: test for deletion file which was appended after taking snapshots
-
}
Modified: hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.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/TestSnapshotReplication.java?rev=1471214&r1=1471213&r2=1471214&view=diff
==============================================================================
--- hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java (original)
+++ hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java Wed Apr 24 00:28:07 2013
@@ -213,7 +213,6 @@ public class TestSnapshotReplication {
checkFileReplication(file1, REPLICATION, REPLICATION);
checkSnapshotFileReplication(file1, snapshotRepMap, REPLICATION);
- // TODO: check replication after deleting snapshot(s)
// Delete file1
hdfs.delete(file1, true);
// Check replication of snapshots