YARN-3434. Interaction between reservations and userlimit can result in significant ULF violation

Author: tgravescs

hadoop-yarn-project/CHANGES.txt

@@ -252,6 +252,9 @@ Release 2.8.0 - UNRELEASED
     YARN-3495. Confusing log generated by FairScheduler.
     (Brahma Reddy Battula via ozawa)
 
+    YARN-3434. Interaction between reservations and userlimit can result in 
+    significant ULF violation (tgraves)
+
 Release 2.7.1 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/ResourceLimits.java

@@ -19,22 +19,44 @@
 package org.apache.hadoop.yarn.server.resourcemanager.scheduler;
 
 import org.apache.hadoop.yarn.api.records.Resource;
+import org.apache.hadoop.yarn.util.resource.Resources;
 
 /**
  * Resource limits for queues/applications, this means max overall (please note
  * that, it's not "extra") resource you can get.
  */
 public class ResourceLimits {
+  volatile Resource limit;
+
+  // This is special limit that goes with the RESERVE_CONT_LOOK_ALL_NODES
+  // config. This limit indicates how much we need to unreserve to allocate
+  // another container.
+  private volatile Resource amountNeededUnreserve;
+
   public ResourceLimits(Resource limit) {
+    this.amountNeededUnreserve = Resources.none();
     this.limit = limit;
   }
-  
-  volatile Resource limit;
+    this.amountNeededUnreserve = amountNeededUnreserve;
+    this.limit = limit;
+  }
+
   public Resource getLimit() {
     return limit;
   }
-  
+  public Resource getAmountNeededUnreserve() {
+    return amountNeededUnreserve;
+  }
+
   public void setLimit(Resource limit) {
     this.limit = limit;
   }
+
+  public void setAmountNeededUnreserve(Resource amountNeededUnreserve) {
+    this.amountNeededUnreserve = amountNeededUnreserve;
+  }
+
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java

@@ -85,7 +85,7 @@ public abstract class AbstractCSQueue implements CSQueue {
   // Track capacities like used-capcity/abs-used-capacity/capacity/abs-capacity,
   // etc.
   QueueCapacities queueCapacities;
-  
   private final RecordFactory recordFactory = 
       RecordFactoryProvider.getRecordFactory(null);
   protected CapacitySchedulerContext csContext;
@@ -473,55 +473,55 @@ synchronized boolean canAssignToThisQueue(Resource clusterResource,
         getCurrentLimitResource(nodePartition, clusterResource,
             currentResourceLimits, schedulingMode);
 
-    // if reservation continous looking enabled, check to see if could we
-    // potentially use this node instead of a reserved node if the application
-    // has reserved containers.
-    // TODO, now only consider reservation cases when the node has no label
-    if (this.reservationsContinueLooking
-        && nodePartition.equals(RMNodeLabelsManager.NO_LABEL)
-        && Resources.greaterThan(resourceCalculator, clusterResource,
-            resourceCouldBeUnreserved, Resources.none())) {
-      // resource-without-reserved = used - reserved
-      Resource newTotalWithoutReservedResource =
-          Resources.subtract(newTotalResource, resourceCouldBeUnreserved);
-
-      // when total-used-without-reserved-resource < currentLimit, we still
-      // have chance to allocate on this node by unreserving some containers
-      if (Resources.lessThan(resourceCalculator, clusterResource,
-          newTotalWithoutReservedResource, currentLimitResource)) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("try to use reserved: " + getQueueName()
-              + " usedResources: " + queueUsage.getUsed()
-              + ", clusterResources: " + clusterResource
-              + ", reservedResources: " + resourceCouldBeUnreserved
-              + ", capacity-without-reserved: "
-              + newTotalWithoutReservedResource + ", maxLimitCapacity: "
-              + currentLimitResource);
-        }
-        return true;
-      }
-    }
-
-    // Check if we over current-resource-limit computed.
     if (Resources.greaterThan(resourceCalculator, clusterResource,
         newTotalResource, currentLimitResource)) {
-      return false;
-    }
 
-    if (LOG.isDebugEnabled()) {
-      LOG.debug(getQueueName()
-          + "Check assign to queue, nodePartition="
-          + nodePartition
-          + " usedResources: "
-          + queueUsage.getUsed(nodePartition)
-          + " clusterResources: "
-          + clusterResource
-          + " currentUsedCapacity "
-          + Resources.divide(resourceCalculator, clusterResource,
-              queueUsage.getUsed(nodePartition),
-              labelManager.getResourceByLabel(nodePartition, clusterResource))
-          + " max-capacity: "
-          + queueCapacities.getAbsoluteMaximumCapacity(nodePartition) + ")");
+        if (Resources.lessThan(resourceCalculator, clusterResource,
+            newTotalWithoutReservedResource, currentLimitResource)) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("try to use reserved: " + getQueueName()
+                + " usedResources: " + queueUsage.getUsed()
+                + ", clusterResources: " + clusterResource
+                + ", reservedResources: " + resourceCouldBeUnreserved
+                + ", capacity-without-reserved: "
+                + newTotalWithoutReservedResource + ", maxLimitCapacity: "
+                + currentLimitResource);
+          }
+          currentResourceLimits.setAmountNeededUnreserve(Resources.subtract(newTotalResource,
+            currentLimitResource));
+          return true;
+        }
+      }
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(getQueueName()
+            + "Check assign to queue, nodePartition="
+            + nodePartition
+            + " usedResources: "
+            + queueUsage.getUsed(nodePartition)
+            + " clusterResources: "
+            + clusterResource
+            + " currentUsedCapacity "
+            + Resources.divide(resourceCalculator, clusterResource,
+                queueUsage.getUsed(nodePartition),
+                labelManager.getResourceByLabel(nodePartition, clusterResource))
+            + " max-capacity: "
+            + queueCapacities.getAbsoluteMaximumCapacity(nodePartition) + ")");
+      }
+      return false;
     }
     return true;
   }