From cc9bc8eef78e6bc0ab038102f45712456c62a5eb Mon Sep 17 00:00:00 2001
From: Junping Du <junping_du@apache.org>
Date: Sun, 8 Jun 2014 07:06:35 +0000
Subject: [PATCH] HDFS-6500. Snapshot shouldn't be removed silently after
 renaming to an existing snapshot. (Contributed by Nicholas SZE)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1601199 13f79535-47bb-0310-9956-ffa450edef68
---
 hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt           |  3 +++
 .../snapshot/INodeDirectorySnapshottable.java         |  4 ++--
 .../org/apache/hadoop/hdfs/TestSnapshotCommands.java  | 11 +++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index 1c8674e19cb..41962edf18a 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -654,6 +654,9 @@ Release 2.5.0 - UNRELEASED
     HDFS-6497. Make TestAvailableSpaceVolumeChoosingPolicy deterministic
     (cmccabe)
 
+    HDFS-6500. Snapshot shouldn't be removed silently after renaming to an 
+    existing snapshot. (Nicholas SZE via junping_du)
+
 Release 2.4.1 - UNRELEASED
 
   INCOMPATIBLE CHANGES
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java
index 8944e90236b..4288ec2afb0 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java
@@ -234,7 +234,7 @@ public class INodeDirectorySnapshottable extends INodeDirectory {
    *           name does not exist or a snapshot with the new name already
    *           exists
    */
-  public void renameSnapshot(String path, String oldName, String newName)
+  void renameSnapshot(String path, String oldName, String newName)
       throws SnapshotException {
     if (newName.equals(oldName)) {
       return;
@@ -246,7 +246,7 @@ public class INodeDirectorySnapshottable extends INodeDirectory {
     } else {
       final byte[] newNameBytes = DFSUtil.string2Bytes(newName);
       int indexOfNew = searchSnapshot(newNameBytes);
-      if (indexOfNew > 0) {
+      if (indexOfNew >= 0) {
         throw new SnapshotException("The snapshot " + newName
             + " already exists for directory " + path);
       }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSnapshotCommands.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSnapshotCommands.java
index d2c7a0991ff..e2db636936d 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSnapshotCommands.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSnapshotCommands.java
@@ -186,6 +186,17 @@ public class TestSnapshotCommands {
     FsShellRun("-ls /sub1/.snapshot", 0, "/sub1/.snapshot/sn.rename");
     FsShellRun("-ls /sub1/.snapshot/sn.rename", 0, "/sub1/.snapshot/sn.rename/sub1sub1");
     FsShellRun("-ls /sub1/.snapshot/sn.rename", 0, "/sub1/.snapshot/sn.rename/sub1sub2");
+
+    //try renaming from a non-existing snapshot
+    FsShellRun("-renameSnapshot /sub1 sn.nonexist sn.rename", 1,
+        "renameSnapshot: The snapshot sn.nonexist does not exist for directory /sub1");
+
+    //try renaming to existing snapshots
+    FsShellRun("-createSnapshot /sub1 sn.new");
+    FsShellRun("-renameSnapshot /sub1 sn.new sn.rename", 1,
+        "renameSnapshot: The snapshot sn.rename already exists for directory /sub1");
+    FsShellRun("-renameSnapshot /sub1 sn.rename sn.new", 1,
+        "renameSnapshot: The snapshot sn.new already exists for directory /sub1");
   }
 
   @Test