From d42d0860cb670c8284bb298029cd6f8f59db9510 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Thu, 14 Feb 2013 22:00:36 +0000 Subject: [PATCH] HDFS-4500. Refactor snapshot INode methods. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-2802@1446355 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-hdfs/CHANGES.HDFS-2802.txt | 2 + .../hadoop/hdfs/server/namenode/INode.java | 65 ++++++++++++------- .../hdfs/server/namenode/INodeDirectory.java | 34 +++++++--- .../hdfs/server/namenode/INodeFile.java | 11 +++- .../snapshot/AbstractINodeDiffList.java | 10 ++- .../snapshot/INodeDirectorySnapshottable.java | 11 ++-- .../snapshot/INodeDirectoryWithSnapshot.java | 37 ++--------- ...NodeFileUnderConstructionWithSnapshot.java | 44 ++----------- .../snapshot/INodeFileWithSnapshot.java | 44 ++----------- 9 files changed, 104 insertions(+), 154 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt index 1921225cb6c..2d80070dff7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt @@ -155,3 +155,5 @@ Branch-2802 Snapshot (Unreleased) HDFS-4480. Eliminate the file snapshot circular linked list. (szetszwo) HDFS-4481. Change fsimage to support snapshot file diffs. (szetszwo) + + HDFS-4500. Refactor snapshot INode methods. (szetszwo) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index 373dac9959f..fe7e3611a92 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -48,17 +48,6 @@ import com.google.common.primitives.SignedBytes; public abstract class INode implements Diff.Element { public static final Log LOG = LogFactory.getLog(INode.class); - /** A pair of objects. */ - public static class Pair { - public final L left; - public final R right; - - public Pair(L left, R right) { - this.left = left; - this.right = right; - } - } - /** Wrapper of two counters for namespace consumed and diskspace consumed. */ static class DirCounts { /** namespace count */ @@ -177,12 +166,12 @@ public abstract class INode implements Diff.Element { this.permission = that.permission; } /** Get the {@link PermissionStatus} */ - public PermissionStatus getPermissionStatus(Snapshot snapshot) { + public final PermissionStatus getPermissionStatus(Snapshot snapshot) { return new PermissionStatus(getUserName(snapshot), getGroupName(snapshot), getFsPermission(snapshot)); } /** The same as getPermissionStatus(null). */ - public PermissionStatus getPermissionStatus() { + public final PermissionStatus getPermissionStatus() { return getPermissionStatus(null); } private INode updatePermissionStatus(PermissionStatusFormat f, long n, @@ -197,12 +186,16 @@ public abstract class INode implements Diff.Element { * otherwise, get the result from the current inode. * @return user name */ - public String getUserName(Snapshot snapshot) { + public final String getUserName(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).getUserName(); + } + int n = (int)PermissionStatusFormat.USER.retrieve(permission); return SerialNumberManager.INSTANCE.getUser(n); } /** The same as getUserName(null). */ - public String getUserName() { + public final String getUserName() { return getUserName(null); } /** Set user */ @@ -216,12 +209,16 @@ public abstract class INode implements Diff.Element { * otherwise, get the result from the current inode. * @return group name */ - public String getGroupName(Snapshot snapshot) { + public final String getGroupName(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).getGroupName(); + } + int n = (int)PermissionStatusFormat.GROUP.retrieve(permission); return SerialNumberManager.INSTANCE.getGroup(n); } /** The same as getGroupName(null). */ - public String getGroupName() { + public final String getGroupName() { return getGroupName(null); } /** Set group */ @@ -235,12 +232,16 @@ public abstract class INode implements Diff.Element { * otherwise, get the result from the current inode. * @return permission. */ - public FsPermission getFsPermission(Snapshot snapshot) { + public final FsPermission getFsPermission(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).getFsPermission(); + } + return new FsPermission( (short)PermissionStatusFormat.MODE.retrieve(permission)); } /** The same as getFsPermission(null). */ - public FsPermission getFsPermission() { + public final FsPermission getFsPermission() { return getFsPermission(null); } protected short getFsPermissionShort() { @@ -252,6 +253,14 @@ public abstract class INode implements Diff.Element { return updatePermissionStatus(PermissionStatusFormat.MODE, mode, latest); } + /** + * @return if the given snapshot is null, return this; + * otherwise return the corresponding snapshot inode. + */ + public INode getSnapshotINode(final Snapshot snapshot) { + return this; + } + /** Is this inode in the latest snapshot? */ public final boolean isInLatestSnapshot(final Snapshot latest) { return latest != null @@ -427,7 +436,11 @@ public abstract class INode implements Diff.Element { * otherwise, get the result from the current inode. * @return modification time. */ - public long getModificationTime(Snapshot snapshot) { + public final long getModificationTime(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).modificationTime; + } + return this.modificationTime; } @@ -464,12 +477,16 @@ public abstract class INode implements Diff.Element { * otherwise, get the result from the current inode. * @return access time */ - public long getAccessTime(Snapshot snapshot) { + public final long getAccessTime(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).accessTime; + } + return accessTime; } /** The same as getAccessTime(null). */ - public long getAccessTime() { + public final long getAccessTime() { return getAccessTime(null); } @@ -598,9 +615,7 @@ public abstract class INode implements Diff.Element { out.print(getObjectString()); out.print("), parent="); out.print(parent == null? null: parent.getLocalName() + "/"); - out.print(", permission=" + getFsPermission(snapshot)); - out.print(", group=" + getGroupName(snapshot)); - out.print(", user=" + getUserName(snapshot)); + out.print(", " + getPermissionStatus(snapshot)); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java index b1b9e6a345f..81852297bb8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java @@ -834,21 +834,20 @@ public class INodeDirectory extends INode { prefix.setLength(prefix.length() - 2); prefix.append(" "); } - dumpTreeRecursively(out, prefix, - new Iterable>() { + dumpTreeRecursively(out, prefix, new Iterable() { final Iterator i = getChildrenList(snapshot).iterator(); @Override - public Iterator> iterator() { - return new Iterator>() { + public Iterator iterator() { + return new Iterator() { @Override public boolean hasNext() { return i.hasNext(); } @Override - public Pair next() { - return new Pair(i.next(), snapshot); + public SnapshotAndINode next() { + return new SnapshotAndINode(snapshot, i.next()); } @Override @@ -867,14 +866,29 @@ public class INodeDirectory extends INode { */ @VisibleForTesting protected static void dumpTreeRecursively(PrintWriter out, - StringBuilder prefix, Iterable> subs) { + StringBuilder prefix, Iterable subs) { if (subs != null) { - for(final Iterator> i = subs.iterator(); i.hasNext();) { - final Pair pair = i.next(); + for(final Iterator i = subs.iterator(); i.hasNext();) { + final SnapshotAndINode pair = i.next(); prefix.append(i.hasNext()? DUMPTREE_EXCEPT_LAST_ITEM: DUMPTREE_LAST_ITEM); - pair.left.dumpTreeRecursively(out, prefix, pair.right); + pair.inode.dumpTreeRecursively(out, prefix, pair.snapshot); prefix.setLength(prefix.length() - 2); } } } + + /** A pair of Snapshot and INode objects. */ + protected static class SnapshotAndINode { + public final Snapshot snapshot; + public final INode inode; + + public SnapshotAndINode(Snapshot snapshot, INode inode) { + this.snapshot = snapshot; + this.inode = inode; + } + + public SnapshotAndINode(Snapshot snapshot) { + this(snapshot, snapshot.getRoot()); + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java index 9e7e87700c5..b5b970cc52f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java @@ -122,6 +122,11 @@ public class INodeFile extends INode implements BlockCollection { clientName, clientMachine, clientNode); } + @Override + public INodeFile getSnapshotINode(final Snapshot snapshot) { + return this; + } + @Override public INodeFile recordModification(final Snapshot latest) { return isInLatestSnapshot(latest)? @@ -141,7 +146,11 @@ public class INodeFile extends INode implements BlockCollection { } /** @return the replication factor of the file. */ - public short getFileReplication(Snapshot snapshot) { + public final short getFileReplication(Snapshot snapshot) { + if (snapshot != null) { + return getSnapshotINode(snapshot).getFileReplication(); + } + return HeaderFormat.getReplication(header); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java index 16590afc935..a476071c02d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java @@ -145,9 +145,15 @@ abstract class AbstractINodeDiffList>() { + dumpTreeRecursively(out, prefix, new Iterable() { @Override - public Iterator> iterator() { - return new Iterator>() { + public Iterator iterator() { + return new Iterator() { final Iterator i = getDiffs().iterator(); private DirectoryDiff next = findNext(); @@ -427,10 +427,9 @@ public class INodeDirectorySnapshottable extends INodeDirectoryWithSnapshot { } @Override - public Pair next() { + public SnapshotAndINode next() { final Snapshot s = next.snapshot; - final Pair pair = - new Pair(s.getRoot(), s); + final SnapshotAndINode pair = new SnapshotAndINode(s); next = findNext(); return pair; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java index 965d34bca94..81a5cf36ab4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry; import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffType; import org.apache.hadoop.hdfs.server.namenode.FSImageSerialization; @@ -412,6 +411,11 @@ public class INodeDirectoryWithSnapshot extends INodeDirectoryWithQuota { return diffs; } + @Override + public INodeDirectory getSnapshotINode(Snapshot snapshot) { + return diffs.getSnapshotINode(snapshot, this); + } + @Override public INodeDirectoryWithSnapshot recordModification(final Snapshot latest) { return isInLatestSnapshot(latest)? @@ -507,37 +511,6 @@ public class INodeDirectoryWithSnapshot extends INodeDirectoryWithQuota { return diff != null? diff.getChild(name, true, this): super.getChild(name, null); } - @Override - public String getUserName(Snapshot snapshot) { - final INodeDirectory inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getUserName(): super.getUserName(null); - } - - @Override - public String getGroupName(Snapshot snapshot) { - final INodeDirectory inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getGroupName(): super.getGroupName(null); - } - - @Override - public FsPermission getFsPermission(Snapshot snapshot) { - final INodeDirectory inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getFsPermission(): super.getFsPermission(null); - } - - @Override - public long getAccessTime(Snapshot snapshot) { - final INodeDirectory inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getAccessTime(): super.getAccessTime(null); - } - - @Override - public long getModificationTime(Snapshot snapshot) { - final INodeDirectory inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getModificationTime() - : super.getModificationTime(null); - } - @Override public String toDetailString() { return super.toDetailString() + ", " + diffs; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java index 4abecb5cb3e..797916c514f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileUnderConstructionWithSnapshot.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hdfs.server.namenode.snapshot; 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; @@ -86,6 +85,11 @@ public class INodeFileUnderConstructionWithSnapshot return isCurrentFileDeleted; } + @Override + public INodeFile getSnapshotINode(Snapshot snapshot) { + return diffs.getSnapshotINode(snapshot, this); + } + @Override public INodeFileUnderConstructionWithSnapshot recordModification( final Snapshot latest) { @@ -105,13 +109,6 @@ public class INodeFileUnderConstructionWithSnapshot return diffs; } - @Override - public short getFileReplication(Snapshot snapshot) { - final INodeFile inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getFileReplication() - : super.getFileReplication(null); - } - @Override public short getBlockReplication() { return Util.getBlockReplication(this); @@ -141,37 +138,6 @@ public class INodeFileUnderConstructionWithSnapshot 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() diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java index 8d203305bc1..cae834e1033 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeFileWithSnapshot.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hdfs.server.namenode.snapshot; 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; @@ -57,6 +56,11 @@ public class INodeFileWithSnapshot extends INodeFile return isCurrentFileDeleted; } + @Override + public INodeFile getSnapshotINode(Snapshot snapshot) { + return diffs.getSnapshotINode(snapshot, this); + } + @Override public INodeFileWithSnapshot recordModification(final Snapshot latest) { if (isInLatestSnapshot(latest)) { @@ -75,13 +79,6 @@ public class INodeFileWithSnapshot extends INodeFile return diffs; } - @Override - public short getFileReplication(Snapshot snapshot) { - final INodeFile inode = diffs.getSnapshotINode(snapshot); - return inode != null? inode.getFileReplication() - : super.getFileReplication(null); - } - @Override public short getBlockReplication() { return Util.getBlockReplication(this); @@ -111,37 +108,6 @@ public class INodeFileWithSnapshot extends INodeFile 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()