diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairSchedulerConfiguration.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairSchedulerConfiguration.java index e6b1de4e7f..cfe07c9ef4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairSchedulerConfiguration.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairSchedulerConfiguration.java @@ -20,10 +20,12 @@ import static org.apache.hadoop.yarn.util.resource.ResourceUtils.RESOURCE_REQUEST_VALUE_PATTERN; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.google.common.collect.Lists; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.classification.InterfaceAudience.Private; @@ -216,6 +218,15 @@ public class FairSchedulerConfiguration extends Configuration { private static final String INVALID_RESOURCE_DEFINITION_PREFIX = "Error reading resource config--invalid resource definition: "; + private static final String RESOURCE_PERCENTAGE_PATTERN = + "^(-?(\\d+)(\\.\\d*)?)\\s*%\\s*"; + private static final String RESOURCE_VALUE_PATTERN = + "^(-?\\d+)(\\.\\d*)?\\s*"; + /** + * For resources separated by spaces instead of a comma. + */ + private static final String RESOURCES_WITH_SPACES_PATTERN = + "-?\\d+(?:\\.\\d*)?\\s*[a-z]+\\s*"; public FairSchedulerConfiguration() { super(); @@ -507,7 +518,7 @@ private static ConfigurableResource parseNewStyleResource(String value, try { if (asPercent) { double percentage = parseNewStyleResourceAsPercentage(value, - resourceValue); + resourceName, resourceValue); configurableResource.setPercentage(resourceName, percentage); } else { long parsedValue = parseNewStyleResourceAsAbsoluteValue(value, @@ -526,10 +537,10 @@ private static ConfigurableResource parseNewStyleResource(String value, } private static double parseNewStyleResourceAsPercentage( - String value, String resourceValue) + String value, String resource, String resourceValue) throws AllocationConfigurationException { try { - return findPercentage(resourceValue, ""); + return findPercentage(resourceValue, resource); } catch (AllocationConfigurationException ex) { throw createConfigException(value, "The resource values must all be percentages. \"" @@ -563,18 +574,39 @@ private static ConfigurableResource parseOldStyleResourceAsPercentage( getResourcePercentage(StringUtils.toLowerCase(value))); } - private static ConfigurableResource parseOldStyleResource(String value) + private static ConfigurableResource parseOldStyleResource(String input) throws AllocationConfigurationException { - final String lCaseValue = StringUtils.toLowerCase(value); - final int memory = parseOldStyleResourceMemory(lCaseValue); - final int vcores = parseOldStyleResourceVcores(lCaseValue); + final String lowerCaseInput = StringUtils.toLowerCase(input); + String[] resources = lowerCaseInput.split(","); + + if (resources.length != 2) { + resources = findOldStyleResourcesInSpaceSeparatedInput(lowerCaseInput); + if (resources.length != 2) { + throw new AllocationConfigurationException( + "Cannot parse resource values from input: " + input); + } + } + final int memory = parseOldStyleResourceMemory(resources); + final int vcores = parseOldStyleResourceVcores(resources); return new ConfigurableResource( BuilderUtils.newResource(memory, vcores)); } - private static int parseOldStyleResourceMemory(String lCaseValue) + private static String[] findOldStyleResourcesInSpaceSeparatedInput( + String input) { + final Pattern pattern = Pattern.compile(RESOURCES_WITH_SPACES_PATTERN); + final Matcher matcher = pattern.matcher(input); + + List resources = Lists.newArrayList(); + while (matcher.find()) { + resources.add(matcher.group(0)); + } + return resources.toArray(new String[0]); + } + + private static int parseOldStyleResourceMemory(String[] resources) throws AllocationConfigurationException { - final int memory = findResource(lCaseValue, "mb"); + final int memory = findResource(resources, "mb"); if (memory < 0) { throw new AllocationConfigurationException( @@ -584,9 +616,9 @@ private static int parseOldStyleResourceMemory(String lCaseValue) return memory; } - private static int parseOldStyleResourceVcores(String lCaseValue) + private static int parseOldStyleResourceVcores(String[] resources) throws AllocationConfigurationException { - final int vcores = findResource(lCaseValue, "vcores"); + final int vcores = findResource(resources, "vcores"); if (vcores < 0) { throw new AllocationConfigurationException( @@ -596,45 +628,62 @@ private static int parseOldStyleResourceVcores(String lCaseValue) return vcores; } - private static double[] getResourcePercentage( - String val) throws AllocationConfigurationException { + private static double[] getResourcePercentage(String val) + throws AllocationConfigurationException { int numberOfKnownResourceTypes = ResourceUtils .getNumberOfCountableResourceTypes(); double[] resourcePercentage = new double[numberOfKnownResourceTypes]; - String[] strings = val.split(","); + String[] values = val.split(","); - if (strings.length == 1) { - double percentage = findPercentage(strings[0], ""); + if (values.length == 1) { + double percentage = findPercentage(values, ""); for (int i = 0; i < numberOfKnownResourceTypes; i++) { resourcePercentage[i] = percentage; } } else { - resourcePercentage[0] = findPercentage(val, "memory"); - resourcePercentage[1] = findPercentage(val, "cpu"); + resourcePercentage[0] = findPercentage(values, "memory"); + resourcePercentage[1] = findPercentage(values, "cpu"); } return resourcePercentage; } - private static double findPercentage(String val, String units) + private static double findPercentage(String resourceValue, String resource) throws AllocationConfigurationException { - final Pattern pattern = - Pattern.compile("(-?(\\d+)(\\.\\d*)?)\\s*%\\s*" + units); - Matcher matcher = pattern.matcher(val); - if (!matcher.find()) { - if (units.equals("")) { + return findPercentageInternal(resource, resourceValue, false); + } + + private static double findPercentage(String[] resourceValues, String resource) + throws AllocationConfigurationException { + String resourceValue = findResourceFromValues(resourceValues, resource); + return findPercentageInternal(resource, resourceValue, true); + } + + private static double findPercentageInternal(String resource, + String resourceValue, boolean includeResourceInPattern) + throws AllocationConfigurationException { + final Pattern pattern; + if (includeResourceInPattern) { + pattern = Pattern.compile(RESOURCE_PERCENTAGE_PATTERN + resource); + } else { + pattern = Pattern.compile(RESOURCE_PERCENTAGE_PATTERN); + } + + Matcher matcher = pattern.matcher(resourceValue); + if (!matcher.matches()) { + if (resource.equals("")) { throw new AllocationConfigurationException("Invalid percentage: " + - val); + resourceValue); } else { - throw new AllocationConfigurationException("Missing resource: " + - units); + throw new AllocationConfigurationException("Invalid percentage of " + + resource + ": " + resourceValue); } } double percentage = Double.parseDouble(matcher.group(1)) / 100.0; if (percentage < 0) { throw new AllocationConfigurationException("Invalid percentage: " + - val + ", percentage should not be negative!"); + resourceValue + ", percentage should not be negative!"); } return percentage; @@ -659,13 +708,26 @@ public long getUpdateInterval() { return getLong(UPDATE_INTERVAL_MS, DEFAULT_UPDATE_INTERVAL_MS); } - private static int findResource(String val, String units) + private static int findResource(String[] resourceValues, String resource) throws AllocationConfigurationException { - final Pattern pattern = Pattern.compile("(-?\\d+)(\\.\\d*)?\\s*" + units); - Matcher matcher = pattern.matcher(val); + String resourceValue = findResourceFromValues(resourceValues, resource); + final Pattern pattern = Pattern.compile(RESOURCE_VALUE_PATTERN + + resource); + Matcher matcher = pattern.matcher(resourceValue); if (!matcher.find()) { - throw new AllocationConfigurationException("Missing resource: " + units); + throw new AllocationConfigurationException("Invalid value of " + + (resource.equals("mb") ? "memory" : resource) + ": " + resourceValue); } return Integer.parseInt(matcher.group(1)); } + + private static String findResourceFromValues(String[] resourceValues, + String resource) throws AllocationConfigurationException { + for (String resourceValue : resourceValues) { + if (resourceValue.contains(resource)) { + return resourceValue.trim(); + } + } + throw new AllocationConfigurationException("Missing resource: " + resource); + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairSchedulerConfiguration.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairSchedulerConfiguration.java index 2a2d2683fb..522d3e5eac 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairSchedulerConfiguration.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairSchedulerConfiguration.java @@ -86,6 +86,28 @@ private void expectMissingResource(String resource) { exception.expectMessage("Missing resource: " + resource); } + private void expectUnparsableResource(String resource) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("Cannot parse resource values from input: " + + resource); + } + + private void expectInvalidResource(String resource) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("Invalid value of " + resource + ": "); + } + + private void expectInvalidResourcePercentage(String resource) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("Invalid percentage of " + resource + ": "); + } + + private void expectInvalidResourcePercentageNewStyle(String value) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("\"" + value + "\" is either " + + "not a non-negative number"); + } + private void expectNegativePercentageOldStyle() { exception.expect(AllocationConfigurationException.class); exception.expectMessage("percentage should not be negative"); @@ -106,6 +128,8 @@ public void testParseResourceConfigValue() throws Exception { Resource expected = BuilderUtils.newResource(5 * 1024, 2); Resource clusterResource = BuilderUtils.newResource(10 * 1024, 4); + assertEquals(expected, + parseResourceConfigValue("5120 mb 2 vcores").getResource()); assertEquals(expected, parseResourceConfigValue("2 vcores, 5120 mb").getResource()); assertEquals(expected, @@ -246,26 +270,30 @@ public void testParseResourceConfigValue() throws Exception { @Test public void testNoUnits() throws Exception { - expectMissingResource("mb"); - parseResourceConfigValue("1024"); + String value = "1024"; + expectUnparsableResource(value); + parseResourceConfigValue(value); } @Test public void testOnlyMemory() throws Exception { - expectMissingResource("vcores"); - parseResourceConfigValue("1024mb"); + String value = "1024mb"; + expectUnparsableResource(value); + parseResourceConfigValue(value); } @Test public void testOnlyCPU() throws Exception { - expectMissingResource("mb"); - parseResourceConfigValue("1024vcores"); + String value = "1024vcores"; + expectUnparsableResource(value); + parseResourceConfigValue(value); } @Test public void testGibberish() throws Exception { - expectMissingResource("mb"); - parseResourceConfigValue("1o24vc0res"); + String value = "1o24vc0res"; + expectUnparsableResource(value); + parseResourceConfigValue(value); } @Test @@ -276,7 +304,7 @@ public void testNoUnitsPercentage() throws Exception { @Test public void testInvalidNumPercentage() throws Exception { - expectMissingResource("cpu"); + expectInvalidResourcePercentage("cpu"); parseResourceConfigValue("95A% cpu, 50% memory"); } @@ -292,6 +320,44 @@ public void testMemoryPercentageCpuAbsolute() throws Exception { parseResourceConfigValue("50% memory, 2 vcores"); } + @Test + public void testDuplicateVcoresDefinitionAbsolute() throws Exception { + expectInvalidResource("vcores"); + parseResourceConfigValue("1024 mb, 2 4 vcores"); + } + + @Test + public void testDuplicateMemoryDefinitionAbsolute() throws Exception { + expectInvalidResource("memory"); + parseResourceConfigValue("2048 1024 mb, 2 vcores"); + } + + @Test + public void testDuplicateVcoresDefinitionPercentage() throws Exception { + expectInvalidResourcePercentage("cpu"); + parseResourceConfigValue("50% memory, 50% 100%cpu"); + } + + @Test + public void testDuplicateMemoryDefinitionPercentage() throws Exception { + expectInvalidResourcePercentage("memory"); + parseResourceConfigValue("50% 80% memory, 100%cpu"); + } + + @Test + public void testParseNewStyleDuplicateMemoryDefinitionPercentage() + throws Exception { + expectInvalidResourcePercentageNewStyle("40% 80%"); + parseResourceConfigValue("vcores = 75%, memory-mb = 40% 80%"); + } + + @Test + public void testParseNewStyleDuplicateVcoresDefinitionPercentage() + throws Exception { + expectInvalidResourcePercentageNewStyle("75% 65%"); + parseResourceConfigValue("vcores = 75% 65%, memory-mb = 40%"); + } + @Test public void testMemoryPercentageNegativeValue() throws Exception { expectNegativePercentageOldStyle(); @@ -334,7 +400,6 @@ public void testMemoryPercentageCpuAbsoluteMemoryNegative() throws Exception { parseResourceConfigValue("-50% memory, 2 vcores"); } - @Test public void testAbsoluteVcoresNegative() throws Exception { expectNegativeValueOfResource("vcores"); @@ -383,6 +448,26 @@ public void testAbsoluteMemoryNegativeFractional() throws Exception { parseResourceConfigValue(" -5120.3 mb, 2.35 vcores "); } + @Test + public void testOldStyleResourcesSeparatedBySpaces() throws Exception { + parseResourceConfigValue("2 vcores, 5120 mb"); + } + + @Test + public void testOldStyleResourcesSeparatedBySpacesInvalid() throws Exception { + String value = "2 vcores 5120 mb 555 mb"; + expectUnparsableResource(value); + parseResourceConfigValue(value); + } + + @Test + public void testOldStyleResourcesSeparatedBySpacesInvalidUppercaseUnits() + throws Exception { + String value = "2 vcores 5120 MB 555 GB"; + expectUnparsableResource(value); + parseResourceConfigValue(value); + } + @Test public void testParseNewStyleResourceMemoryNegative() throws Exception { expectNegativeValueOfResource("memory");