YARN-7270. Fix unsafe casting from long to int for class Resource and its sub-classes. (Yufei)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/7a27c2c3 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/7a27c2c3 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/7a27c2c3 Branch: refs/heads/YARN-1011 Commit: 7a27c2c367518e1bf6ee393391a2f9b412113819 Parents: f4fb669 Author: Yufei Gu <yu...@apache.org> Authored: Fri Oct 13 12:38:58 2017 -0700 Committer: Yufei Gu <yu...@apache.org> Committed: Fri Oct 13 12:41:59 2017 -0700 ---------------------------------------------------------------------- .../hadoop/yarn/api/records/Resource.java | 14 +++++++ .../api/records/impl/LightWeightResource.java | 4 +- .../hadoop/yarn/api/records/TestResource.java | 43 ++++++++++++++++++++ .../api/records/impl/pb/ResourcePBImpl.java | 4 +- .../hadoop/yarn/util/resource/Resources.java | 11 +---- .../hadoop/yarn/api/TestResourcePBImpl.java | 27 ++++++++++++ 6 files changed, 90 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Resource.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Resource.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Resource.java index acd0e60..9a5bc79 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Resource.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Resource.java @@ -466,4 +466,18 @@ public abstract class Resource implements Comparable<Resource> { } return (int) result; } + + /** + * Convert long to int for a resource value safely. This method assumes + * resource value is positive. + * + * @param value long resource value + * @return int resource value + */ + protected static int castToIntSafely(long value) { + if (value > Integer.MAX_VALUE) { + return Integer.MAX_VALUE; + } + return Long.valueOf(value).intValue(); + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/LightWeightResource.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/LightWeightResource.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/LightWeightResource.java index b80e133..a64d242 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/LightWeightResource.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/LightWeightResource.java @@ -92,7 +92,7 @@ public class LightWeightResource extends Resource { @Override @SuppressWarnings("deprecation") public int getMemory() { - return (int) memoryResInfo.getValue(); + return castToIntSafely(memoryResInfo.getValue()); } @Override @@ -113,7 +113,7 @@ public class LightWeightResource extends Resource { @Override public int getVirtualCores() { - return (int) vcoresResInfo.getValue(); + return castToIntSafely(vcoresResInfo.getValue()); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestResource.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestResource.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestResource.java new file mode 100644 index 0000000..e0ec370 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestResource.java @@ -0,0 +1,43 @@ +/** + * 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.api.records; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * The class to test {@link Resource}. + */ +public class TestResource { + + @Test + public void testCastToIntSafely() { + assertEquals(0, Resource.castToIntSafely(0)); + assertEquals(1, Resource.castToIntSafely(1)); + assertEquals(Integer.MAX_VALUE, + Resource.castToIntSafely(Integer.MAX_VALUE)); + + assertEquals("Cast to Integer.MAX_VALUE if the long is greater than " + + "Integer.MAX_VALUE", Integer.MAX_VALUE, + Resource.castToIntSafely(Integer.MAX_VALUE + 1L)); + assertEquals("Cast to Integer.MAX_VALUE if the long is greater than " + + "Integer.MAX_VALUE", Integer.MAX_VALUE, + Resource.castToIntSafely(Long.MAX_VALUE)); + } +} http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ResourcePBImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ResourcePBImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ResourcePBImpl.java index 92beec7..4ae64c2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ResourcePBImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ResourcePBImpl.java @@ -87,7 +87,7 @@ public class ResourcePBImpl extends Resource { @Override @SuppressWarnings("deprecation") public int getMemory() { - return (int) this.getMemorySize(); + return castToIntSafely(this.getMemorySize()); } @Override @@ -117,7 +117,7 @@ public class ResourcePBImpl extends Resource { @Override public int getVirtualCores() { // vcores should always be present - return (int) resources[VCORES_INDEX].getValue(); + return castToIntSafely(resources[VCORES_INDEX].getValue()); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.java index 793aebf..3690946 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.java @@ -60,17 +60,10 @@ public class Resources { initResourceMap(); } - private int resourceValueToInt() { - if(this.resourceValue > Integer.MAX_VALUE) { - return Integer.MAX_VALUE; - } - return Long.valueOf(this.resourceValue).intValue(); - } - @Override @SuppressWarnings("deprecation") public int getMemory() { - return resourceValueToInt(); + return castToIntSafely(resourceValue); } @Override @@ -91,7 +84,7 @@ public class Resources { @Override public int getVirtualCores() { - return resourceValueToInt(); + return castToIntSafely(resourceValue); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/7a27c2c3/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/TestResourcePBImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/TestResourcePBImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/TestResourcePBImpl.java index 569a7b7..4887b50 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/TestResourcePBImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/TestResourcePBImpl.java @@ -25,6 +25,8 @@ import org.apache.hadoop.yarn.proto.YarnProtos; import org.junit.Assert; import org.junit.Test; +import static org.junit.Assert.assertEquals; + /** * Test class to handle various proto related tests for resources. */ @@ -58,4 +60,29 @@ public class TestResourcePBImpl { res.getResourceInformation(ResourceInformation.VCORES.getName()) .getUnits()); } + + @Test + @SuppressWarnings("deprecation") + public void testGetMemory() { + Resource res = new ResourcePBImpl(); + long memorySize = Integer.MAX_VALUE + 1L; + res.setMemorySize(memorySize); + + assertEquals("No need to cast if both are long", memorySize, + res.getMemorySize()); + assertEquals("Cast to Integer.MAX_VALUE if the long is greater than " + + "Integer.MAX_VALUE", Integer.MAX_VALUE, res.getMemory()); + } + + @Test + public void testGetVirtualCores() { + Resource res = new ResourcePBImpl(); + long vcores = Integer.MAX_VALUE + 1L; + res.getResourceInformation("vcores").setValue(vcores); + + assertEquals("No need to cast if both are long", vcores, + res.getResourceInformation("vcores").getValue()); + assertEquals("Cast to Integer.MAX_VALUE if the long is greater than " + + "Integer.MAX_VALUE", Integer.MAX_VALUE, res.getVirtualCores()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
