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 8c4932bfe67..3138dd42acb 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 @@ -506,11 +506,13 @@ private static ConfigurableResource parseNewStyleResource(String value, String resourceValue = parts[1].trim(); try { if (asPercent) { - configurableResource.setPercentage(resourceName, - findPercentage(resourceValue, "")); + double percentage = parseNewStyleResourceAsPercentage(value, + resourceValue); + configurableResource.setPercentage(resourceName, percentage); } else { - configurableResource.setValue(resourceName, - Long.parseLong(resourceValue)); + long parsedValue = parseNewStyleResourceAsAbsoluteValue(value, + resourceValue, resourceName); + configurableResource.setValue(resourceName, parsedValue); } } catch (ResourceNotFoundException ex) { throw createConfigException(value, "The " @@ -518,22 +520,45 @@ private static ConfigurableResource parseNewStyleResource(String value, + "recognized. Please check the value of " + YarnConfiguration.RESOURCE_TYPES + " in the Resource " + "Manager's configuration files.", ex); - } catch (NumberFormatException ex) { - // This only comes from Long.parseLong() - throw createConfigException(value, "The " - + "resource values must all be integers. \"" + resourceValue - + "\" is not an integer.", ex); - } catch (AllocationConfigurationException ex) { - // This only comes from findPercentage() - throw createConfigException(value, "The " - + "resource values must all be percentages. \"" - + resourceValue + "\" is either not a number or does not " - + "include the '%' symbol.", ex); } } return configurableResource; } + private static double parseNewStyleResourceAsPercentage( + String value, String resourceValue) + throws AllocationConfigurationException { + try { + return findPercentage(resourceValue, ""); + } catch (NegativeResourceDefinitionException ex) { + throw ex; + } catch (AllocationConfigurationException ex) { + throw createConfigException(value, + "The resource values must all be percentages. \"" + + resourceValue + "\" is either not a number or does not " + + "include the '%' symbol.", ex); + } + } + + private static long parseNewStyleResourceAsAbsoluteValue(String value, + String resourceValue, String resourceName) + throws AllocationConfigurationException { + final long parsedValue; + try { + parsedValue = Long.parseLong(resourceValue); + } catch (NumberFormatException e) { + throw createConfigException(value, "The " + + "resource values must all be integers. \"" + resourceValue + + "\" is not an integer.", e); + } + if (parsedValue < 0) { + throw new NegativeResourceDefinitionException( + "Invalid value of " + resourceName + + ": " + parsedValue + ", value should not be negative!"); + } + return parsedValue; + } + private static ConfigurableResource parseOldStyleResourceAsPercentage( String value) throws AllocationConfigurationException { return new ConfigurableResource( @@ -543,13 +568,36 @@ private static ConfigurableResource parseOldStyleResourceAsPercentage( private static ConfigurableResource parseOldStyleResource(String value) throws AllocationConfigurationException { final String lCaseValue = StringUtils.toLowerCase(value); - int memory = findResource(lCaseValue, "mb"); - int vcores = findResource(lCaseValue, "vcores"); - + final int memory = parseOldStyleResourceMemory(lCaseValue); + final int vcores = parseOldStyleResourceVcores(lCaseValue); return new ConfigurableResource( BuilderUtils.newResource(memory, vcores)); } + private static int parseOldStyleResourceMemory(String lCaseValue) + throws AllocationConfigurationException { + final int memory = findResource(lCaseValue, "mb"); + + if (memory < 0) { + throw new NegativeResourceDefinitionException( + "Invalid value of memory: " + memory + + ", value should not be negative!"); + } + return memory; + } + + private static int parseOldStyleResourceVcores(String lCaseValue) + throws AllocationConfigurationException { + final int vcores = findResource(lCaseValue, "vcores"); + + if (vcores < 0) { + throw new NegativeResourceDefinitionException( + "Invalid value of vcores: " + vcores + + ", value should not be negative!"); + } + return vcores; + } + private static double[] getResourcePercentage( String val) throws AllocationConfigurationException { int numberOfKnownResourceTypes = ResourceUtils @@ -573,7 +621,7 @@ private static ConfigurableResource parseOldStyleResource(String value) private static double findPercentage(String val, String units) throws AllocationConfigurationException { final Pattern pattern = - Pattern.compile("((\\d+)(\\.\\d*)?)\\s*%\\s*" + units); + Pattern.compile("(-?(\\d+)(\\.\\d*)?)\\s*%\\s*" + units); Matcher matcher = pattern.matcher(val); if (!matcher.find()) { if (units.equals("")) { @@ -584,7 +632,14 @@ private static double findPercentage(String val, String units) units); } } - return Double.parseDouble(matcher.group(1)) / 100.0; + double percentage = Double.parseDouble(matcher.group(1)) / 100.0; + + if (percentage < 0) { + throw new NegativeResourceDefinitionException("Invalid percentage: " + + val + ", percentage should not be negative!"); + } + + return percentage; } private static AllocationConfigurationException createConfigException( @@ -608,7 +663,7 @@ public long getUpdateInterval() { private static int findResource(String val, String units) throws AllocationConfigurationException { - final Pattern pattern = Pattern.compile("(\\d+)(\\.\\d*)?\\s*" + units); + final Pattern pattern = Pattern.compile("(-?\\d+)(\\.\\d*)?\\s*" + units); Matcher matcher = pattern.matcher(val); if (!matcher.find()) { throw new AllocationConfigurationException("Missing resource: " + units); 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/NegativeResourceDefinitionException.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/NegativeResourceDefinitionException.java new file mode 100644 index 00000000000..81eb456c8b8 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/NegativeResourceDefinitionException.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair; + +import org.apache.hadoop.classification.InterfaceAudience.Private; +import org.apache.hadoop.classification.InterfaceStability.Unstable; + +/** + * Thrown when the allocation file for {@link QueueManager} is malformed + * with a negative resource definition. + */ +@Private +@Unstable +public class NegativeResourceDefinitionException extends + AllocationConfigurationException { + private static final long serialVersionUID = 4046517047810854249L; + + NegativeResourceDefinitionException(String message) { + super(message); + } +} 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 69d3ab9ada0..4ab85cb7f89 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 @@ -46,7 +46,9 @@ import org.apache.log4j.Level; import org.apache.log4j.spi.LoggingEvent; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; /** * Tests fair scheduler configuration. @@ -103,6 +105,14 @@ protected void append(LoggingEvent arg0) { } } + @Rule + public ExpectedException exception = ExpectedException.none(); + + private void expectMissingResource(String resource) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("Missing resource: " + resource); + } + @Test public void testParseResourceConfigValue() throws Exception { Resource expected = BuilderUtils.newResource(5 * 1024, 2); @@ -246,48 +256,56 @@ public void testParseResourceConfigValue() throws Exception { + "test1 = 50 % ").getResource(clusterResource)); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testNoUnits() throws Exception { + expectMissingResource("mb"); parseResourceConfigValue("1024"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testOnlyMemory() throws Exception { + expectMissingResource("vcores"); parseResourceConfigValue("1024mb"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testOnlyCPU() throws Exception { + expectMissingResource("mb"); parseResourceConfigValue("1024vcores"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testGibberish() throws Exception { + expectMissingResource("mb"); parseResourceConfigValue("1o24vc0res"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testNoUnitsPercentage() throws Exception { + expectMissingResource("cpu"); parseResourceConfigValue("95%, 50% memory"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testInvalidNumPercentage() throws Exception { + expectMissingResource("cpu"); parseResourceConfigValue("95A% cpu, 50% memory"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testCpuPercentageMemoryAbsolute() throws Exception { + expectMissingResource("memory"); parseResourceConfigValue("50% cpu, 1024 mb"); } - @Test(expected = AllocationConfigurationException.class) + @Test public void testMemoryPercentageCpuAbsolute() throws Exception { + expectMissingResource("cpu"); parseResourceConfigValue("50% memory, 2 vcores"); } @Test - public void testAllocationIncrementMemoryDefaultUnit() throws Exception { + public void testAllocationIncrementMemoryDefaultUnit() { Configuration conf = new Configuration(); conf.set(YarnConfiguration.RESOURCE_TYPES + "." + ResourceInformation.MEMORY_MB.getName() + 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/TestFairSchedulerConfigurationNegativeResourceValues.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/TestFairSchedulerConfigurationNegativeResourceValues.java new file mode 100644 index 00000000000..60ecdaec0f9 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairSchedulerConfigurationNegativeResourceValues.java @@ -0,0 +1,252 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairSchedulerConfiguration.parseResourceConfigValue; + +public class TestFairSchedulerConfigurationNegativeResourceValues { + + @Rule + public ExpectedException exception = ExpectedException.none(); + + private void expectMissingResource(String resource) { + exception.expect(AllocationConfigurationException.class); + exception.expectMessage("Missing resource: " + resource); + } + + private void expectNegativePercentage() { + exception.expect(NegativeResourceDefinitionException.class); + exception.expectMessage("percentage should not be negative"); + } + + private void expectNegativeValueOfResource(String resource) { + exception.expect(NegativeResourceDefinitionException.class); + exception.expectMessage("Invalid value of " + resource); + } + + @Test + public void testMemoryPercentageNegativeValue() throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("-10% memory, 50% cpu"); + } + + @Test + public void testCpuPercentageNegativeValue() throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("10% memory, -10% cpu"); + } + + @Test + public void testMemoryAndCpuPercentageNegativeValue() throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("-20% memory, -10% cpu"); + } + + @Test + public void testCpuPercentageMemoryAbsoluteCpuNegative() throws Exception { + expectMissingResource("memory"); + parseResourceConfigValue("-50% cpu, 1024 mb"); + } + + @Test + public void testCpuPercentageMemoryAbsoluteMemoryNegative() throws Exception { + expectMissingResource("memory"); + parseResourceConfigValue("50% cpu, -1024 mb"); + } + + @Test + public void testMemoryPercentageCpuAbsoluteCpuNegative() throws Exception { + expectMissingResource("cpu"); + parseResourceConfigValue("50% memory, -2 vcores"); + } + + @Test + public void testMemoryPercentageCpuAbsoluteMemoryNegative() throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("-50% memory, 2 vcores"); + } + + + @Test + public void testAbsoluteVcoresNegative() throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("-2 vcores,5120 mb"); + } + + @Test + public void testAbsoluteMemoryNegative() throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("2 vcores,-5120 mb"); + } + + @Test + public void testAbsoluteVcoresNegativeWithSpaces() throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("-2 vcores, 5120 mb"); + } + + @Test + public void testAbsoluteMemoryNegativeWithSpaces() throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("2 vcores, -5120 mb"); + } + + @Test + public void testAbsoluteVcoresNegativeWithMoreSpaces() throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("5120mb mb, -2 vcores"); + } + + @Test + public void testAbsoluteMemoryNegativeWithMoreSpaces() throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("-5120mb mb, 2 vcores"); + } + + @Test + public void testAbsoluteVcoresNegativeFractional() throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue(" 5120.3 mb, -2.35 vcores "); + } + + @Test + public void testAbsoluteMemoryNegativeFractional() throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue(" -5120.3 mb, 2.35 vcores "); + } + + @Test + public void testParseNewStyleResourceMemoryNegative() throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("memory-mb=-5120,vcores=2"); + } + + @Test + public void testParseNewStyleResourceVcoresNegative() throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("memory-mb=5120,vcores=-2"); + } + + @Test + public void testParseNewStyleResourceMemoryNegativeWithSpaces() + throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("memory-mb=-5120, vcores=2"); + } + + @Test + public void testParseNewStyleResourceVcoresNegativeWithSpaces() + throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("memory-mb=5120, vcores=-2"); + } + + @Test + public void testParseNewStyleResourceMemoryNegativeWithMoreSpaces() + throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue(" vcores = 2 , memory-mb = -5120 "); + } + + @Test + public void testParseNewStyleResourceVcoresNegativeWithMoreSpaces() + throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue(" vcores = -2 , memory-mb = 5120 "); + } + + @Test + public void testParseNewStyleResourceWithCustomResourceMemoryNegative() + throws Exception { + expectNegativeValueOfResource("memory"); + parseResourceConfigValue("vcores=2,memory-mb=-5120,test1=4"); + } + + @Test + public void testParseNewStyleResourceWithCustomResourceVcoresNegative() + throws Exception { + expectNegativeValueOfResource("vcores"); + parseResourceConfigValue("vcores=-2,memory-mb=-5120,test1=4"); + } + + @Test + public void testParseNewStyleResourceWithCustomResourceNegative() + throws Exception { + expectNegativeValueOfResource("test1"); + parseResourceConfigValue("vcores=2,memory-mb=5120,test1=-4"); + } + + @Test + public void testParseNewStyleResourceWithCustomResourceNegativeWithSpaces() + throws Exception { + expectNegativeValueOfResource("test1"); + parseResourceConfigValue(" vcores = 2 , memory-mb = 5120 , test1 = -4 "); + } + + @Test + public void testParseNewStyleResourceWithPercentagesVcoresNegative() throws + Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores=-75%,memory-mb=40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesMemoryNegative() throws + Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores=75%,memory-mb=-40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesVcoresNegativeWithSpaces() + throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores=-75%, memory-mb=40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesMemoryNegativeWithSpaces() + throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores=75%, memory-mb=-40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesVcoresNegativeWithMoreSpaces() + throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores = -75%, memory-mb = 40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesMemoryNegativeWithMoreSpaces() + throws Exception { + expectNegativePercentage(); + parseResourceConfigValue("vcores = 75%, memory-mb = -40%"); + } + + @Test + public void testParseNewStyleResourceWithPercentagesCustomResourceNegativeWithSpaces() + throws Exception { + expectNegativeValueOfResource("test1"); + parseResourceConfigValue(" vcores = 2 , memory-mb = 5120 , test1 = -4 "); + } +}