diff --git a/hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/SLSRunner.java b/hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/SLSRunner.java index 6f75bd17c6f..e5b24d5c981 100644 --- a/hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/SLSRunner.java +++ b/hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/SLSRunner.java @@ -157,12 +157,24 @@ private TraceType inputType; private SynthTraceJobProducer stjp; - public SLSRunner() throws ClassNotFoundException { + // properties that need to be positive integers + private final List numberProps = new ArrayList<>( + Arrays.asList(SLSConfiguration.RUNNER_POOL_SIZE, + SLSConfiguration.NM_MEMORY_MB, + SLSConfiguration.NM_VCORES, + SLSConfiguration.NM_HEARTBEAT_INTERVAL_MS, + SLSConfiguration.AM_HEARTBEAT_INTERVAL_MS, + SLSConfiguration.CONTAINER_MEMORY_MB, + SLSConfiguration.CONTAINER_VCORES, + SLSConfiguration.METRICS_WEB_ADDRESS_PORT)); + + public SLSRunner() throws ClassNotFoundException, SLSConfigurationException { Configuration tempConf = new Configuration(false); init(tempConf); } - public SLSRunner(Configuration tempConf) throws ClassNotFoundException { + public SLSRunner(Configuration tempConf) throws ClassNotFoundException, + SLSConfigurationException { init(tempConf); } @@ -176,7 +188,8 @@ public void setConf(Configuration conf) { super.setConf(conf); } - private void init(Configuration tempConf) throws ClassNotFoundException { + private void init(Configuration tempConf) throws ClassNotFoundException, + SLSConfigurationException { nmMap = new ConcurrentHashMap<>(); queueAppNumMap = new HashMap<>(); amMap = new ConcurrentHashMap<>(); @@ -184,6 +197,7 @@ private void init(Configuration tempConf) throws ClassNotFoundException { appIdAMSim = new ConcurrentHashMap<>(); // runner configuration setConf(tempConf); + validateConfig(this.getConf()); // runner poolSize = tempConf.getInt(SLSConfiguration.RUNNER_POOL_SIZE, @@ -201,6 +215,32 @@ private void init(Configuration tempConf) throws ClassNotFoundException { nodeManagerResource = getNodeManagerResource(); } + /** + * Validates parameters that should be positive integers + */ + void validateConfig(Configuration conf) throws SLSConfigurationException { + Map props = + conf.getPropsWithPrefix(SLSConfiguration.PREFIX); + StringBuilder exceptionMessage = new StringBuilder(); + for (String prop : props.keySet()) { + if (numberProps.contains(SLSConfiguration.PREFIX + prop)) { + try { + if (Integer.parseInt(props.get(prop)) < 0) { + exceptionMessage.append("Property ") + .append(SLSConfiguration.PREFIX) + .append(prop).append(" needs to be a positive number. "); + } + } catch(NumberFormatException e) { + exceptionMessage.append("Property ").append(SLSConfiguration.PREFIX) + .append(prop).append(" needs to be an integer. "); + } + } + } + if (exceptionMessage.length() > 0) { + throw new SLSConfigurationException(exceptionMessage.toString()); + } + } + private Resource getNodeManagerResource() { Resource resource = Resources.createResource(0); ResourceInformation[] infors = ResourceUtils.getResourceTypesArray(); @@ -1114,4 +1154,15 @@ public int hashCode() { return result; } } + + /** + * Exception class that indicates an issue with config values in SLS + */ + public static class SLSConfigurationException extends Exception { + + public SLSConfigurationException(String message) { + super(message); + } + } + } diff --git a/hadoop-tools/hadoop-sls/src/test/java/org/apache/hadoop/yarn/sls/TestSLSRunner.java b/hadoop-tools/hadoop-sls/src/test/java/org/apache/hadoop/yarn/sls/TestSLSRunner.java index 2463ccf06a9..b742e4df2c3 100644 --- a/hadoop-tools/hadoop-sls/src/test/java/org/apache/hadoop/yarn/sls/TestSLSRunner.java +++ b/hadoop-tools/hadoop-sls/src/test/java/org/apache/hadoop/yarn/sls/TestSLSRunner.java @@ -31,8 +31,11 @@ import java.security.Security; import java.util.*; +import org.apache.hadoop.yarn.sls.SLSRunner.SLSConfigurationException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * This test performs simple runs of the SLS with different trace types and @@ -125,4 +128,64 @@ public void testEnableCaching() { String.valueOf(networkNegativeCacheDefault)); } } + + @Test + public void testNegativeNumberConfigValue() { + Configuration conf = new Configuration(); + conf.set(SLSConfiguration.RUNNER_POOL_SIZE, "-10"); + try { + SLSRunner sls = new SLSRunner(); + sls.validateConfig(conf); + } catch (ClassNotFoundException e) { + fail("ClassNotFoundException not expected."); + } catch (SLSConfigurationException se) { + assertTrue(se.getMessage().equals("Property " + + SLSConfiguration.RUNNER_POOL_SIZE + + " needs to be a positive number. ")); + return; + } + fail("Configuration exception expected"); + } + + @Test + public void testNonNumberConfigValues() { + Configuration conf = new Configuration(); + conf.set(SLSConfiguration.CONTAINER_MEMORY_MB, "75MB"); + try { + SLSRunner sls = new SLSRunner(); + sls.validateConfig(conf); + } catch (ClassNotFoundException e) { + fail("ClassNotFoundException not expected."); + } catch (SLSConfigurationException se) { + assertTrue(se.getMessage().equals("Property " + + SLSConfiguration.CONTAINER_MEMORY_MB + + " needs to be an integer. ")); + return; + } + fail("Configuration exception expected"); + } + + @Test + public void testTwoBadConfigValues() { + Configuration conf = new Configuration(); + conf.set(SLSConfiguration.RUNNER_POOL_SIZE, "-10"); + conf.set(SLSConfiguration.CONTAINER_MEMORY_MB, "75MB"); + conf.set(SLSConfiguration.NM_HEARTBEAT_INTERVAL_MS, "3"); + try { + SLSRunner sls = new SLSRunner(); + sls.validateConfig(conf); + } catch (ClassNotFoundException e) { + fail("ClassNotFoundException not expected."); + } catch (SLSConfigurationException se) { + assertTrue(se.getMessage().equals("Property " + + SLSConfiguration.RUNNER_POOL_SIZE + + " needs to be a positive number. " + + "Property " + + SLSConfiguration.CONTAINER_MEMORY_MB + + " needs to be an integer. ")); + return; + } + fail("Configuration exception expected"); + } + }