From e0b1dc514f1b9e26b21d97ece0d94cd9c827fc59 Mon Sep 17 00:00:00 2001
From: cnauroth <cnauroth@apache.org>
Date: Thu, 25 Sep 2014 10:03:59 -0700
Subject: [PATCH] HDFS-7118. Improve diagnostics on storage directory rename
 operations by using NativeIO#renameTo in Storage#rename. Contributed by Chris
 Nauroth.

---
 hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt   |  3 ++
 .../hadoop/hdfs/server/common/Storage.java    | 12 ++++--
 .../namenode/TestFileJournalManager.java      | 37 +++++++++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index 13beb8bd7ef..ec4674ba1cd 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -582,6 +582,9 @@ Release 2.6.0 - UNRELEASED
 
     HDFS-7138. Fix hftp to work with encryption. (clamb via wang)
 
+    HDFS-7118. Improve diagnostics on storage directory rename operations by
+    using NativeIO#renameTo in Storage#rename. (cnauroth)
+
   OPTIMIZATIONS
 
     HDFS-6690. Deduplicate xattr names in memory. (wang)
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
index 7933feddeda..f9706bdc8a6 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
@@ -36,6 +36,8 @@ import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.NodeType;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption;
+import org.apache.hadoop.io.nativeio.NativeIO;
+import org.apache.hadoop.io.nativeio.NativeIOException;
 import org.apache.hadoop.util.ToolRunner;
 import org.apache.hadoop.util.VersionInfo;
 
@@ -986,9 +988,13 @@ public abstract class Storage extends StorageInfo {
   }
 
   public static void rename(File from, File to) throws IOException {
-    if (!from.renameTo(to))
-      throw new IOException("Failed to rename " 
-                            + from.getCanonicalPath() + " to " + to.getCanonicalPath());
+    try {
+      NativeIO.renameTo(from, to);
+    } catch (NativeIOException e) {
+      throw new IOException("Failed to rename " + from.getCanonicalPath()
+        + " to " + to.getCanonicalPath() + " due to failure in native rename. "
+        + e.toString());
+    }
   }
 
   /**
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileJournalManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileJournalManager.java
index c7fe67f0163..f5172c35d9e 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileJournalManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileJournalManager.java
@@ -21,6 +21,7 @@ import static org.apache.hadoop.hdfs.server.namenode.TestEditLog.TXNS_PER_FAIL;
 import static org.apache.hadoop.hdfs.server.namenode.TestEditLog.TXNS_PER_ROLL;
 import static org.apache.hadoop.hdfs.server.namenode.TestEditLog.setupEdits;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -42,8 +43,11 @@ import org.apache.hadoop.hdfs.server.namenode.JournalManager.CorruptionException
 import org.apache.hadoop.hdfs.server.namenode.NNStorage.NameNodeDirType;
 import org.apache.hadoop.hdfs.server.namenode.TestEditLog.AbortSpec;
 import org.apache.hadoop.io.IOUtils;
+import org.apache.hadoop.util.NativeCodeLoader;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -59,6 +63,9 @@ public class TestFileJournalManager {
     EditLogFileOutputStream.setShouldSkipFsyncForTesting(true);
   }
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Before
   public void setUp() {
     conf = new Configuration();
@@ -472,6 +479,36 @@ public class TestFileJournalManager {
     }
   }
 
+  /**
+   * Tests that internal renames are done using native code on platforms that
+   * have it.  The native rename includes more detailed information about the
+   * failure, which can be useful for troubleshooting.
+   */
+  @Test
+  public void testDoPreUpgradeIOError() throws IOException {
+    File storageDir = new File(TestEditLog.TEST_DIR, "preupgradeioerror");
+    List<URI> editUris = Collections.singletonList(storageDir.toURI());
+    NNStorage storage = setupEdits(editUris, 5);
+    StorageDirectory sd = storage.dirIterator(NameNodeDirType.EDITS).next();
+    assertNotNull(sd);
+    // Change storage directory so that renaming current to previous.tmp fails.
+    FileUtil.setWritable(storageDir, false);
+    FileJournalManager jm = null;
+    try {
+      jm = new FileJournalManager(conf, sd, storage);
+      exception.expect(IOException.class);
+      if (NativeCodeLoader.isNativeCodeLoaded()) {
+        exception.expectMessage("failure in native rename");
+      }
+      jm.doPreUpgrade();
+    } finally {
+      IOUtils.cleanup(LOG, jm);
+      // Restore permissions on storage directory and make sure we can delete.
+      FileUtil.setWritable(storageDir, true);
+      FileUtil.fullyDelete(storageDir);
+    }
+  }
+
   private static String getLogsAsString(
       FileJournalManager fjm, long firstTxId) throws IOException {
     return Joiner.on(",").join(fjm.getRemoteEditLogs(firstTxId, false));