Author: szetszwo
Date: Tue Apr 16 22:03:58 2013
New Revision: 1468632
URL: http://svn.apache.org/r1468632
Log:
HDFS-4700. Fix the undo section of rename with snapshots. 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/FSDirectory.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/INodeReference.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/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/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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -241,3 +241,6 @@ Branch-2802 Snapshot (Unreleased)
create a file/directory with ".snapshot" as the name. If ".snapshot" is used
in a previous version of HDFS, it must be renamed before upgrade; otherwise,
upgrade will fail. (szetszwo)
+
+ HDFS-4700. Fix the undo section of rename with snapshots. (Jing Zhao via
+ 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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -62,6 +62,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.INodeReference.WithCount;
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.Snapshot;
@@ -582,12 +583,17 @@ public class FSDirectory implements Clos
// check srcChild for reference
final INodeReference.WithCount withCount;
- if (srcChildIsReference || isSrcInSnapshot) {
+ int srcRefDstSnapshot = srcChildIsReference ? srcChild.asReference()
+ .getDstSnapshotId() : Snapshot.INVALID_ID;
+ if (isSrcInSnapshot) {
final INodeReference.WithName withName =
srcIIP.getINode(-2).asDirectory()
.replaceChild4ReferenceWithName(srcChild);
- withCount = (INodeReference.WithCount)withName.getReferredINode();
+ withCount = (INodeReference.WithCount) withName.getReferredINode();
srcChild = withName;
srcIIP.setLastINode(srcChild);
+ } else if (srcChildIsReference) {
+ // srcChild is reference but srcChild is not in latest snapshot
+ withCount = (WithCount) srcChild.asReference().getReferredINode();
} else {
withCount = null;
}
@@ -602,7 +608,6 @@ public class FSDirectory implements Clos
return false;
}
- // add src to the destination
if (dstParent.getParent() == null) {
// src and dst file/dir are in the same directory, and the dstParent
has
// been replaced when we removed the src. Refresh the dstIIP and
@@ -611,6 +616,8 @@ public class FSDirectory implements Clos
dstParent = dstIIP.getINode(-2);
}
+ // add src to the destination
+
srcChild = srcIIP.getLastINode();
final byte[] dstChildName = dstIIP.getLastLocalName();
final INode toDst;
@@ -645,17 +652,29 @@ public class FSDirectory implements Clos
}
} finally {
if (!added) {
+ final INodeDirectory srcParent = srcIIP.getINode(-2).asDirectory();
+ final INode oldSrcChild = srcChild;
// put it back
if (withCount == null) {
srcChild.setLocalName(srcChildName);
} else if (!srcChildIsReference) { // src must be in snapshot
- final INodeDirectoryWithSnapshot srcParent =
- (INodeDirectoryWithSnapshot) srcIIP.getINode(-2).asDirectory();
final INode originalChild = withCount.getReferredINode();
- srcParent.replaceRemovedChild(srcChild, originalChild);
srcChild = originalChild;
+ } else {
+ final INodeReference originalRef = new INodeReference.DstReference(
+ srcParent, withCount, srcRefDstSnapshot);
+ withCount.setParentReference(originalRef);
+ srcChild = originalRef;
+ }
+
+ if (isSrcInSnapshot) {
+ ((INodeDirectoryWithSnapshot) srcParent).undoRename4ScrParent(
+ oldSrcChild.asReference(), srcChild, srcIIP.getLatestSnapshot());
+ } else {
+ // srcParent is not an INodeDirectoryWithSnapshot, we only need to
add
+ // the srcChild back
+ addLastINodeNoQuotaCheck(srcIIP, srcChild);
}
- addLastINodeNoQuotaCheck(srcIIP, srcChild);
}
}
NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
@@ -797,12 +816,17 @@ public class FSDirectory implements Clos
// check srcChild for reference
final INodeReference.WithCount withCount;
- if (srcChildIsReference || isSrcInSnapshot) {
+ int srcRefDstSnapshot = srcChildIsReference ? srcChild.asReference()
+ .getDstSnapshotId() : Snapshot.INVALID_ID;
+ if (isSrcInSnapshot) {
final INodeReference.WithName withName =
srcIIP.getINode(-2).asDirectory()
.replaceChild4ReferenceWithName(srcChild);
- withCount = (INodeReference.WithCount)withName.getReferredINode();
+ withCount = (INodeReference.WithCount) withName.getReferredINode();
srcChild = withName;
srcIIP.setLastINode(srcChild);
+ } else if (srcChildIsReference) {
+ // srcChild is reference but srcChild is not in latest snapshot
+ withCount = (WithCount) srcChild.asReference().getReferredINode();
} else {
withCount = null;
}
@@ -888,21 +912,38 @@ public class FSDirectory implements Clos
} finally {
if (undoRemoveSrc) {
// Rename failed - restore src
- srcChild = srcIIP.getLastINode();
+ final INodeDirectory srcParent = srcIIP.getINode(-2).asDirectory();
+ final INode oldSrcChild = srcChild;
+ // put it back
if (withCount == null) {
srcChild.setLocalName(srcChildName);
} else if (!srcChildIsReference) { // src must be in snapshot
- final INodeDirectoryWithSnapshot srcParent
- = (INodeDirectoryWithSnapshot)srcIIP.getINode(-2).asDirectory();
final INode originalChild = withCount.getReferredINode();
- srcParent.replaceRemovedChild(srcChild, originalChild);
srcChild = originalChild;
+ } else {
+ final INodeReference originalRef = new INodeReference.DstReference(
+ srcParent, withCount, srcRefDstSnapshot);
+ withCount.setParentReference(originalRef);
+ srcChild = originalRef;
+ }
+
+ if (srcParent instanceof INodeDirectoryWithSnapshot) {
+ ((INodeDirectoryWithSnapshot) srcParent).undoRename4ScrParent(
+ oldSrcChild.asReference(), srcChild, srcIIP.getLatestSnapshot());
+ } else {
+ // srcParent is not an INodeDirectoryWithSnapshot, we only need to
add
+ // the srcChild back
+ addLastINodeNoQuotaCheck(srcIIP, srcChild);
}
- addLastINodeNoQuotaCheck(srcIIP, srcChild);
}
if (undoRemoveDst) {
// Rename failed - restore dst
- addLastINodeNoQuotaCheck(dstIIP, removedDst);
+ if (dstParent instanceof INodeDirectoryWithSnapshot) {
+ ((INodeDirectoryWithSnapshot) dstParent).undoRename4DstParent(
+ removedDst, dstIIP.getLatestSnapshot());
+ } else {
+ addLastINodeNoQuotaCheck(dstIIP, removedDst);
+ }
}
}
NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: "
@@ -1270,7 +1311,7 @@ public class FSDirectory implements Clos
Quota.Counts counts = targetNode.cleanSubtree(null, latestSnapshot,
collectedBlocks);
parent.addSpaceConsumed(-counts.get(Quota.NAMESPACE),
- -counts.get(Quota.DISKSPACE));
+ -counts.get(Quota.DISKSPACE), true);
removed = counts.get(Quota.NAMESPACE);
}
if (NameNode.stateChangeLog.isDebugEnabled()) {
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -376,11 +376,11 @@ public abstract class INode implements D
* Check and add namespace/diskspace consumed to itself and the ancestors.
* @throws QuotaExceededException if quote is violated.
*/
- public void addSpaceConsumed(long nsDelta, long dsDelta)
+ public void addSpaceConsumed(long nsDelta, long dsDelta, boolean verify)
throws QuotaExceededException {
final INodeDirectory parentDir = getParent();
if (parentDir != null) {
- parentDir.addSpaceConsumed(nsDelta, dsDelta);
+ parentDir.addSpaceConsumed(nsDelta, dsDelta, verify);
}
}
@@ -403,7 +403,7 @@ public abstract class INode implements D
/**
* Count subtree {@link Quota#NAMESPACE} and {@link Quota#DISKSPACE} usages.
*/
- final Quota.Counts computeQuotaUsage() {
+ public final Quota.Counts computeQuotaUsage() {
return computeQuotaUsage(new Quota.Counts(), true);
}
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -216,6 +216,7 @@ public class INodeDirectory extends INod
Preconditions.checkState(i >= 0);
Preconditions.checkState(oldChild == children.get(i));
+ // TODO: the first case may never be hit
if (oldChild.isReference() && !newChild.isReference()) {
final INode withCount = oldChild.asReference().getReferredINode();
withCount.asReference().setReferredINode(newChild);
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -140,21 +140,23 @@ public class INodeDirectoryWithQuota ext
}
@Override
- public final void addSpaceConsumed(final long nsDelta, final long dsDelta)
- throws QuotaExceededException {
+ public final void addSpaceConsumed(final long nsDelta, final long dsDelta,
+ boolean verify) throws QuotaExceededException {
if (isQuotaSet()) {
// The following steps are important:
// check quotas in this inode and all ancestors before changing counts
// so that no change is made if there is any quota violation.
- // (1) verify quota in this inode
- verifyQuota(nsDelta, dsDelta);
+ // (1) verify quota in this inode
+ if (verify) {
+ verifyQuota(nsDelta, dsDelta);
+ }
// (2) verify quota and then add count in ancestors
- super.addSpaceConsumed(nsDelta, dsDelta);
+ super.addSpaceConsumed(nsDelta, dsDelta, verify);
// (3) add count in this inode
addSpaceConsumed2Cache(nsDelta, dsDelta);
} else {
- super.addSpaceConsumed(nsDelta, dsDelta);
+ super.addSpaceConsumed(nsDelta, dsDelta, verify);
}
}
Modified:
hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.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/INodeReference.java?rev=1468632&r1=1468631&r2=1468632&view=diff
==============================================================================
---
hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java
(original)
+++
hadoop/common/branches/HDFS-2802/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java
Tue Apr 16 22:03:58 2013
@@ -254,9 +254,9 @@ public abstract class INodeReference ext
}
@Override
- public final void addSpaceConsumed(long nsDelta, long dsDelta
- ) throws QuotaExceededException {
- referred.addSpaceConsumed(nsDelta, dsDelta);
+ public final void addSpaceConsumed(long nsDelta, long dsDelta, boolean
verify)
+ throws QuotaExceededException {
+ referred.addSpaceConsumed(nsDelta, dsDelta, verify);
}
@Override
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -108,7 +108,7 @@ abstract class AbstractINodeDiffList<N e
/** Add an {@link AbstractINodeDiff} for the given snapshot. */
final D addDiff(Snapshot latest, N currentINode)
throws QuotaExceededException {
- currentINode.addSpaceConsumed(1, 0);
+ currentINode.addSpaceConsumed(1, 0, true);
return addLast(factory.createDiff(latest, currentINode));
}
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -325,7 +325,7 @@ public class INodeDirectorySnapshottable
INodeDirectory parent = getParent();
if (parent != null) {
parent.addSpaceConsumed(-counts.get(Quota.NAMESPACE),
- -counts.get(Quota.DISKSPACE));
+ -counts.get(Quota.DISKSPACE), true);
}
} catch(QuotaExceededException e) {
LOG.error("BUG: removeSnapshot increases namespace usage.", e);
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -81,16 +81,16 @@ public class INodeDirectoryWithSnapshot
return true;
}
- private final boolean removeCreatedChild(final int c, final INode child) {
- final List<INode> created = getList(ListType.CREATED);
- if (created.get(c) == child) {
- final INode removed = created.remove(c);
- Preconditions.checkState(removed == child);
+ private final boolean removeChild(ListType type, final INode child) {
+ final List<INode> list = getList(type);
+ final int i = searchIndex(type, child.getLocalNameBytes());
+ if (i >= 0 && list.get(i) == child) {
+ list.remove(i);
return true;
}
return false;
}
-
+
/** clear the created list */
private Quota.Counts destroyCreatedList(
final INodeDirectoryWithSnapshot currentINode,
@@ -452,6 +452,18 @@ public class INodeDirectoryWithSnapshot
}
return false;
}
+
+ /** Remove the given child in the created/deleted list, if there is any. */
+ private boolean removeChild(final ListType type, final INode child) {
+ final List<DirectoryDiff> diffList = asList();
+ for(int i = diffList.size() - 1; i >= 0; i--) {
+ final ChildrenDiff diff = diffList.get(i).diff;
+ if (diff.removeChild(type, child)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
/**
@@ -627,15 +639,11 @@ public class INodeDirectoryWithSnapshot
}
// remove same child from the created list, if there is any.
- final byte[] name = child.getLocalNameBytes();
final List<DirectoryDiff> diffList = diffs.asList();
for(int i = diffList.size() - 1; i >= 0; i--) {
final ChildrenDiff diff = diffList.get(i).diff;
- final int c = diff.searchIndex(ListType.CREATED, name);
- if (c >= 0) {
- if (diff.removeCreatedChild(c, child)) {
- return true;
- }
+ if (diff.removeChild(ListType.CREATED, child)) {
+ return true;
}
}
return true;
@@ -646,12 +654,65 @@ public class INodeDirectoryWithSnapshot
super.replaceChild(oldChild, newChild);
diffs.replaceChild(ListType.CREATED, oldChild, newChild);
}
-
- /** The child just has been removed, replace it with a reference. */
- public void replaceRemovedChild(INode oldChild, INode newChild) {
- // the old child must be in the deleted list
- Preconditions.checkState(
- diffs.replaceChild(ListType.DELETED, oldChild, newChild));
+
+ /**
+ * This method is usually called by the undo section of rename.
+ *
+ * Before calling this function, in the rename operation, we replace the
+ * original src node (of the rename operation) with a reference node
(WithName
+ * instance) in both the children list and a created list, delete the
+ * reference node from the children list, and add it to the corresponding
+ * deleted list.
+ *
+ * To undo the above operations, we have the following steps in particular:
+ *
+ * <pre>
+ * 1) remove the WithName node from the deleted list (if it exists)
+ * 2) replace the WithName node in the created list with srcChild
+ * 3) add srcChild back as a child of srcParent. Note that we already add
+ * the node into the created list of a snapshot diff in step 2, we do not
need
+ * to add srcChild to the created list of the latest snapshot.
+ * </pre>
+ *
+ * We do not need to update quota usage because the old child is in the
+ * deleted list before.
+ *
+ * @param oldChild
+ * The reference node to be removed/replaced
+ * @param newChild
+ * The node to be added back
+ * @param latestSnapshot
+ * The latest snapshot. Note this may not be the last snapshot in
the
+ * {@link #diffs}, since the src tree of the current rename
operation
+ * may be the dst tree of a previous rename.
+ * @throws QuotaExceededException should not throw this exception
+ */
+ public void undoRename4ScrParent(final INodeReference oldChild,
+ final INode newChild, Snapshot latestSnapshot)
+ throws QuotaExceededException {
+ diffs.removeChild(ListType.DELETED, oldChild);
+ diffs.replaceChild(ListType.CREATED, oldChild, newChild);
+ addChild(newChild, true, null);
+ }
+
+ /**
+ * Undo the rename operation for the dst tree, i.e., if the rename operation
+ * (with OVERWRITE option) removes a file/dir from the dst tree, add it back
+ * and delete possible record in the deleted list.
+ */
+ public void undoRename4DstParent(final INode deletedChild,
+ Snapshot latestSnapshot) throws QuotaExceededException {
+ boolean removeDeletedChild = diffs.removeChild(ListType.DELETED,
+ deletedChild);
+ final boolean added = addChild(deletedChild, true, removeDeletedChild ?
null
+ : latestSnapshot);
+ // update quota usage if adding is successfully and the old child has not
+ // been stored in deleted list before
+ if (added && !removeDeletedChild) {
+ final Quota.Counts counts = deletedChild.computeQuotaUsage();
+ addSpaceConsumed(counts.get(Quota.NAMESPACE),
+ counts.get(Quota.DISKSPACE), false);
+ }
}
@Override
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=1468632&r1=1468631&r2=1468632&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
Tue Apr 16 22:03:58 2013
@@ -21,6 +21,10 @@ import static org.junit.Assert.assertEqu
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
import java.io.File;
import java.io.IOException;
@@ -48,7 +52,10 @@ import org.apache.hadoop.hdfs.server.nam
import org.apache.hadoop.hdfs.server.namenode.INodeReference;
import org.apache.hadoop.hdfs.server.namenode.INodeReference.WithCount;
import
org.apache.hadoop.hdfs.server.namenode.snapshot.FileWithSnapshot.FileDiff;
+import
org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot.ChildrenDiff;
import
org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot.DirectoryDiff;
+import org.apache.hadoop.hdfs.util.Diff.ListType;
+import org.apache.hadoop.hdfs.util.ReadOnlyList;
import org.apache.hadoop.test.GenericTestUtils;
import org.junit.After;
import org.junit.Assert;
@@ -1159,4 +1166,224 @@ public class TestRenameWithSnapshots {
hdfs.deleteSnapshot(sdir1, "s1");
restartClusterAndCheckImage();
}
+
+ /**
+ * Test the undo section of rename. Before the rename, we create the renamed
+ * file/dir before taking the snapshot.
+ */
+ @Test
+ public void testRenameUndo() throws Exception {
+ final Path sdir1 = new Path("/dir1");
+ final Path sdir2 = new Path("/dir2");
+ hdfs.mkdirs(sdir1);
+ hdfs.mkdirs(sdir2);
+ final Path foo = new Path(sdir1, "foo");
+ final Path bar = new Path(foo, "bar");
+ DFSTestUtil.createFile(hdfs, bar, BLOCKSIZE, REPL, SEED);
+ final Path dir2file = new Path(sdir2, "file");
+ DFSTestUtil.createFile(hdfs, dir2file, BLOCKSIZE, REPL, SEED);
+
+ SnapshotTestHelper.createSnapshot(hdfs, sdir1, "s1");
+
+ INodeDirectory dir2 = fsdir.getINode4Write(sdir2.toString()).asDirectory();
+ INodeDirectory mockDir2 = spy(dir2);
+ doReturn(false).when(mockDir2).addChild((INode) anyObject(), anyBoolean(),
+ (Snapshot) anyObject());
+ INodeDirectory root = fsdir.getINode4Write("/").asDirectory();
+ root.replaceChild(dir2, mockDir2);
+
+ final Path newfoo = new Path(sdir2, "foo");
+ boolean result = hdfs.rename(foo, newfoo);
+ assertFalse(result);
+
+ // check the current internal details
+ INodeDirectorySnapshottable dir1Node = (INodeDirectorySnapshottable) fsdir
+ .getINode4Write(sdir1.toString());
+ ReadOnlyList<INode> dir1Children = dir1Node.getChildrenList(null);
+ assertEquals(1, dir1Children.size());
+ assertEquals(foo.getName(), dir1Children.get(0).getLocalName());
+ List<DirectoryDiff> dir1Diffs = dir1Node.getDiffs().asList();
+ assertEquals(1, dir1Diffs.size());
+ assertEquals("s1", dir1Diffs.get(0).snapshot.getRoot().getLocalName());
+
+ // after the undo of rename, both the created and deleted list of sdir1
+ // should be empty
+ ChildrenDiff childrenDiff = dir1Diffs.get(0).getChildrenDiff();
+ assertEquals(0, childrenDiff.getList(ListType.DELETED).size());
+ assertEquals(0, childrenDiff.getList(ListType.CREATED).size());
+
+ INode fooNode = fsdir.getINode4Write(foo.toString());
+ assertTrue(fooNode instanceof INodeDirectoryWithSnapshot);
+ List<DirectoryDiff> fooDiffs = ((INodeDirectoryWithSnapshot) fooNode)
+ .getDiffs().asList();
+ assertEquals(1, fooDiffs.size());
+ assertEquals("s1", fooDiffs.get(0).snapshot.getRoot().getLocalName());
+
+ final Path foo_s1 = SnapshotTestHelper.getSnapshotPath(sdir1, "s1", "foo");
+ INode fooNode_s1 = fsdir.getINode(foo_s1.toString());
+ assertTrue(fooNode_s1 == fooNode);
+
+ // check sdir2
+ assertFalse(hdfs.exists(newfoo));
+ INodeDirectory dir2Node = fsdir.getINode4Write(sdir2.toString())
+ .asDirectory();
+ assertFalse(dir2Node instanceof INodeDirectoryWithSnapshot);
+ ReadOnlyList<INode> dir2Children = dir2Node.getChildrenList(null);
+ assertEquals(1, dir2Children.size());
+ assertEquals(dir2file.getName(), dir2Children.get(0).getLocalName());
+ }
+
+ /**
+ * Test the undo section of rename. Before the rename, we create the renamed
+ * file/dir after taking the snapshot.
+ */
+ @Test
+ public void testRenameUndo_2() throws Exception {
+ final Path sdir1 = new Path("/dir1");
+ final Path sdir2 = new Path("/dir2");
+ hdfs.mkdirs(sdir1);
+ hdfs.mkdirs(sdir2);
+ final Path dir2file = new Path(sdir2, "file");
+ DFSTestUtil.createFile(hdfs, dir2file, BLOCKSIZE, REPL, SEED);
+
+ SnapshotTestHelper.createSnapshot(hdfs, sdir1, "s1");
+
+ // create foo after taking snapshot
+ final Path foo = new Path(sdir1, "foo");
+ final Path bar = new Path(foo, "bar");
+ DFSTestUtil.createFile(hdfs, bar, BLOCKSIZE, REPL, SEED);
+
+ INodeDirectory dir2 = fsdir.getINode4Write(sdir2.toString()).asDirectory();
+ INodeDirectory mockDir2 = spy(dir2);
+ doReturn(false).when(mockDir2).addChild((INode) anyObject(), anyBoolean(),
+ (Snapshot) anyObject());
+ INodeDirectory root = fsdir.getINode4Write("/").asDirectory();
+ root.replaceChild(dir2, mockDir2);
+
+ final Path newfoo = new Path(sdir2, "foo");
+ boolean result = hdfs.rename(foo, newfoo);
+ assertFalse(result);
+
+ // check the current internal details
+ INodeDirectorySnapshottable dir1Node = (INodeDirectorySnapshottable) fsdir
+ .getINode4Write(sdir1.toString());
+ ReadOnlyList<INode> dir1Children = dir1Node.getChildrenList(null);
+ assertEquals(1, dir1Children.size());
+ assertEquals(foo.getName(), dir1Children.get(0).getLocalName());
+ List<DirectoryDiff> dir1Diffs = dir1Node.getDiffs().asList();
+ assertEquals(1, dir1Diffs.size());
+ assertEquals("s1", dir1Diffs.get(0).snapshot.getRoot().getLocalName());
+
+ // after the undo of rename, the created list of sdir1 should contain
+ // 1 element
+ ChildrenDiff childrenDiff = dir1Diffs.get(0).getChildrenDiff();
+ assertEquals(0, childrenDiff.getList(ListType.DELETED).size());
+ assertEquals(1, childrenDiff.getList(ListType.CREATED).size());
+
+ INode fooNode = fsdir.getINode4Write(foo.toString());
+ assertTrue(fooNode instanceof INodeDirectory);
+ assertTrue(childrenDiff.getList(ListType.CREATED).get(0) == fooNode);
+
+ final Path foo_s1 = SnapshotTestHelper.getSnapshotPath(sdir1, "s1", "foo");
+ assertFalse(hdfs.exists(foo_s1));
+
+ // check sdir2
+ assertFalse(hdfs.exists(newfoo));
+ INodeDirectory dir2Node = fsdir.getINode4Write(sdir2.toString())
+ .asDirectory();
+ assertFalse(dir2Node instanceof INodeDirectoryWithSnapshot);
+ ReadOnlyList<INode> dir2Children = dir2Node.getChildrenList(null);
+ assertEquals(1, dir2Children.size());
+ assertEquals(dir2file.getName(), dir2Children.get(0).getLocalName());
+ }
+
+ /**
+ * Test the undo section of the second-time rename.
+ */
+ @Test
+ public void testRenameUndo_3() throws Exception {
+ final Path sdir1 = new Path("/dir1");
+ final Path sdir2 = new Path("/dir2");
+ final Path sdir3 = new Path("/dir3");
+ hdfs.mkdirs(sdir1);
+ hdfs.mkdirs(sdir2);
+ hdfs.mkdirs(sdir3);
+ final Path foo = new Path(sdir1, "foo");
+ final Path bar = new Path(foo, "bar");
+ DFSTestUtil.createFile(hdfs, bar, BLOCKSIZE, REPL, SEED);
+
+ SnapshotTestHelper.createSnapshot(hdfs, sdir1, "s1");
+ SnapshotTestHelper.createSnapshot(hdfs, sdir2, "s2");
+
+ INodeDirectory dir3 = fsdir.getINode4Write(sdir3.toString()).asDirectory();
+ INodeDirectory mockDir3 = spy(dir3);
+ doReturn(false).when(mockDir3).addChild((INode) anyObject(), anyBoolean(),
+ (Snapshot) anyObject());
+ INodeDirectory root = fsdir.getINode4Write("/").asDirectory();
+ root.replaceChild(dir3, mockDir3);
+
+ final Path foo_dir2 = new Path(sdir2, "foo");
+ final Path foo_dir3 = new Path(sdir3, "foo");
+ hdfs.rename(foo, foo_dir2);
+ boolean result = hdfs.rename(foo_dir2, foo_dir3);
+ assertFalse(result);
+
+ // check the current internal details
+ INodeDirectorySnapshottable dir2Node = (INodeDirectorySnapshottable) fsdir
+ .getINode4Write(sdir2.toString());
+ ReadOnlyList<INode> dir2Children = dir2Node.getChildrenList(null);
+ assertEquals(1, dir2Children.size());
+ List<DirectoryDiff> dir2Diffs = dir2Node.getDiffs().asList();
+ assertEquals(1, dir2Diffs.size());
+ assertEquals("s2", Snapshot.getSnapshotName(dir2Diffs.get(0).snapshot));
+ ChildrenDiff childrenDiff = dir2Diffs.get(0).getChildrenDiff();
+ assertEquals(0, childrenDiff.getList(ListType.DELETED).size());
+ assertEquals(1, childrenDiff.getList(ListType.CREATED).size());
+ final Path foo_s2 = SnapshotTestHelper.getSnapshotPath(sdir2, "s2", "foo");
+ assertFalse(hdfs.exists(foo_s2));
+
+ INode fooNode = fsdir.getINode4Write(foo_dir2.toString());
+ assertTrue(childrenDiff.getList(ListType.CREATED).get(0) == fooNode);
+ assertTrue(fooNode instanceof INodeReference.DstReference);
+ List<DirectoryDiff> fooDiffs = ((INodeDirectoryWithSnapshot) fooNode
+ .asDirectory()).getDiffs().asList();
+ assertEquals(1, fooDiffs.size());
+ assertEquals("s1", fooDiffs.get(0).snapshot.getRoot().getLocalName());
+
+ // create snapshot on sdir2 and rename again
+ hdfs.createSnapshot(sdir2, "s3");
+ result = hdfs.rename(foo_dir2, foo_dir3);
+ assertFalse(result);
+
+ // check internal details again
+ dir2Node = (INodeDirectorySnapshottable) fsdir.getINode4Write(sdir2
+ .toString());
+ fooNode = fsdir.getINode4Write(foo_dir2.toString());
+ dir2Children = dir2Node.getChildrenList(null);
+ assertEquals(1, dir2Children.size());
+ dir2Diffs = dir2Node.getDiffs().asList();
+ assertEquals(2, dir2Diffs.size());
+ assertEquals("s2", Snapshot.getSnapshotName(dir2Diffs.get(0).snapshot));
+ assertEquals("s3", Snapshot.getSnapshotName(dir2Diffs.get(1).snapshot));
+
+ childrenDiff = dir2Diffs.get(0).getChildrenDiff();
+ assertEquals(0, childrenDiff.getList(ListType.DELETED).size());
+ assertEquals(1, childrenDiff.getList(ListType.CREATED).size());
+ assertTrue(childrenDiff.getList(ListType.CREATED).get(0) == fooNode);
+
+ childrenDiff = dir2Diffs.get(1).getChildrenDiff();
+ assertEquals(0, childrenDiff.getList(ListType.DELETED).size());
+ assertEquals(0, childrenDiff.getList(ListType.CREATED).size());
+
+ final Path foo_s3 = SnapshotTestHelper.getSnapshotPath(sdir2, "s3", "foo");
+ assertFalse(hdfs.exists(foo_s2));
+ assertTrue(hdfs.exists(foo_s3));
+
+ assertTrue(fooNode instanceof INodeReference.DstReference);
+ fooDiffs = ((INodeDirectoryWithSnapshot) fooNode.asDirectory()).getDiffs()
+ .asList();
+ assertEquals(2, fooDiffs.size());
+ assertEquals("s1", fooDiffs.get(0).snapshot.getRoot().getLocalName());
+ assertEquals("s3", fooDiffs.get(1).snapshot.getRoot().getLocalName());
+ }
}
