From 010e6ac328855bad59f138b6aeaec535272f448c Mon Sep 17 00:00:00 2001
From: Andrew Wang <wang@apache.org>
Date: Wed, 18 May 2016 14:13:41 -0700
Subject: [PATCH] HDFS-2173. saveNamespace should not throw IOE when only one
 storage directory fails to write VERSION file. Contributed by Andras Bokor.

---
 .../hdfs/server/namenode/NNStorage.java       | 25 +++++++++++++-
 .../server/namenode/TestSaveNamespace.java    | 33 +++++++++++++++----
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorage.java
index ed36b27d2e..44e0ce34d4 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorage.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorage.java
@@ -1116,4 +1116,27 @@ public class NNStorage extends Storage implements Closeable,
     nameDirSizeMap.clear();
     nameDirSizeMap.putAll(nnDirSizeMap);
   }
-}
+
+  /**
+   * Write all data storage files.
+   * @throws IOException When all the storage directory fails to write
+   * VERSION file
+   */
+  @Override
+  public void writeAll() throws IOException {
+    this.layoutVersion = getServiceLayoutVersion();
+    for (StorageDirectory sd : storageDirs) {
+      try {
+        writeProperties(sd);
+      } catch (Exception e) {
+        LOG.warn("Error during write properties to the VERSION file to " +
+            sd.toString(), e);
+        reportErrorsOnDirectory(sd);
+        if (storageDirs.isEmpty()) {
+          throw new IOException("All the storage failed while writing " +
+              "properties to VERSION file");
+        }
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSaveNamespace.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSaveNamespace.java
index a03e900f0d..8191cc2033 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSaveNamespace.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSaveNamespace.java
@@ -111,6 +111,29 @@ public class TestSaveNamespace {
     }
   }
 
+  private static class FaultyWriteProperties implements Answer<Void> {
+    private int count = 0;
+    private Fault faultType;
+
+    FaultyWriteProperties(Fault faultType) {
+      this.faultType = faultType;
+    }
+
+    @Override
+    public Void answer(InvocationOnMock invocation) throws Throwable {
+      Object[] args = invocation.getArguments();
+      StorageDirectory sd = (StorageDirectory)args[0];
+
+      if (faultType == Fault.WRITE_STORAGE_ALL ||
+          (faultType==Fault.WRITE_STORAGE_ONE && count++==1)) {
+        LOG.info("Injecting fault for sd: " + sd);
+        throw new IOException("Injected fault: writeProperties second time");
+      }
+      LOG.info("Not injecting fault for sd: " + sd);
+      return (Void)invocation.callRealMethod();
+    }
+  }
+
   private enum Fault {
     SAVE_SECOND_FSIMAGE_RTE,
     SAVE_SECOND_FSIMAGE_IOE,
@@ -164,17 +187,15 @@ public class TestSaveNamespace {
       break;
     case WRITE_STORAGE_ALL:
       // The spy throws an exception before writing any VERSION files
-      doThrow(new RuntimeException("Injected"))
-        .when(spyStorage).writeAll();
+      doAnswer(new FaultyWriteProperties(Fault.WRITE_STORAGE_ALL))
+          .when(spyStorage).writeProperties((StorageDirectory)anyObject());
       shouldFail = true;
       break;
     case WRITE_STORAGE_ONE:
       // The spy throws on exception on one particular storage directory
-      doAnswer(new FaultySaveImage(true))
+      doAnswer(new FaultyWriteProperties(Fault.WRITE_STORAGE_ONE))
         .when(spyStorage).writeProperties((StorageDirectory)anyObject());
-      // TODO: unfortunately this fails -- should be improved.
-      // See HDFS-2173.
-      shouldFail = true;
+      shouldFail = false;
       break;
     }