diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/HAUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/HAUtil.java index 18f9896..143e9f3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/HAUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/HAUtil.java @@ -60,7 +60,14 @@ public static boolean isHAEnabled(Configuration conf) { } public static Collection getRMHAIds(Configuration conf) { - return conf.getTrimmedStringCollection(YarnConfiguration.RM_HA_IDS); + Collection ids = + conf.getTrimmedStringCollection(YarnConfiguration.RM_HA_IDS); + if (ids.size() <= 0) { + throwBadConfigurationException(YarnConfiguration.RM_HA_IDS + + " is invalid. Current value is " + + conf.get(YarnConfiguration.RM_HA_IDS)); + } + return ids; } /** @@ -70,18 +77,31 @@ public static boolean isHAEnabled(Configuration conf) { */ @VisibleForTesting public static String getRMHAId(Configuration conf) { - String rmId = conf.get(YarnConfiguration.RM_HA_ID); + String rmId = conf.getTrimmed(YarnConfiguration.RM_HA_ID); if (rmId == null) { throwBadConfigurationException(YarnConfiguration.RM_HA_ID + " needs to be set in a HA configuration"); + } else { + Collection ids = getRMHAIds(conf); + if (!ids.contains(rmId)) { + throwBadConfigurationException(YarnConfiguration.RM_HA_IDS + "(" + + ids + ") need to contain " + YarnConfiguration.RM_HA_ID + "(" + + rmId + ") in a HA configuration. "); + } } + return rmId; } + private static String getConfKeyForRMInstance(String prefix, + Configuration conf) { + return addSuffix(prefix, getRMHAId(conf)); + } + private static String getConfValueForRMInstance(String prefix, Configuration conf) { - String confKey = addSuffix(prefix, getRMHAId(conf)); - String retVal = conf.get(confKey); + String confKey = getConfKeyForRMInstance(prefix, conf); + String retVal = conf.getTrimmed(confKey); if (LOG.isTraceEnabled()) { LOG.trace("getConfValueForRMInstance: prefix = " + prefix + "; confKey being looked up = " + confKey + @@ -97,7 +117,27 @@ static String getConfValueForRMInstance(String prefix, String defaultValue, } private static void setConfValue(String prefix, Configuration conf) { - conf.set(prefix, getConfValueForRMInstance(prefix, conf)); + String confKey = null; + String confValue = null; + try { + confKey = getConfKeyForRMInstance(prefix, conf); + confValue = getConfValueForRMInstance(prefix, conf); + conf.set(prefix, confValue); + } catch (YarnRuntimeException yre) { + // Error at getRMHAId() + throw yre; + } catch (IllegalArgumentException iae) { + String errmsg; + if (confKey == null) { + // Error at addSuffix + errmsg = "Invalid value of " + YarnConfiguration.RM_HA_ID + + ". Current value is " + getRMHAId(conf); + } else { + // Error at Configuration#set. + errmsg = confKey + " needs to be set in a HA configuration."; + } + throwBadConfigurationException(errmsg); + } } public static void setAllRpcAddresses(Configuration conf) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/conf/TestHAUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/conf/TestHAUtil.java index e0e46c4..47c2aee 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/conf/TestHAUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/conf/TestHAUtil.java @@ -21,6 +21,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.yarn.exceptions.YarnRuntimeException; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -32,10 +33,12 @@ public class TestHAUtil { private Configuration conf; - private static final String RM1_ADDRESS = "1.2.3.4:8021"; + private static final String RM1_ADDRESS = " \t\t\n 1.2.3.4:8021 \n\t "; private static final String RM2_ADDRESS = "localhost:8022"; - private static final String RM1_NODE_ID = "rm1"; + private static final String RM1_NODE_ID = "rm1 "; private static final String RM2_NODE_ID = "rm2"; + private static final String RM3_NODE_ID = "rm3"; + private static final String RM_INVALID_NODE_ID = ".rm"; @Before public void setUp() { @@ -44,7 +47,8 @@ public void setUp() { conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID); for (String confKey : HAUtil.RPC_ADDRESS_CONF_KEYS) { - conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS); + // configuration key itself cannot contains space/tab/return chars. + conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID.trim()), RM1_ADDRESS); conf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS); } } @@ -53,12 +57,30 @@ public void setUp() { public void testGetRMServiceId() throws Exception { Collection rmhaIds = HAUtil.getRMHAIds(conf); assertEquals(2, rmhaIds.size()); + + String[] ids = rmhaIds.toArray(new String[0]); + assertEquals(RM1_NODE_ID.trim(), ids[0]); + assertEquals(RM2_NODE_ID, ids[1]); + + // simulate the case YarnConfiguration.RM_HA_IDS is not set + conf.clear(); + conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID); + for (String confKey : HAUtil.RPC_ADDRESS_CONF_KEYS) { + conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS); + } + try { + HAUtil.getRMHAIds(conf); + } catch (YarnRuntimeException e) { + assertEquals("YarnRuntimeException by validation in getRMHAIds()", + "Invalid configuration! " + YarnConfiguration.RM_HA_IDS + + " is invalid. Current value is null", e.getMessage()); + } } @Test public void testGetRMId() throws Exception { assertEquals("Does not honor " + YarnConfiguration.RM_HA_ID, - RM1_NODE_ID, HAUtil.getRMHAId(conf)); + RM1_NODE_ID.trim(), HAUtil.getRMHAId(conf)); conf = new YarnConfiguration(); try { HAUtil.getRMHAId(conf); @@ -73,7 +95,68 @@ public void testSetGetRpcAddresses() throws Exception { HAUtil.setAllRpcAddresses(conf); for (String confKey : HAUtil.RPC_ADDRESS_CONF_KEYS) { assertEquals("RPC address not set for " + confKey, - RM1_ADDRESS, conf.get(confKey)); + RM1_ADDRESS.trim(), conf.get(confKey)); + } + } + + @Test + public void testErrorHandlingOfSetGetRpcAddresses() throws Exception { + conf.clear(); + try { + HAUtil.setAllRpcAddresses(conf); + } catch (YarnRuntimeException e) { + assertEquals("YarnRuntimeException by getRMId()", + "Invalid configuration! " +YarnConfiguration.RM_HA_ID + + " needs to be set in a HA configuration" , e.getMessage()); + } + + conf.set(YarnConfiguration.RM_HA_ID, RM_INVALID_NODE_ID); + conf.set(YarnConfiguration.RM_HA_IDS, RM_INVALID_NODE_ID); + for (String confKey : HAUtil.RPC_ADDRESS_CONF_KEYS) { + // simulate xml with invalid node id + conf.set(confKey + RM_INVALID_NODE_ID, RM_INVALID_NODE_ID); + } + try { + HAUtil.setAllRpcAddresses(conf); + } catch (YarnRuntimeException e) { + assertEquals("YarnRuntimeException by addSuffix()", + "Invalid configuration! Invalid value of " + + YarnConfiguration.RM_HA_ID + ". Current value is .rm", + e.getMessage()); + } + + conf.clear(); + // simulate the case HAUtil.RPC_ADDRESS_CONF_KEYS are not set + conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID); + conf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID); + try { + HAUtil.setAllRpcAddresses(conf); + fail("should throw YarnRuntimeException. by Configuration#set()"); + } catch (YarnRuntimeException e) { + String confKey = HAUtil.addSuffix(YarnConfiguration.RM_ADDRESS, RM1_NODE_ID.trim()); + assertEquals("YarnRuntimeException by Configuration#set()", + "Invalid configuration! " + confKey + " needs to be set " + + "in a HA configuration.", + e.getMessage()); + } + + // simulate the case YarnConfiguration.RM_HA_IDS doesn't contain + // the value of YarnConfiguration.RM_HA_ID + conf.clear(); + conf.set(YarnConfiguration.RM_HA_IDS, RM2_NODE_ID + "," + RM3_NODE_ID); + conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID); + for (String confKey : HAUtil.RPC_ADDRESS_CONF_KEYS) { + conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS); + conf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS); + } + try { + HAUtil.setAllRpcAddresses(conf); + } catch (YarnRuntimeException e) { + assertEquals("YarnRuntimeException by getRMId()'s validation", + "Invalid configuration! " + + YarnConfiguration.RM_HA_IDS + "([rm2, rm3]) need to contain " + + YarnConfiguration.RM_HA_ID + "(rm1) in a HA configuration. ", + e.getMessage()); } } }