Author: szetszwo Date: Sat Feb 2 01:53:17 2013 New Revision: 1441680 URL: http://svn.apache.org/viewvc?rev=1441680&view=rev Log: HDFS-4464. Combine collectSubtreeBlocksAndClear with deleteDiffsForSnapshot and rename it to destroySubtreeAndCollectBlocks.
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/INodeFile.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/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/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 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -143,3 +143,6 @@ Branch-2802 Snapshot (Unreleased) HDFS-4361. When listing snapshottable directories, only return those where the user has permission to take snapshots. (Jing Zhao via szetszwo) + + HDFS-4464. Combine collectSubtreeBlocksAndClear with deleteDiffsForSnapshot + and rename it to destroySubtreeAndCollectBlocks. (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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -766,7 +766,7 @@ public class FSDirectory implements Clos INode rmdst = removedDst; removedDst = null; BlocksMapUpdateInfo collectedBlocks = new BlocksMapUpdateInfo(); - filesDeleted = rmdst.collectSubtreeBlocksAndClear(collectedBlocks); + filesDeleted = rmdst.destroySubtreeAndCollectBlocks(null, collectedBlocks); getFSNamesystem().removePathAndBlocks(src, collectedBlocks); } @@ -1009,19 +1009,18 @@ public class FSDirectory implements Clos } waitForReady(); long now = now(); - int filesRemoved; + final int filesRemoved; writeLock(); try { final INodesInPath inodesInPath = rootDir.getINodesInPath4Write( normalizePath(src), false); - final INode[] inodes = inodesInPath.getINodes(); - if (checkPathINodes(inodes, src) == 0) { + if (!deleteAllowed(inodesInPath, src) ) { filesRemoved = 0; } else { // Before removing the node, first check if the targetNode is for a // snapshottable dir with snapshots, or its descendants have // snapshottable dir with snapshots - INode targetNode = inodes[inodes.length-1]; + final INode targetNode = inodesInPath.getLastINode(); List<INodeDirectorySnapshottable> snapshottableDirs = new ArrayList<INodeDirectorySnapshottable>(); INode snapshotNode = hasSnapshot(targetNode, snapshottableDirs); @@ -1050,21 +1049,23 @@ public class FSDirectory implements Clos return true; } - private int checkPathINodes(INode[] inodes, String src) { + private static boolean deleteAllowed(final INodesInPath iip, + final String src) { + final INode[] inodes = iip.getINodes(); if (inodes == null || inodes.length == 0 || inodes[inodes.length - 1] == null) { if(NameNode.stateChangeLog.isDebugEnabled()) { NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedDelete: " + "failed to remove " + src + " because it does not exist"); } - return 0; + return false; } else if (inodes.length == 1) { // src is the root NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedDelete: " + "failed to remove " + src + " because the root is not allowed to be deleted"); - return 0; + return false; } - return inodes.length; + return true; } /** @@ -1100,16 +1101,11 @@ public class FSDirectory implements Clos throws UnresolvedLinkException, SnapshotAccessControlException { assert hasWriteLock(); BlocksMapUpdateInfo collectedBlocks = new BlocksMapUpdateInfo(); - int filesRemoved = 0; final INodesInPath inodesInPath = rootDir.getINodesInPath4Write( normalizePath(src), false); - final INode[] inodes = inodesInPath.getINodes(); - if (checkPathINodes(inodes, src) == 0) { - filesRemoved = 0; - } else { - filesRemoved = unprotectedDelete(inodesInPath, collectedBlocks, mtime); - } + final int filesRemoved = deleteAllowed(inodesInPath, src)? + unprotectedDelete(inodesInPath, collectedBlocks, mtime): 0; if (filesRemoved > 0) { getFSNamesystem().removePathAndBlocks(src, collectedBlocks); } @@ -1143,7 +1139,7 @@ public class FSDirectory implements Clos // 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.collectSubtreeBlocksAndClear(collectedBlocks); + : targetNode.destroySubtreeAndCollectBlocks(null, collectedBlocks); if (NameNode.stateChangeLog.isDebugEnabled()) { NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedDelete: " + targetNode.getFullPathName() + " is removed"); 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -314,16 +314,19 @@ public abstract class INode implements D } /** - * Collect all the blocks in all children of this INode. Count and return the - * number of files in the sub tree. Also clears references since this INode is - * deleted. + * Destroy the subtree under this inode and collect the blocks from the + * descents for further block deletion/update. If snapshot is null, the + * subtree resides in the current state; otherwise, the subtree resides in the + * given snapshot. The method also clears the references in the deleted inodes + * and remove the corresponding snapshot information, if there is any. * - * @param info - * Containing all the blocks collected from the children of this - * INode. These blocks later should be removed/updated in the - * blocksMap. + * @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. */ - abstract int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info); + abstract int destroySubtreeAndCollectBlocks(Snapshot snapshot, + BlocksMapUpdateInfo collectedBlocks); /** Compute {@link ContentSummary}. */ public final ContentSummary computeContentSummary() { 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -581,16 +581,16 @@ public class INodeDirectory extends INod } @Override - int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info) { - int total = 1; - if (children == null) { - return total; + public int destroySubtreeAndCollectBlocks(final Snapshot snapshot, + final BlocksMapUpdateInfo collectedBlocks) { + int total = 0; + for (INode child : getChildrenList(snapshot)) { + total += child.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks); + } + if (snapshot == null) { + parent = null; + children = null; } - for (INode child : children) { - total += child.collectSubtreeBlocksAndClear(info); - } - parent = null; - children = null; return total; } 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -236,11 +236,16 @@ public class INodeFile extends INode imp } @Override - public int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info) { + public int destroySubtreeAndCollectBlocks(final Snapshot snapshot, + final BlocksMapUpdateInfo collectedBlocks) { + if (snapshot != null) { + return 0; + } + parent = null; - if(blocks != null && info != null) { + if (blocks != null && collectedBlocks != null) { for (BlockInfo blk : blocks) { - info.addDeleteBlock(blk); + collectedBlocks.addDeleteBlock(blk); blk.setBlockCollection(null); } } 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -71,7 +71,8 @@ public class INodeSymlink extends INode } @Override - int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info) { + int destroySubtreeAndCollectBlocks(final Snapshot snapshot, + final BlocksMapUpdateInfo collectedBlocks) { return 1; } 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -60,7 +60,7 @@ abstract class AbstractINodeDiffList<N e * @return The SnapshotDiff containing the deleted snapshot. * Null if the snapshot with the given name does not exist. */ - final AbstractINodeDiff<N, D> deleteSnapshotDiff(Snapshot snapshot, + final D deleteSnapshotDiff(final Snapshot snapshot, final BlocksMapUpdateInfo collectedBlocks) { int snapshotIndex = Collections.binarySearch(diffs, snapshot); if (snapshotIndex < 0) { 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -273,38 +273,19 @@ public class INodeDirectorySnapshottable */ Snapshot removeSnapshot(String snapshotName, BlocksMapUpdateInfo collectedBlocks) throws SnapshotException { - final int indexOfOld = searchSnapshot(DFSUtil.string2Bytes(snapshotName)); - if (indexOfOld < 0) { + final int i = searchSnapshot(DFSUtil.string2Bytes(snapshotName)); + if (i < 0) { throw new SnapshotException("Cannot delete snapshot " + snapshotName + " from path " + this.getFullPathName() + ": the snapshot does not exist."); } else { - Snapshot snapshot = snapshotsByNames.remove(indexOfOld); - deleteDiffsForSnapshot(snapshot, this, collectedBlocks); + final Snapshot snapshot = snapshotsByNames.remove(i); + destroySubtreeAndCollectBlocks(snapshot, collectedBlocks); return snapshot; } } /** - * Recursively delete DirectoryDiff associated with the given snapshot under a - * directory - */ - private void deleteDiffsForSnapshot(Snapshot snapshot, INodeDirectory dir, - BlocksMapUpdateInfo collectedBlocks) { - if (dir instanceof INodeDirectoryWithSnapshot) { - INodeDirectoryWithSnapshot sdir = (INodeDirectoryWithSnapshot) dir; - sdir.getDiffs().deleteSnapshotDiff(snapshot, collectedBlocks); - } - ReadOnlyList<INode> children = dir.getChildrenList(null); - for (INode child : children) { - if (child instanceof INodeDirectory) { - deleteDiffsForSnapshot(snapshot, (INodeDirectory) child, - collectedBlocks); - } - } - } - - /** * Compute the difference between two snapshots (or a snapshot and the current * directory) of the directory. * 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -219,7 +219,8 @@ public class INodeDirectoryWithSnapshot @Override public void process(INode inode) { if (inode != null && inode instanceof INodeFile) { - ((INodeFile)inode).collectSubtreeBlocksAndClear(collectedBlocks); + ((INodeFile)inode).destroySubtreeAndCollectBlocks(null, + collectedBlocks); } } }); @@ -602,4 +603,14 @@ public class INodeDirectoryWithSnapshot } return dirNum; } + + @Override + public int destroySubtreeAndCollectBlocks(final Snapshot snapshot, + final BlocksMapUpdateInfo collectedBlocks) { + final int n = super.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks); + if (snapshot != null) { + getDiffs().deleteSnapshotDiff(snapshot, collectedBlocks); + } + 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -117,12 +117,16 @@ public class INodeFileUnderConstructionW } @Override - public int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info) { + 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.collectSubtreeBlocksAndClear(info); + return super.destroySubtreeAndCollectBlocks(null, collectedBlocks); } else { - return Util.collectSubtreeBlocksAndClear(this, info); + return Util.collectSubtreeBlocksAndClear(this, collectedBlocks); } } } 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=1441680&r1=1441679&r2=1441680&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 Sat Feb 2 01:53:17 2013 @@ -100,12 +100,16 @@ public class INodeFileWithSnapshot exten } @Override - public int collectSubtreeBlocksAndClear(BlocksMapUpdateInfo info) { + 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.collectSubtreeBlocksAndClear(info); + return super.destroySubtreeAndCollectBlocks(snapshot, collectedBlocks); } else { - return Util.collectSubtreeBlocksAndClear(this, info); + return Util.collectSubtreeBlocksAndClear(this, collectedBlocks); } } }