From e5a65ebc281a3dc333f5223b23df384781326e8a Mon Sep 17 00:00:00 2001
From: Arpit Agarwal <arp@apache.org>
Date: Tue, 12 Sep 2017 16:18:41 -0700
Subject: [PATCH] HDFS-12407. Journal node fails to shutdown cleanly if
 JournalNodeHttpServer or JournalNodeRpcServer fails to start. Contributed by
 Ajay Kumar.

---
 .../hdfs/qjournal/server/JournalNode.java     | 45 +++++++++++--------
 .../hdfs/qjournal/server/TestJournalNode.java | 21 +++++++++
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java
index 6056e34d8f3..f56848cfdee 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java
@@ -157,27 +157,36 @@ public class JournalNode implements Tool, Configurable, JournalNodeMXBean {
    */
   public void start() throws IOException {
     Preconditions.checkState(!isStarted(), "JN already running");
-    
-    validateAndCreateJournalDir(localDir);
-    
-    DefaultMetricsSystem.initialize("JournalNode");
-    JvmMetrics.create("JournalNode",
-        conf.get(DFSConfigKeys.DFS_METRICS_SESSION_ID_KEY),
-        DefaultMetricsSystem.instance());
 
-    InetSocketAddress socAddr = JournalNodeRpcServer.getAddress(conf);
-    SecurityUtil.login(conf, DFSConfigKeys.DFS_JOURNALNODE_KEYTAB_FILE_KEY,
-        DFSConfigKeys.DFS_JOURNALNODE_KERBEROS_PRINCIPAL_KEY, socAddr.getHostName());
-    
-    registerJNMXBean();
-    
-    httpServer = new JournalNodeHttpServer(conf, this);
-    httpServer.start();
+    try {
 
-    httpServerURI = httpServer.getServerURI().toString();
+      validateAndCreateJournalDir(localDir);
 
-    rpcServer = new JournalNodeRpcServer(conf, this);
-    rpcServer.start();
+      DefaultMetricsSystem.initialize("JournalNode");
+      JvmMetrics.create("JournalNode",
+          conf.get(DFSConfigKeys.DFS_METRICS_SESSION_ID_KEY),
+          DefaultMetricsSystem.instance());
+
+      InetSocketAddress socAddr = JournalNodeRpcServer.getAddress(conf);
+      SecurityUtil.login(conf, DFSConfigKeys.DFS_JOURNALNODE_KEYTAB_FILE_KEY,
+          DFSConfigKeys.DFS_JOURNALNODE_KERBEROS_PRINCIPAL_KEY,
+          socAddr.getHostName());
+
+      registerJNMXBean();
+
+      httpServer = new JournalNodeHttpServer(conf, this);
+      httpServer.start();
+
+      httpServerURI = httpServer.getServerURI().toString();
+
+      rpcServer = new JournalNodeRpcServer(conf, this);
+      rpcServer.start();
+    } catch (IOException ioe) {
+      //Shutdown JournalNode of JournalNodeRpcServer fails to start
+      LOG.error("Failed to start JournalNode.", ioe);
+      this.stop(1);
+      throw ioe;
+    }
   }
 
   public boolean isStarted() {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
index 28ec7082537..77b50a178eb 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java
@@ -55,6 +55,7 @@ import org.junit.Test;
 import com.google.common.base.Charsets;
 import com.google.common.primitives.Bytes;
 import com.google.common.primitives.Ints;
+import org.mockito.Mockito;
 
 
 public class TestJournalNode {
@@ -342,4 +343,24 @@ public class TestJournalNode {
     System.err.println("Time per batch: " + avgRtt + "ms");
     System.err.println("Throughput: " + throughput + " bytes/sec");
   }
+
+  /**
+   * Test case to check if JournalNode exits cleanly when httpserver or rpc
+   * server fails to start. Call to JournalNode start should fail with bind
+   * exception as the port is in use by the JN started in @Before routine
+   */
+  @Test
+  public void testJournalNodeStartupFailsCleanly() {
+    JournalNode jNode = Mockito.spy(new JournalNode());
+    try {
+      jNode.setConf(conf);
+      jNode.start();
+      fail("Should throw bind exception");
+    } catch (Exception e) {
+      GenericTestUtils
+          .assertExceptionContains("java.net.BindException: Port in use", e);
+    }
+    Mockito.verify(jNode).stop(1);
+  }
+
 }