From a1864600049f81fae434554003a2e7046d73ccb8 Mon Sep 17 00:00:00 2001
From: Akshat Bordia <31816865+akshatb1@users.noreply.github.com>
Date: Wed, 8 Sep 2021 23:06:56 +0530
Subject: [PATCH] YARN-10829. Follow up: Adding null checks before merging
 ResourceUsage Report (#3252)

---
 .../clientrm/RouterYarnClientUtils.java       | 61 ++++++++++---------
 .../clientrm/TestRouterYarnClientUtils.java   | 42 +++++++++++++
 2 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterYarnClientUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterYarnClientUtils.java
index 9c36f30952..934636b104 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterYarnClientUtils.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterYarnClientUtils.java
@@ -133,43 +133,48 @@ private static void mergeAMWithUAM(ApplicationReport am,
     ApplicationResourceUsageReport uamResourceReport =
         uam.getApplicationResourceUsageReport();
 
-    amResourceReport.setNumUsedContainers(
-        amResourceReport.getNumUsedContainers() +
-            uamResourceReport.getNumUsedContainers());
+    if (amResourceReport == null) {
+      am.setApplicationResourceUsageReport(uamResourceReport);
+    } else if (uamResourceReport != null) {
 
-    amResourceReport.setNumReservedContainers(
-        amResourceReport.getNumReservedContainers() +
-            uamResourceReport.getNumReservedContainers());
+      amResourceReport.setNumUsedContainers(
+          amResourceReport.getNumUsedContainers() +
+              uamResourceReport.getNumUsedContainers());
 
-    amResourceReport.setUsedResources(Resources.add(
-        amResourceReport.getUsedResources(),
-        uamResourceReport.getUsedResources()));
+      amResourceReport.setNumReservedContainers(
+          amResourceReport.getNumReservedContainers() +
+              uamResourceReport.getNumReservedContainers());
 
-    amResourceReport.setReservedResources(Resources.add(
-        amResourceReport.getReservedResources(),
-        uamResourceReport.getReservedResources()));
+      amResourceReport.setUsedResources(Resources.add(
+          amResourceReport.getUsedResources(),
+          uamResourceReport.getUsedResources()));
 
-    amResourceReport.setNeededResources(Resources.add(
-        amResourceReport.getNeededResources(),
-        uamResourceReport.getNeededResources()));
+      amResourceReport.setReservedResources(Resources.add(
+          amResourceReport.getReservedResources(),
+          uamResourceReport.getReservedResources()));
 
-    amResourceReport.setMemorySeconds(
-        amResourceReport.getMemorySeconds() +
-            uamResourceReport.getMemorySeconds());
+      amResourceReport.setNeededResources(Resources.add(
+          amResourceReport.getNeededResources(),
+          uamResourceReport.getNeededResources()));
 
-    amResourceReport.setVcoreSeconds(
-        amResourceReport.getVcoreSeconds() +
-            uamResourceReport.getVcoreSeconds());
+      amResourceReport.setMemorySeconds(
+          amResourceReport.getMemorySeconds() +
+              uamResourceReport.getMemorySeconds());
 
-    amResourceReport.setQueueUsagePercentage(
-        amResourceReport.getQueueUsagePercentage() +
-            uamResourceReport.getQueueUsagePercentage());
+      amResourceReport.setVcoreSeconds(
+          amResourceReport.getVcoreSeconds() +
+              uamResourceReport.getVcoreSeconds());
 
-    amResourceReport.setClusterUsagePercentage(
-        amResourceReport.getClusterUsagePercentage() +
-            uamResourceReport.getClusterUsagePercentage());
+      amResourceReport.setQueueUsagePercentage(
+          amResourceReport.getQueueUsagePercentage() +
+              uamResourceReport.getQueueUsagePercentage());
 
-    am.setApplicationResourceUsageReport(amResourceReport);
+      amResourceReport.setClusterUsagePercentage(
+          amResourceReport.getClusterUsagePercentage() +
+              uamResourceReport.getClusterUsagePercentage());
+
+      am.setApplicationResourceUsageReport(amResourceReport);
+    }
   }
 
   /**
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java
index 3b64c23107..c91da35279 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestRouterYarnClientUtils.java
@@ -124,6 +124,48 @@ public void testMergeUnmanagedApplications() {
     Assert.assertTrue(result.getApplicationList().isEmpty());
   }
 
+  /**
+   * This test validates the correctness of
+   * RouterYarnClientUtils#mergeApplications when
+   * ApplicationResourceUsageReport might be null.
+   */
+  @Test
+  public void testMergeApplicationsNullResourceUsage() {
+    ApplicationId appId = ApplicationId.newInstance(1234, 1);
+    ApplicationReport appReport = ApplicationReport.newInstance(
+        appId, ApplicationAttemptId.newInstance(appId, 1),
+        "user", "queue", "app1", "host",
+        124, null, YarnApplicationState.RUNNING,
+        "diagnostics", "url", 0, 0,
+        0, FinalApplicationStatus.SUCCEEDED, null, "N/A",
+        0.53789f, "YARN", null, null, false, null, null, null);
+
+    ApplicationReport uamAppReport = ApplicationReport.newInstance(
+        appId, ApplicationAttemptId.newInstance(appId, 1),
+        "user", "queue", "app1", "host",
+        124, null, YarnApplicationState.RUNNING,
+        "diagnostics", "url", 0, 0,
+        0, FinalApplicationStatus.SUCCEEDED, null, "N/A",
+        0.53789f, "YARN", null, null, true, null, null, null);
+
+
+    ArrayList<GetApplicationsResponse> responses = new ArrayList<>();
+    List<ApplicationReport> applications = new ArrayList<>();
+    applications.add(appReport);
+    applications.add(uamAppReport);
+    responses.add(GetApplicationsResponse.newInstance(applications));
+
+    GetApplicationsResponse result = RouterYarnClientUtils.
+        mergeApplications(responses, false);
+    Assert.assertNotNull(result);
+    Assert.assertEquals(1, result.getApplicationList().size());
+
+    String appName = result.getApplicationList().get(0).getName();
+
+    // Check that no Unmanaged applications are added to the result
+    Assert.assertFalse(appName.contains(UnmanagedApplicationManager.APP_NAME));
+  }
+
   /**
    * This generates a GetApplicationsResponse with 2 applications with
    * same ApplicationId.