YARN-11732. Fix potential NPE when calling SchedulerNode#reservedCont…

…ainer for CapacityScheduler (#7065). Contributed by Tao Yang. Reviewed-by: Syed Shameerur Rahman <[email protected]> Signed-off-by: He Xiaoqiao <[email protected]>
apache · Oct 17, 2024 · c36c72a · c36c72a
1 parent 870a216
commit c36c72a
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 7 deletions.
diff --git a/...oop/yarn/server/resourcemanager/monitor/capacity/ReservedContainerCandidatesSelector.java b/...oop/yarn/server/resourcemanager/monitor/capacity/ReservedContainerCandidatesSelector.java
@@ -170,6 +170,9 @@ private NodeForPreemption getPreemptionCandidatesOnNode(
       Map<ApplicationAttemptId, Set<RMContainer>> selectedCandidates,
       Resource totalPreemptionAllowed, boolean readOnly) {
     RMContainer reservedContainer = node.getReservedContainer();
+    if (reservedContainer == null) {
+      return null;
+    }
     Resource available = Resources.clone(node.getUnallocatedResource());
     Resource totalSelected = Resources.createResource(0);
     List<RMContainer> sortedRunningContainers =

diff --git a/...n/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java b/...n/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java
@@ -743,10 +743,9 @@ private void completeOustandingUpdatesWhichAreReserved(
       RMContainer rmContainer, ContainerStatus containerStatus,
       RMContainerEventType event) {
     N schedulerNode = getSchedulerNode(rmContainer.getNodeId());
-    if (schedulerNode != null &&
-        schedulerNode.getReservedContainer() != null) {
+    if (schedulerNode != null) {
       RMContainer resContainer = schedulerNode.getReservedContainer();
-      if (resContainer.getReservedSchedulerKey() != null) {
+      if (resContainer != null && resContainer.getReservedSchedulerKey() != null) {
         ContainerId containerToUpdate = resContainer
             .getReservedSchedulerKey().getContainerToUpdate();
         if (containerToUpdate != null &&

diff --git a/...org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java b/...org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java
@@ -520,13 +520,13 @@ public boolean accept(Resource cluster,
             // When reserve a resource (state == NEW is for new container,
             // state == RUNNING is for increase container).
             // Just check if the node is not already reserved by someone
-            if (schedulerContainer.getSchedulerNode().getReservedContainer()
-                != null) {
+            RMContainer reservedContainer =
+                schedulerContainer.getSchedulerNode().getReservedContainer();
+            if (reservedContainer != null) {
               if (LOG.isDebugEnabled()) {
                 LOG.debug("Try to reserve a container, but the node is "
                     + "already reserved by another container="
-                    + schedulerContainer.getSchedulerNode()
-                    .getReservedContainer().getContainerId());
+                    + reservedContainer.getContainerId());
               }
               return false;
             }