From 3a812e9f3ae66cfa525faceb5af9085a4cc37232 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Wed, 24 Apr 2013 02:11:18 +0000 Subject: [PATCH] HDFS-4738. Changes AbstractINodeDiff to implement Comparable, and fix javadoc and other warnings. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-2802@1471228 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-hdfs/CHANGES.HDFS-2802.txt | 21 +++++++++++++++ .../apache/hadoop/hdfs/client/HdfsAdmin.java | 4 +-- .../SnapshottableDirectoryStatus.java | 25 ++++++++++------- .../hadoop/hdfs/server/namenode/Content.java | 3 +-- .../hdfs/server/namenode/INodeDirectory.java | 5 +--- .../namenode/snapshot/AbstractINodeDiff.java | 6 ++--- .../snapshot/AbstractINodeDiffList.java | 19 +++++++------ .../snapshot/INodeDirectoryWithSnapshot.java | 27 +++++++++---------- .../server/namenode/snapshot/Snapshot.java | 20 +++++++++++--- .../snapshot/SnapshotFSImageFormat.java | 2 -- .../namenode/snapshot/SnapshotManager.java | 7 +++-- .../org/apache/hadoop/hdfs/util/Diff.java | 22 +++++++-------- 12 files changed, 96 insertions(+), 65 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt index a1bfe0f24bd..e9291cece3c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt @@ -1,3 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + Branch-2802 Snapshot (Unreleased) HDFS-4076. Support snapshot of single files. (szetszwo) @@ -272,3 +290,6 @@ Branch-2802 Snapshot (Unreleased) HDFS-4735. DisallowSnapshot throws IllegalStateException for nested snapshottable directories. (Jing Zhao via szetszwo) + + HDFS-4738. Changes AbstractINodeDiff to implement Comparable, and + fix javadoc and other warnings. (szetszwo) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java index b80adcc339b..03ff7f45c3c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java @@ -108,7 +108,7 @@ public class HdfsAdmin { /** * Allow snapshot on a directory. - * @param the path of the directory where snapshots will be taken + * @param path The path of the directory where snapshots will be taken. */ public void allowSnapshot(Path path) throws IOException { dfs.allowSnapshot(path); @@ -116,7 +116,7 @@ public class HdfsAdmin { /** * Disallow snapshot on a directory. - * @param path of the snapshottable directory. + * @param path The path of the snapshottable directory. */ public void disallowSnapshot(Path path) throws IOException { dfs.disallowSnapshot(path); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshottableDirectoryStatus.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshottableDirectoryStatus.java index 87808aa4dd3..c1f9388eef3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshottableDirectoryStatus.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshottableDirectoryStatus.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdfs.protocol; import java.io.PrintStream; import java.text.SimpleDateFormat; +import java.util.Comparator; import java.util.Date; import org.apache.hadoop.fs.Path; @@ -28,8 +29,20 @@ import org.apache.hadoop.hdfs.DFSUtil; /** * Metadata about a snapshottable directory */ -public class SnapshottableDirectoryStatus - implements Comparable { +public class SnapshottableDirectoryStatus { + /** Compare the statuses by full paths. */ + public static final Comparator COMPARATOR + = new Comparator() { + @Override + public int compare(SnapshottableDirectoryStatus left, + SnapshottableDirectoryStatus right) { + int d = DFSUtil.compareBytes(left.parentFullPath, right.parentFullPath); + return d != 0? d + : DFSUtil.compareBytes(left.dirStatus.getLocalNameInBytes(), + right.dirStatus.getLocalNameInBytes()); + } + }; + /** Basic information of the snapshottable directory */ private HdfsFileStatus dirStatus; @@ -145,12 +158,4 @@ public class SnapshottableDirectoryStatus private static int maxLength(int n, Object value) { return Math.max(n, String.valueOf(value).length()); } - - @Override - public int compareTo(SnapshottableDirectoryStatus that) { - int d = DFSUtil.compareBytes(this.parentFullPath, that.parentFullPath); - return d != 0? d - : DFSUtil.compareBytes(this.dirStatus.getLocalNameInBytes(), - that.dirStatus.getLocalNameInBytes()); - } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Content.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Content.java index 893a7620218..bb700051ccc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Content.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Content.java @@ -20,8 +20,7 @@ package org.apache.hadoop.hdfs.server.namenode; import org.apache.hadoop.hdfs.util.EnumCounters; /** - * The content types such as file, directory and symlink to be computed - * in {@link INode#computeContentSummary(CountsMap)}. + * The content types such as file, directory and symlink to be computed. */ public enum Content { /** The number of files. */ 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 c281667bafa..1574783ede7 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 @@ -474,10 +474,7 @@ public class INodeDirectory extends INodeWithAdditionalFields { clearChildren(); } - /** - * Call {@link INode#cleanSubtree(SnapshotDeletionInfo, BlocksMapUpdateInfo)} - * recursively down the subtree. - */ + /** Call cleanSubtree(..) recursively down the subtree. */ public Quota.Counts cleanSubtreeRecursively(final Snapshot snapshot, Snapshot prior, final BlocksMapUpdateInfo collectedBlocks, final List removedINodes) throws QuotaExceededException { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java index 8218fa2ab9c..e6a41509891 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiff.java @@ -48,7 +48,7 @@ import com.google.common.base.Preconditions; */ abstract class AbstractINodeDiff> - implements Comparable { + implements Comparable { /** The snapshot will be obtained after this diff is applied. */ Snapshot snapshot; @@ -72,8 +72,8 @@ abstract class AbstractINodeDiff removedINodes) { - int snapshotIndex = Collections.binarySearch(diffs, snapshot); + int snapshotIndex = Collections.binarySearch(diffs, snapshot.getId()); Quota.Counts counts = Quota.Counts.newInstance(); D removed = null; @@ -151,7 +151,7 @@ abstract class AbstractINodeDiffList= 0) { // exact match return diffs.get(i); @@ -197,23 +197,22 @@ abstract class AbstractINodeDiffList 0) { - earlierSnapshot = toSnapshot; - laterSnapshot = fromSnapshot; + earlier = toSnapshot; + later = fromSnapshot; } - boolean modified = diffs.changedBetweenSnapshots(earlierSnapshot, - laterSnapshot); + boolean modified = diffs.changedBetweenSnapshots(earlier, + later); if (!modified) { return false; } final List difflist = diffs.asList(); final int size = difflist.size(); - int earlierDiffIndex = Collections.binarySearch(difflist, earlierSnapshot); - int laterDiffIndex = laterSnapshot == null ? size : Collections - .binarySearch(difflist, laterSnapshot); + int earlierDiffIndex = Collections.binarySearch(difflist, earlier.getId()); + int laterDiffIndex = later == null ? size : Collections + .binarySearch(difflist, later.getId()); earlierDiffIndex = earlierDiffIndex < 0 ? (-earlierDiffIndex - 1) : earlierDiffIndex; laterDiffIndex = laterDiffIndex < 0 ? (-laterDiffIndex - 1) @@ -507,10 +507,8 @@ public class INodeDirectoryWithSnapshot extends INodeDirectoryWithQuota { if (dirMetadataChanged == false && sdiff.snapshotINode != null) { if (dirCopy == null) { dirCopy = sdiff.snapshotINode; - } else { - if (!dirCopy.metadataEquals(sdiff.snapshotINode)) { - dirMetadataChanged = true; - } + } else if (!dirCopy.metadataEquals(sdiff.snapshotINode)) { + dirMetadataChanged = true; } } } @@ -524,8 +522,9 @@ public class INodeDirectoryWithSnapshot extends INodeDirectoryWithQuota { } } return !dirCopy.metadataEquals(this); + } else { + return false; } - return false; } /** Diff list sorted by snapshot IDs, i.e. in chronological order. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java index 24046159638..cdb2c77ddee 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java @@ -71,18 +71,32 @@ public class Snapshot implements Comparable { } /** - * Compare snapshot IDs. Null indicates the current status thus is greater - * than non-null snapshots. + * Compare snapshot with IDs, where null indicates the current status thus + * is greater than any non-null snapshot. */ public static final Comparator ID_COMPARATOR = new Comparator() { @Override public int compare(Snapshot left, Snapshot right) { + return ID_INTEGER_COMPARATOR.compare( + left == null? null: left.getId(), + right == null? null: right.getId()); + } + }; + + /** + * Compare snapshot with IDs, where null indicates the current status thus + * is greater than any non-null ID. + */ + public static final Comparator ID_INTEGER_COMPARATOR + = new Comparator() { + @Override + public int compare(Integer left, Integer right) { // null means the current state, thus should be the largest if (left == null) { return right == null? 0: 1; } else { - return right == null? -1: left.id - right.id; + return right == null? -1: left - right; } } }; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotFSImageFormat.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotFSImageFormat.java index 2c7b32b6749..bbb0c41e7c6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotFSImageFormat.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotFSImageFormat.java @@ -229,8 +229,6 @@ public class SnapshotFSImageFormat { * Load the {@link SnapshotDiff} list for the INodeDirectoryWithSnapshot * directory. * @param dir The snapshottable directory for loading. - * @param numSnapshotDiffs The number of {@link SnapshotDiff} that the - * directory has. * @param in The {@link DataInput} instance to read. * @param loader The {@link Loader} instance that this loading procedure is * using. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java index 106cc100569..325be95ce19 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java @@ -214,7 +214,7 @@ public class SnapshotManager implements SnapshotStats { /** * Write {@link #snapshotCounter}, {@link #numSnapshots}, - * {@link #numSnapshottableDirs} and all snapshots to the DataOutput. + * and all snapshots to the DataOutput. */ public void write(DataOutput out) throws IOException { out.writeInt(snapshotCounter); @@ -230,7 +230,7 @@ public class SnapshotManager implements SnapshotStats { /** * Read values of {@link #snapshotCounter}, {@link #numSnapshots}, and - * {@link #numSnapshottableDirs} and all snapshots from the DataInput + * all snapshots from the DataInput */ public Map read(DataInput in, FSImageFormat.Loader loader ) throws IOException { @@ -273,7 +273,7 @@ public class SnapshotManager implements SnapshotStats { statusList.add(status); } } - Collections.sort(statusList); + Collections.sort(statusList, SnapshottableDirectoryStatus.COMPARATOR); return statusList.toArray( new SnapshottableDirectoryStatus[statusList.size()]); } @@ -302,5 +302,4 @@ public class SnapshotManager implements SnapshotStats { return snapshotRoot.computeDiff(from, to); } - } \ No newline at end of file diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java index c79dacc99f6..fff53fb5e05 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java @@ -104,7 +104,7 @@ public class Diff> { } /** - * Undo information for some operations such as {@link Diff#delete(E)} + * Undo information for some operations such as delete(E) * and {@link Diff#modify(Element, Element)}. */ public static class UndoInfo { @@ -215,8 +215,8 @@ public class Diff> { } /** - * Undo the previous {@link #create(E)} operation. Note that the behavior is - * undefined if the previous operation is not {@link #create(E)}. + * Undo the previous create(E) operation. Note that the behavior is + * undefined if the previous operation is not create(E). */ public void undoCreate(final E element, final int insertionPoint) { remove(created, insertionPoint, element); @@ -242,8 +242,8 @@ public class Diff> { } /** - * Undo the previous {@link #delete(E)} operation. Note that the behavior is - * undefined if the previous operation is not {@link #delete(E)}. + * Undo the previous delete(E) operation. Note that the behavior is + * undefined if the previous operation is not delete(E). */ public void undoDelete(final E element, final UndoInfo undoInfo) { final int c = undoInfo.createdInsertionPoint; @@ -285,8 +285,8 @@ public class Diff> { } /** - * Undo the previous {@link #modify(E, E)} operation. Note that the behavior - * is undefined if the previous operation is not {@link #modify(E, E)}. + * Undo the previous modify(E, E) operation. Note that the behavior + * is undefined if the previous operation is not modify(E, E). */ public void undoModify(final E oldElement, final E newElement, final UndoInfo undoInfo) { @@ -383,24 +383,24 @@ public class Diff> { * 1.2 (0, d') in this diff: put in c-list --> (c, d') * 1.3 (c', d') in this diff: impossible * 1.4 (0, 0) in this diff: put in c-list --> (c, 0) - * This is the same logic as {@link #create(E)}. + * This is the same logic as create(E). * * 2. For (0, d) in the posterior diff, * 2.1 (c', 0) in this diff: remove from c-list --> (0, 0) * 2.2 (0, d') in this diff: impossible * 2.3 (c', d') in this diff: remove from c-list --> (0, d') * 2.4 (0, 0) in this diff: put in d-list --> (0, d) - * This is the same logic as {@link #delete(E)}. + * This is the same logic as delete(E). * * 3. For (c, d) in the posterior diff, * 3.1 (c', 0) in this diff: replace the element in c-list --> (c, 0) * 3.2 (0, d') in this diff: impossible * 3.3 (c', d') in this diff: replace the element in c-list --> (c, d') * 3.4 (0, 0) in this diff: put in c-list and d-list --> (c, d) - * This is the same logic as {@link #modify(E, E)}. + * This is the same logic as modify(E, E). * * - * @param the posterior diff to combine with. + * @param posterior The posterior diff to combine with. * @param deletedProcesser * process the deleted/overwritten elements in case 2.1, 2.3, 3.1 and 3.3. */