From 2363bad7934770bb87250e324d3f42cf3aca462d Mon Sep 17 00:00:00 2001 From: Jing Zhao Date: Thu, 4 Sep 2014 16:12:44 -0700 Subject: [PATCH] HDFS-6996. SnapshotDiff report can hit IndexOutOfBoundsException when there are nested renamed directory/file. Contributed by Jing Zhao. --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 7 ++-- .../hdfs/server/namenode/FSDirectory.java | 5 +-- .../hdfs/server/namenode/INodeReference.java | 4 +-- .../DirectorySnapshottableFeature.java | 2 +- .../snapshot/TestSnapshotDiffReport.java | 36 +++++++++++++++++++ 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index ba45414870d..d2731a8f3db 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -329,8 +329,11 @@ Release 2.6.0 - UNRELEASED HDFS-6942. Fix typos in log messages. (Ray Chiang via wheat9) - HDFS-6848. Lack of synchronization on access to datanodeUuid in - DataStorage#format(). (Xiaoyu Yao via Arpit Agarwal) + HDFS-6848. Lack of synchronization on access to datanodeUuid in + DataStorage#format(). (Xiaoyu Yao via Arpit Agarwal) + + HDFS-6996. SnapshotDiff report can hit IndexOutOfBoundsException when there + are nested renamed directory/file. (jing9) BREAKDOWN OF HDFS-6134 AND HADOOP-10150 SUBTASKS AND RELATED JIRAS diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index ba154ca9df3..7b5f485d0cd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -428,7 +428,8 @@ public class FSDirectory implements Closeable { /** * @throws SnapshotAccessControlException * @see #unprotectedRenameTo(String, String, long) - * @deprecated Use {@link #renameTo(String, String, boolean, Rename...)} + * @deprecated Use {@link #renameTo(String, String, long, + * BlocksMapUpdateInfo, Rename...)} */ @Deprecated boolean renameTo(String src, String dst, long mtime) @@ -479,7 +480,7 @@ public class FSDirectory implements Closeable { * @throws QuotaExceededException if the operation violates any quota limit * @throws FileAlreadyExistsException if the src is a symlink that points to dst * @throws SnapshotAccessControlException if path is in RO snapshot - * @deprecated See {@link #renameTo(String, String, boolean, Rename...)} + * @deprecated See {@link #renameTo(String, String, long, BlocksMapUpdateInfo, Rename...)} */ @Deprecated boolean unprotectedRenameTo(String src, String dst, long timestamp) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java index 05e144d21ba..9bd2ad0ebd5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java @@ -28,7 +28,6 @@ import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; import org.apache.hadoop.hdfs.server.namenode.snapshot.DirectoryWithSnapshotFeature; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; -import org.apache.hadoop.hdfs.server.namenode.XAttrFeature; import com.google.common.base.Preconditions; @@ -450,7 +449,8 @@ public abstract class INodeReference extends INode { end = mid; } } - if (withNameList.get(start).lastSnapshotId >= snapshotId) { + if (start < withNameList.size() && + withNameList.get(start).lastSnapshotId >= snapshotId) { return withNameList.get(start); } else { return this.getParentReference(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java index ae70b885d8b..42dc5e7c232 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java @@ -61,7 +61,7 @@ public class DirectorySnapshottableFeature extends DirectoryWithSnapshotFeature /** * Snapshots of this directory in ascending order of snapshot names. * Note that snapshots in ascending order of snapshot id are stored in - * {@link INodeDirectoryWithSnapshot}.diffs (a private field). + * {@link DirectoryWithSnapshotFeature}.diffs (a private field). */ private final List snapshotsByNames = new ArrayList(); /** Number of snapshots allowed. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java index 0cc318b5ad2..786f9249f69 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java @@ -492,4 +492,40 @@ public class TestSnapshotDiffReport { new DiffReportEntry(DiffType.RENAME, DFSUtil.string2Bytes("foo"), DFSUtil.string2Bytes("bar"))); } + + /** + * Nested renamed dir/file and the withNameList in the WithCount node of the + * parental directory is empty due to snapshot deletion. See HDFS-6996 for + * details. + */ + @Test + public void testDiffReportWithRenameAndSnapshotDeletion() throws Exception { + final Path root = new Path("/"); + final Path foo = new Path(root, "foo"); + final Path bar = new Path(foo, "bar"); + DFSTestUtil.createFile(hdfs, bar, BLOCKSIZE, REPLICATION, seed); + + SnapshotTestHelper.createSnapshot(hdfs, root, "s0"); + // rename /foo to /foo2 + final Path foo2 = new Path(root, "foo2"); + hdfs.rename(foo, foo2); + // now /foo/bar becomes /foo2/bar + final Path bar2 = new Path(foo2, "bar"); + + // delete snapshot s0 so that the withNameList inside of the WithCount node + // of foo becomes empty + hdfs.deleteSnapshot(root, "s0"); + + // create snapshot s1 and rename bar again + SnapshotTestHelper.createSnapshot(hdfs, root, "s1"); + final Path bar3 = new Path(foo2, "bar-new"); + hdfs.rename(bar2, bar3); + + // we always put modification on the file before rename + verifyDiffReport(root, "s1", "", + new DiffReportEntry(DiffType.MODIFY, DFSUtil.string2Bytes("")), + new DiffReportEntry(DiffType.MODIFY, DFSUtil.string2Bytes("foo2")), + new DiffReportEntry(DiffType.RENAME, DFSUtil.string2Bytes("foo2/bar"), + DFSUtil.string2Bytes("foo2/bar-new"))); + } } \ No newline at end of file