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 16, 2024 · c63aafd · c63aafd
1 parent 78a08b3
commit c63aafd
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 9 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
@@ -876,10 +876,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/...p/yarn/server/resourcemanager/scheduler/capacity/allocator/RegularContainerAllocator.java b/...p/yarn/server/resourcemanager/scheduler/capacity/allocator/RegularContainerAllocator.java
@@ -858,12 +858,13 @@ private ContainerAllocation allocate(Resource clusterResource,
  FiCaSchedulerNode node = iter.next();
 
  // Do not schedule if there are any reservations to fulfill on the node
+ RMContainer nodeReservedContainer = node.getReservedContainer();
  if (iter.hasNext() &&
- node.getReservedContainer() != null &&
+ nodeReservedContainer != null &&
  isSkipAllocateOnNodesWithReservedContainer()) {
  LOG.debug("Skipping scheduling on node {} since it has already been"
  + " reserved by {}", node.getNodeID(),
- node.getReservedContainer().getContainerId());
+ nodeReservedContainer.getContainerId());
  ActivitiesLogger.APP.recordSkippedAppActivityWithoutAllocation(
  activitiesManager, node, application, schedulerKey,
  ActivityDiagnosticConstant.NODE_HAS_BEEN_RESERVED, ActivityLevel.NODE);

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;
  }