Browse Source

YARN-9785. Fix DominantResourceCalculator when one resource is zero. Contributed by Bibin A Chundatt, Sunil Govindan, Bilwa S T.

Zhankun Tang 5 years ago
parent
commit
fff4fbc957

+ 26 - 2
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java

@@ -103,7 +103,7 @@ public class DominantResourceCalculator extends ResourceCalculator {
       return 0;
       return 0;
     }
     }
 
 
-    if (isInvalidDivisor(clusterResource)) {
+    if (isAllInvalidDivisor(clusterResource)) {
       return this.compare(lhs, rhs);
       return this.compare(lhs, rhs);
     }
     }
 
 
@@ -280,6 +280,11 @@ public class DominantResourceCalculator extends ResourceCalculator {
       firstShares[i] = calculateShare(clusterRes[i], firstRes[i]);
       firstShares[i] = calculateShare(clusterRes[i], firstRes[i]);
       secondShares[i] = calculateShare(clusterRes[i], secondRes[i]);
       secondShares[i] = calculateShare(clusterRes[i], secondRes[i]);
 
 
+      if (firstShares[i] == Float.POSITIVE_INFINITY ||
+              secondShares[i] == Float.POSITIVE_INFINITY) {
+        continue;
+      }
+
       if (firstShares[i] > max[0]) {
       if (firstShares[i] > max[0]) {
         max[0] = firstShares[i];
         max[0] = firstShares[i];
       }
       }
@@ -298,7 +303,10 @@ public class DominantResourceCalculator extends ResourceCalculator {
    */
    */
   private double calculateShare(ResourceInformation clusterRes,
   private double calculateShare(ResourceInformation clusterRes,
       ResourceInformation res) {
       ResourceInformation res) {
-      // Convert the resources' units into the cluster resource's units
+    if (clusterRes.getValue() == 0) {
+      return Float.POSITIVE_INFINITY;
+    }
+    // Convert the resources' units into the cluster resource's units
     long value = UnitsConversionUtil.convert(res.getUnits(),
     long value = UnitsConversionUtil.convert(res.getUnits(),
           clusterRes.getUnits(), res.getValue());
           clusterRes.getUnits(), res.getValue());
 
 
@@ -321,6 +329,10 @@ public class DominantResourceCalculator extends ResourceCalculator {
     // lhsShares and rhsShares must necessarily have the same length, because
     // lhsShares and rhsShares must necessarily have the same length, because
     // everyone uses the same master resource list.
     // everyone uses the same master resource list.
     for (int i = lhsShares.length - 1; i >= 0; i--) {
     for (int i = lhsShares.length - 1; i >= 0; i--) {
+      if (lhsShares[i] == Float.POSITIVE_INFINITY ||
+              rhsShares[i] == Float.POSITIVE_INFINITY) {
+        continue;
+      }
       diff = lhsShares[i] - rhsShares[i];
       diff = lhsShares[i] - rhsShares[i];
 
 
       if (diff != 0.0) {
       if (diff != 0.0) {
@@ -380,6 +392,18 @@ public class DominantResourceCalculator extends ResourceCalculator {
     return false;
     return false;
   }
   }
 
 
+  public boolean isAllInvalidDivisor(Resource r) {
+    boolean flag = true;
+    for (ResourceInformation res : r.getResources()) {
+      if (flag == true && res.getValue() == 0L) {
+        flag = true;
+        continue;
+      }
+      flag = false;
+    }
+    return flag;
+  }
+
   @Override
   @Override
   public float ratio(Resource a, Resource b) {
   public float ratio(Resource a, Resource b) {
     float ratio = 0.0f;
     float ratio = 0.0f;

+ 23 - 0
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java

@@ -186,6 +186,7 @@ public class TestResourceCalculator {
       testCompareDefault(cluster);
       testCompareDefault(cluster);
     } else if (resourceCalculator instanceof DominantResourceCalculator) {
     } else if (resourceCalculator instanceof DominantResourceCalculator) {
       testCompareDominant(cluster);
       testCompareDominant(cluster);
+      testCompareDominantZeroValueResource();
     }
     }
   }
   }
 
 
@@ -201,6 +202,28 @@ public class TestResourceCalculator {
     assertComparison(cluster, newResource(2, 1, 1), newResource(1, 0, 0), 1);
     assertComparison(cluster, newResource(2, 1, 1), newResource(1, 0, 0), 1);
   }
   }
 
 
+  /**
+   * Verify compare when one or all the resource are zero.
+   */
+  private void testCompareDominantZeroValueResource(){
+    Resource cluster = newResource(4L, 4, 0);
+    assertComparison(cluster, newResource(2, 1, 1), newResource(1, 1, 2), 1);
+    assertComparison(cluster, newResource(2, 2, 1), newResource(1, 2, 2), 1);
+    assertComparison(cluster, newResource(2, 2, 1), newResource(2, 2, 2), 0);
+    assertComparison(cluster, newResource(0, 2, 1), newResource(0, 2, 2), 0);
+    assertComparison(cluster, newResource(0, 1, 2), newResource(1, 1, 2), -1);
+    assertComparison(cluster, newResource(1, 1, 2), newResource(2, 1, 2), -1);
+
+    // cluster resource zero
+    cluster = newResource(0, 0, 0);
+    assertComparison(cluster, newResource(2, 1, 1), newResource(1, 1, 1), 1);
+    assertComparison(cluster, newResource(2, 2, 2), newResource(1, 1, 1), 1);
+    assertComparison(cluster, newResource(2, 1, 1), newResource(1, 2, 1), 0);
+    assertComparison(cluster, newResource(1, 1, 1), newResource(1, 1, 1), 0);
+    assertComparison(cluster, newResource(1, 1, 1), newResource(1, 1, 2), -1);
+    assertComparison(cluster, newResource(1, 1, 1), newResource(1, 2, 1), -1);
+  }
+
   private void testCompareDominant(Resource cluster) {
   private void testCompareDominant(Resource cluster) {
     assertComparison(cluster, newResource(2, 1, 1), newResource(2, 1, 1), 0);
     assertComparison(cluster, newResource(2, 1, 1), newResource(2, 1, 1), 0);
     assertComparison(cluster, newResource(2, 1, 1), newResource(1, 2, 1), 0);
     assertComparison(cluster, newResource(2, 1, 1), newResource(1, 2, 1), 0);