From 912b1f9d64a61ef2663d95e2b4f286e6ee8d5ff9 Mon Sep 17 00:00:00 2001
From: Haibo Chen <haibochen@apache.org>
Date: Wed, 5 Dec 2018 15:15:30 -0800
Subject: [PATCH] YARN-9019. Ratio calculation of ResourceCalculator
 implementations could return NaN. (Contributed by Szilard Nemeth)

---
 .../resource/DefaultResourceCalculator.java   |  2 +-
 .../resource/DominantResourceCalculator.java  |  4 +--
 .../util/resource/ResourceCalculator.java     | 18 +++++++++++++
 .../util/resource/TestResourceCalculator.java | 26 ++++++++++++++-----
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java
index ab6d7f5748..9a3f7037ee 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java
@@ -57,7 +57,7 @@ public class DefaultResourceCalculator extends ResourceCalculator {
 
   @Override
   public float ratio(Resource a, Resource b) {
-    return (float)a.getMemorySize() / b.getMemorySize();
+    return divideSafelyAsFloat(a.getMemorySize(), b.getMemorySize());
   }
 
   @Override
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java
index 9aeb51cc2c..29d7e7ed3b 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java
@@ -379,8 +379,8 @@ public class DominantResourceCalculator extends ResourceCalculator {
     for (int i = 0; i < maxLength; i++) {
       ResourceInformation aResourceInformation = a.getResourceInformation(i);
       ResourceInformation bResourceInformation = b.getResourceInformation(i);
-      float tmp = (float) aResourceInformation.getValue()
-          / (float) bResourceInformation.getValue();
+      final float tmp = divideSafelyAsFloat(aResourceInformation.getValue(),
+          bResourceInformation.getValue());
       ratio = ratio > tmp ? ratio : tmp;
     }
     return ratio;
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java
index 27394f73a7..09d5ec18a8 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java
@@ -87,6 +87,24 @@ public abstract class ResourceCalculator {
     return (long) Math.ceil(a/b);
   }
 
+  /**
+   * Divides lhs by rhs.
+   * If both lhs and rhs are having a value of 0, then we return 0.
+   * This is to avoid division by zero and return NaN as a result.
+   * If lhs is zero but rhs is not, Float.infinity will be returned
+   * as the result.
+   * @param lhs
+   * @param rhs
+   * @return
+   */
+  public static float divideSafelyAsFloat(long lhs, long rhs) {
+    if (lhs == 0 && rhs == 0) {
+      return 0;
+    } else {
+      return (float) lhs / (float) rhs;
+    }
+  }
+
   public static int roundUp(int a, int b) {
     return divideAndCeil(a, b) * b;
   }
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java
index 5f3ed19604..bf04557ebb 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java
@@ -36,11 +36,11 @@ import static org.junit.Assert.assertEquals;
 public class TestResourceCalculator {
   private final ResourceCalculator resourceCalculator;
 
-  @Parameterized.Parameters
-  public static Collection<ResourceCalculator[]> getParameters() {
-    return Arrays.asList(new ResourceCalculator[][] {
-        { new DefaultResourceCalculator() },
-        { new DominantResourceCalculator() } });
+  @Parameterized.Parameters(name = "{0}")
+  public static Collection<Object[]> getParameters() {
+    return Arrays.asList(new Object[][] {
+        { "DefaultResourceCalculator", new DefaultResourceCalculator() },
+        { "DominantResourceCalculator", new DominantResourceCalculator() } });
   }
 
   @Before
@@ -57,7 +57,7 @@ public class TestResourceCalculator {
     ResourceUtils.resetResourceTypes(conf);
   }
 
-  public TestResourceCalculator(ResourceCalculator rs) {
+  public TestResourceCalculator(String name, ResourceCalculator rs) {
     this.resourceCalculator = rs;
   }
   
@@ -392,4 +392,18 @@ public class TestResourceCalculator {
       assertEquals(2, result.getVirtualCores());
     }
   }
+
+  @Test
+  public void testDivisionByZeroRatioDenominatorIsZero() {
+    float ratio = resourceCalculator.ratio(newResource(1, 1), newResource(0,
+        0));
+    assertEquals(Float.POSITIVE_INFINITY, ratio, 0.00001);
+  }
+
+  @Test
+  public void testDivisionByZeroRatioNumeratorAndDenominatorIsZero() {
+    float ratio = resourceCalculator.ratio(newResource(0, 0), newResource(0,
+        0));
+    assertEquals(0.0, ratio, 0.00001);
+  }
 }
\ No newline at end of file