HDFS-2894. HA: automatically determine the nameservice Id if only one nameservice is configured. Contributed by Eli Collins

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-1623@1240917 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-1623.txt

@@ -172,3 +172,5 @@ HDFS-2808. HA: haadmin should use namenode ids. (eli)
 HDFS-2819. Document new HA-related configs in hdfs-default.xml. (eli)
 
 HDFS-2752. HA: exit if multiple shared dirs are configured. (eli)
+
+HDFS-2894. HA: automatically determine the nameservice Id if only one nameservice is configured. (eli)

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java

@@ -907,9 +907,10 @@ public class DFSUtil {
    * the address of the local node. 
    * 
    * If {@link DFSConfigKeys#DFS_FEDERATION_NAMESERVICE_ID} is not specifically
-   * configured, this method determines the nameservice Id by matching the local
+   * configured, and more than one nameservice Id is configured, this method 
-   * node's address with the configured addresses. When a match is found, it
+   * determines the nameservice Id by matching the local node's address with the
-   * returns the nameservice Id from the corresponding configuration key.
+   * configured addresses. When a match is found, it returns the nameservice Id
+   * from the corresponding configuration key.
    * 
    * @param conf Configuration
    * @param addressKey configuration key to get the address.
@@ -921,6 +922,10 @@ public class DFSUtil {
     if (nameserviceId != null) {
       return nameserviceId;
     }
+    Collection<String> nsIds = getNameServiceIds(conf);
+    if (1 == nsIds.size()) {
+      return nsIds.toArray(new String[1])[0];
+    }
     String nnId = conf.get(DFS_HA_NAMENODE_ID_KEY);
     
     return getSuffixIDs(conf, addressKey, null, nnId, LOCAL_ADDRESS_MATCHER)[0];
@@ -1057,11 +1062,11 @@ public class DFSUtil {
 
     if (nsId == null) {
       Collection<String> nsIds = getNameServiceIds(conf);
-      if (nsIds.size() != 1) {
+      if (1 == nsIds.size()) {
+        nsId = nsIds.toArray(new String[1])[0];
+      } else {
         // No nameservice ID was given and more than one is configured
         return null;
-      } else {
-        nsId = nsIds.toArray(new String[1])[0];
       }
     }
 

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

@@ -47,7 +47,6 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.*;
 import org.apache.hadoop.hdfs.DFSUtil;
 import org.apache.hadoop.hdfs.HAUtil;
 import org.apache.hadoop.hdfs.DFSUtil.ErrorSimulator;
-import org.apache.hadoop.hdfs.HAUtil;
 import org.apache.hadoop.hdfs.HdfsConfiguration;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocolPB.NamenodeProtocolTranslatorPB;
@@ -170,20 +169,17 @@ public class SecondaryNameNode implements Runnable {
     try {
       String nsId = DFSUtil.getSecondaryNameServiceId(conf);
       if (HAUtil.isHAEnabled(conf, nsId)) {
-        LOG.fatal("Cannot use SecondaryNameNode in an HA cluster." +
+        throw new IOException(
+            "Cannot use SecondaryNameNode in an HA cluster." +
             " The Standby Namenode will perform checkpointing.");
-        shutdown();
-        return;
       }
       NameNode.initializeGenericKeys(conf, nsId, null);
       initialize(conf, commandLineOpts);
-    } catch(IOException e) {
+    } catch (IOException e) {
       shutdown();
-      LOG.fatal("Failed to start secondary namenode. ", e);
       throw e;
-    } catch(HadoopIllegalArgumentException e) {
+    } catch (HadoopIllegalArgumentException e) {
       shutdown();
-      LOG.fatal("Failed to start secondary namenode. ", e);
       throw e;
     }
   }
@@ -335,7 +331,6 @@ public class SecondaryNameNode implements Runnable {
   // The main work loop
   //
   public void doWork() {
-
     //
     // Poll the Namenode (once every checkpointCheckPeriod seconds) to find the
     // number of transactions in the edit log that haven't yet been checkpointed.
@@ -612,7 +607,13 @@ public class SecondaryNameNode implements Runnable {
     
     StringUtils.startupShutdownMessage(SecondaryNameNode.class, argv, LOG);
     Configuration tconf = new HdfsConfiguration();
-    SecondaryNameNode secondary = new SecondaryNameNode(tconf, opts);
+    SecondaryNameNode secondary = null;
+    try {
+      secondary = new SecondaryNameNode(tconf, opts);
+    } catch (IOException ioe) {
+      LOG.fatal("Failed to start secondary namenode", ioe);
+      System.exit(-1);
+    }
 
     if (opts.getCommand() != null) {
       int ret = secondary.processStartupCommand(opts);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java

@@ -214,6 +214,10 @@ public class TestDFSUtil {
     checkNameServiceId(conf, NN1_ADDRESS, "nn1");
     checkNameServiceId(conf, NN2_ADDRESS, "nn2");
     checkNameServiceId(conf, NN3_ADDRESS, null);
+
+    // HA is not enabled in a purely federated config
+    assertFalse(HAUtil.isHAEnabled(conf, "nn1"));
+    assertFalse(HAUtil.isHAEnabled(conf, "nn2"));
   }
 
   public void checkNameServiceId(Configuration conf, String addr,
@@ -399,8 +403,10 @@ public class TestDFSUtil {
     
     Map<String, Map<String, InetSocketAddress>> map =
       DFSUtil.getHaNnRpcAddresses(conf);
-    System.err.println("TestHANameNodesWithFederation:\n" +
+
-        DFSUtil.addressMapToString(map));
+    assertTrue(HAUtil.isHAEnabled(conf, "ns1"));
+    assertTrue(HAUtil.isHAEnabled(conf, "ns2"));
+    assertFalse(HAUtil.isHAEnabled(conf, "ns3"));
     
     assertEquals(NS1_NN1_HOST, map.get("ns1").get("ns1-nn1").toString());
     assertEquals(NS1_NN2_HOST, map.get("ns1").get("ns1-nn2").toString());
@@ -414,9 +420,13 @@ public class TestDFSUtil {
     assertEquals(NS2_NN1_HOST, 
         DFSUtil.getNamenodeServiceAddr(conf, "ns2", "ns2-nn1"));
 
-    // No nameservice was given and we can't determine which to use
+    // No nameservice was given and we can't determine which service addr
-    // as two nameservices could share a namenode ID.
+    // to use as two nameservices could share a namenode ID.
     assertEquals(null, DFSUtil.getNamenodeServiceAddr(conf, null, "ns1-nn1"));
+
+    // Ditto for nameservice IDs, if multiple are defined
+    assertEquals(null, DFSUtil.getNamenodeNameServiceId(conf));
+    assertEquals(null, DFSUtil.getSecondaryNameServiceId(conf));
   }
 
   @Test
@@ -453,6 +463,10 @@ public class TestDFSUtil {
 
     assertEquals(NS1_NN1_HOST_SVC, DFSUtil.getNamenodeServiceAddr(conf, null, "nn1"));
     assertEquals(NS1_NN2_HOST_SVC, DFSUtil.getNamenodeServiceAddr(conf, null, "nn2"));
+
+    // We can determine the nameservice ID, there's only one listed
+    assertEquals("ns1", DFSUtil.getNamenodeNameServiceId(conf));
+    assertEquals("ns1", DFSUtil.getSecondaryNameServiceId(conf));
   }
 
   @Test

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHAConfiguration.java

@@ -29,6 +29,7 @@ import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.DFSUtil;
 import org.apache.hadoop.hdfs.server.namenode.FSNamesystem;
 import org.apache.hadoop.hdfs.server.namenode.NameNode;
+import org.apache.hadoop.hdfs.server.namenode.SecondaryNameNode;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -39,16 +40,13 @@ import org.mockito.Mockito;
  * which don't start daemons.
  */
 public class TestHAConfiguration {
-  private static final String NSID = "ns1";
-  private static String HOST_A = "1.2.3.1";
-  private static String HOST_B = "1.2.3.2";
 
   private FSNamesystem fsn = Mockito.mock(FSNamesystem.class);
-  private Configuration conf = new Configuration();
 
   @Test
   public void testCheckpointerValidityChecks() throws Exception {
     try {
+      Configuration conf = new Configuration();
       new StandbyCheckpointer(conf, fsn);
       fail("Bad config did not throw an error");
     } catch (IllegalArgumentException iae) {
@@ -56,30 +54,37 @@ public class TestHAConfiguration {
           "Invalid URI for NameNode address", iae);
     }
   }
-  
+
-  @Test
+  private Configuration getHAConf(String nsId, String host1, String host2) {
-  public void testGetOtherNNHttpAddress() {
+    Configuration conf = new Configuration();
-    conf.set(DFSConfigKeys.DFS_FEDERATION_NAMESERVICES, NSID);    
+    conf.set(DFSConfigKeys.DFS_FEDERATION_NAMESERVICES, nsId);    
-    conf.set(DFSConfigKeys.DFS_FEDERATION_NAMESERVICE_ID, NSID);
     conf.set(DFSUtil.addKeySuffixes(
-        DFSConfigKeys.DFS_HA_NAMENODES_KEY_PREFIX, NSID),
+        DFSConfigKeys.DFS_HA_NAMENODES_KEY_PREFIX, nsId),
         "nn1,nn2");    
     conf.set(DFSConfigKeys.DFS_HA_NAMENODE_ID_KEY, "nn1");
     conf.set(DFSUtil.addKeySuffixes(
-            DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_KEY,
+        DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_KEY, nsId, "nn1"),
-            NSID, "nn1"),
+        host1 + ":12345");
-        HOST_A + ":12345");
     conf.set(DFSUtil.addKeySuffixes(
-            DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_KEY,
+        DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_KEY, nsId, "nn2"),
-            NSID, "nn2"),
+        host2 + ":12345");
-        HOST_B + ":12345");
+    return conf;
-    NameNode.initializeGenericKeys(conf, NSID, "nn1");
+  }
+
+  @Test
+  public void testGetOtherNNHttpAddress() {
+    // Use non-local addresses to avoid host address matching
+    Configuration conf = getHAConf("ns1", "1.2.3.1", "1.2.3.2");
+    conf.set(DFSConfigKeys.DFS_FEDERATION_NAMESERVICE_ID, "ns1");
+
+    // This is done by the NN before the StandbyCheckpointer is created
+    NameNode.initializeGenericKeys(conf, "ns1", "nn1");
 
     // Since we didn't configure the HTTP address, and the default is
-    // 0.0.0.0, it should substitute the address from the RPC configuratoin
+    // 0.0.0.0, it should substitute the address from the RPC configuration
     // above.
     StandbyCheckpointer checkpointer = new StandbyCheckpointer(conf, fsn);
-    assertEquals(HOST_B + ":" + DFSConfigKeys.DFS_NAMENODE_HTTP_PORT_DEFAULT,
+    assertEquals("1.2.3.2:" + DFSConfigKeys.DFS_NAMENODE_HTTP_PORT_DEFAULT,
         checkpointer.getActiveNNAddress());
   }
   
@@ -89,14 +94,33 @@ public class TestHAConfiguration {
    */
   @Test
   public void testHAUniqueEditDirs() throws IOException {
-    Configuration config = new Configuration();
+    Configuration conf = new Configuration();
 
-    config.set(DFS_NAMENODE_EDITS_DIR_KEY, "file://edits/dir, "
+    conf.set(DFS_NAMENODE_EDITS_DIR_KEY, "file://edits/dir, "
         + "file://edits/shared/dir"); // overlapping
-    config.set(DFS_NAMENODE_SHARED_EDITS_DIR_KEY, "file://edits/shared/dir");
+    conf.set(DFS_NAMENODE_SHARED_EDITS_DIR_KEY, "file://edits/shared/dir");
 
     // getNamespaceEditsDirs removes duplicates across edits and shared.edits
-    Collection<URI> editsDirs = FSNamesystem.getNamespaceEditsDirs(config);
+    Collection<URI> editsDirs = FSNamesystem.getNamespaceEditsDirs(conf);
     assertEquals(2, editsDirs.size());
   }
+  
+  /**
+   * Test that the 2NN does not start if given a config with HA NNs.
+   */
+  @Test
+  public void testSecondaryNameNodeDoesNotStart() throws IOException {
+    // Note we're not explicitly setting the nameservice Id in the
+    // config as it is not required to be set and we want to test
+    // that we can determine if HA is enabled when the nameservice Id
+    // is not explicitly defined.
+    Configuration conf = getHAConf("ns1", "1.2.3.1", "1.2.3.2");
+    try {
+      new SecondaryNameNode(conf);
+      fail("Created a 2NN with an HA config");
+    } catch (IOException ioe) {
+      GenericTestUtils.assertExceptionContains(
+          "Cannot use SecondaryNameNode in an HA cluster", ioe);
+    }
+  }
 }