HDFS-4738. Changes AbstractINodeDiff to implement Comparable<Integer>, 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-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<Integer>, and
+  fix javadoc and other warnings.  (szetszwo)

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);

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
+public class SnapshottableDirectoryStatus {
-    implements Comparable<SnapshottableDirectoryStatus> {
+  /** Compare the statuses by full paths. */
+  public static final Comparator<SnapshottableDirectoryStatus> COMPARATOR
+      = new Comparator<SnapshottableDirectoryStatus>() {
+    @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());
-  }
 }

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
+ * The content types such as file, directory and symlink to be computed.
- * in {@link INode#computeContentSummary(CountsMap)}.
  */
 public enum Content {
   /** The number of files. */

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 cleanSubtree(..) recursively down the subtree. */
-   * Call {@link INode#cleanSubtree(SnapshotDeletionInfo, BlocksMapUpdateInfo)}
-   * recursively down the subtree.
-   */
   public Quota.Counts cleanSubtreeRecursively(final Snapshot snapshot,
       Snapshot prior, final BlocksMapUpdateInfo collectedBlocks,
       final List<INode> removedINodes) throws QuotaExceededException {

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<N extends INode,
                                  D extends AbstractINodeDiff<N, D>>
-    implements Comparable<Snapshot> {
+    implements Comparable<Integer> {
 
   /** The snapshot will be obtained after this diff is applied. */
   Snapshot snapshot;
@@ -72,8 +72,8 @@ abstract class AbstractINodeDiff<N extends INode,
 
   /** Compare diffs with snapshot ID. */
   @Override
-  public final int compareTo(final Snapshot that) {
+  public final int compareTo(final Integer that) {
-    return Snapshot.ID_COMPARATOR.compare(this.snapshot, that);
+    return Snapshot.ID_INTEGER_COMPARATOR.compare(this.snapshot.getId(), that);
   }
 
   /** @return the snapshot object of this diff. */

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java

@@ -69,7 +69,7 @@ abstract class AbstractINodeDiffList<N extends INode,
   final Quota.Counts deleteSnapshotDiff(final Snapshot snapshot,
       Snapshot prior, final N currentINode,
       final BlocksMapUpdateInfo collectedBlocks, final List<INode> 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<N extends INode,
     if (anchor == null) {
       return getLastSnapshot();
     }
-    final int i = Collections.binarySearch(diffs, anchor);
+    final int i = Collections.binarySearch(diffs, anchor.getId());
     if (i == -1 || i == 0) {
       return null;
     } else {
@@ -182,7 +182,7 @@ abstract class AbstractINodeDiffList<N extends INode,
       // snapshot == null means the current state, therefore, return null.
       return null;
     }
-    final int i = Collections.binarySearch(diffs, snapshot);
+    final int i = Collections.binarySearch(diffs, snapshot.getId());
     if (i >= 0) {
       // exact match
       return diffs.get(i);
@@ -197,23 +197,22 @@ abstract class AbstractINodeDiffList<N extends INode,
   
   /**
    * Check if changes have happened between two snapshots.
-   * @param earlierSnapshot The snapshot taken earlier
+   * @param earlier The snapshot taken earlier
-   * @param laterSnapshot The snapshot taken later
+   * @param later The snapshot taken later
    * @return Whether or not modifications (including diretory/file metadata
    *         change, file creation/deletion under the directory) have happened
    *         between snapshots.
    */
-  final boolean changedBetweenSnapshots(Snapshot earlierSnapshot,
+  final boolean changedBetweenSnapshots(Snapshot earlier, Snapshot later) {
-      Snapshot laterSnapshot) {
     final int size = diffs.size();
-    int earlierDiffIndex = Collections.binarySearch(diffs, earlierSnapshot);
+    int earlierDiffIndex = Collections.binarySearch(diffs, earlier.getId());
     if (-earlierDiffIndex - 1 == size) {
       // if the earlierSnapshot is after the latest SnapshotDiff stored in
       // diffs, no modification happened after the earlierSnapshot
       return false;
     }
-    if (laterSnapshot != null) {
+    if (later != null) {
-      int laterDiffIndex = Collections.binarySearch(diffs, laterSnapshot);
+      int laterDiffIndex = Collections.binarySearch(diffs, later.getId());
       if (laterDiffIndex == -1 || laterDiffIndex == 0) {
         // if the laterSnapshot is the earliest SnapshotDiff stored in diffs, or
         // before it, no modification happened before the laterSnapshot

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectoryWithSnapshot.java

@@ -476,24 +476,24 @@ public class INodeDirectoryWithSnapshot extends INodeDirectoryWithQuota {
    */
   boolean computeDiffBetweenSnapshots(Snapshot fromSnapshot,
       Snapshot toSnapshot, ChildrenDiff diff) {
-    Snapshot earlierSnapshot = fromSnapshot;
+    Snapshot earlier = fromSnapshot;
-    Snapshot laterSnapshot = toSnapshot;
+    Snapshot later = toSnapshot;
     if (Snapshot.ID_COMPARATOR.compare(fromSnapshot, toSnapshot) > 0) {
-      earlierSnapshot = toSnapshot;
+      earlier = toSnapshot;
-      laterSnapshot = fromSnapshot;
+      later = fromSnapshot;
     }
     
-    boolean modified = diffs.changedBetweenSnapshots(earlierSnapshot,
+    boolean modified = diffs.changedBetweenSnapshots(earlier,
-        laterSnapshot);
+        later);
     if (!modified) {
       return false;
     }
     
     final List<DirectoryDiff> difflist = diffs.asList();
     final int size = difflist.size();
-    int earlierDiffIndex = Collections.binarySearch(difflist, earlierSnapshot);
+    int earlierDiffIndex = Collections.binarySearch(difflist, earlier.getId());
-    int laterDiffIndex = laterSnapshot == null ? size : Collections
+    int laterDiffIndex = later == null ? size : Collections
-        .binarySearch(difflist, laterSnapshot);
+        .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 {
+        } else if (!dirCopy.metadataEquals(sdiff.snapshotINode)) {
-          if (!dirCopy.metadataEquals(sdiff.snapshotINode)) {
+          dirMetadataChanged = true;
-            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. */

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<byte[]> {
   }
 
   /**
-   * Compare snapshot IDs. Null indicates the current status thus is greater
+   * Compare snapshot with IDs, where null indicates the current status thus
-   * than non-null snapshots.
+   * is greater than any non-null snapshot.
    */
   public static final Comparator<Snapshot> ID_COMPARATOR
       = new Comparator<Snapshot>() {
     @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<Integer> ID_INTEGER_COMPARATOR
+      = new Comparator<Integer>() {
+    @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; 
       }
     }
   };

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.

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<Integer, Snapshot> 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);
   }
- 
 }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/Diff.java

@@ -104,7 +104,7 @@ public class Diff<K, E extends Diff.Element<K>> {
   }
   
   /** 
-   * 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<E> {
@@ -215,8 +215,8 @@ public class Diff<K, E extends Diff.Element<K>> {
   }
 
   /**
-   * Undo the previous {@link #create(E)} operation. Note that the behavior is
+   * Undo the previous create(E) operation. Note that the behavior is
-   * undefined if the previous operation is not {@link #create(E)}.
+   * 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<K, E extends Diff.Element<K>> {
   }
   
   /**
-   * Undo the previous {@link #delete(E)} operation. Note that the behavior is
+   * Undo the previous delete(E) operation. Note that the behavior is
-   * undefined if the previous operation is not {@link #delete(E)}.
+   * undefined if the previous operation is not delete(E).
    */
   public void undoDelete(final E element, final UndoInfo<E> undoInfo) {
     final int c = undoInfo.createdInsertionPoint;
@@ -285,8 +285,8 @@ public class Diff<K, E extends Diff.Element<K>> {
   }
 
   /**
-   * Undo the previous {@link #modify(E, E)} operation. Note that the behavior
+   * Undo the previous modify(E, E) operation. Note that the behavior
-   * is undefined if the previous operation is not {@link #modify(E, E)}.
+   * is undefined if the previous operation is not modify(E, E).
    */
   public void undoModify(final E oldElement, final E newElement,
       final UndoInfo<E> undoInfo) {
@@ -383,24 +383,24 @@ public class Diff<K, E extends Diff.Element<K>> {
    * 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).
    * </pre>
    * 
-   * @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.
    */