YARN-2602. Fixed possible NPE in ApplicationHistoryManagerOnTimelineStore. Contributed by Zhijie Shen

hadoop-yarn-project/CHANGES.txt

@@ -491,6 +491,9 @@ Release 2.6.0 - UNRELEASED
     YARN-2594. Potential deadlock in RM when querying 
     ApplicationResourceUsageReport. (Wangda Tan via kasha)
 
+    YARN-2602. Fixed possible NPE in ApplicationHistoryManagerOnTimelineStore.
+    (Zhijie Shen via jianhe)
+
 Release 2.5.1 - 2014-09-05
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java

@@ -227,8 +227,10 @@ public class ApplicationHistoryManagerOnTimelineStore extends AbstractService
       if (entityInfo.containsKey(ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO)) {
         String appViewACLsStr = entityInfo.get(
             ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO).toString();
+        if (appViewACLsStr.length() > 0) {
           appViewACLs.put(ApplicationAccessType.VIEW_APP, appViewACLsStr);
         }
+      }
       if (field == ApplicationReportField.USER_AND_ACLS) {
         return new ApplicationReportExt(ApplicationReport.newInstance(
             ConverterUtils.toApplicationId(entity.getEntityId()),

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java

@@ -122,7 +122,11 @@ public class TestApplicationHistoryManagerOnTimelineStore {
     for (int i = 1; i <= SCALE; ++i) {
       TimelineEntities entities = new TimelineEntities();
       ApplicationId appId = ApplicationId.newInstance(0, i);
-      entities.addEntity(createApplicationTimelineEntity(appId));
+      if (i == 2) {
+        entities.addEntity(createApplicationTimelineEntity(appId, true));
+      } else {
+        entities.addEntity(createApplicationTimelineEntity(appId, false));
+      }
       store.put(entities);
       for (int j = 1; j <= SCALE; ++j) {
         entities = new TimelineEntities();
@@ -142,7 +146,8 @@ public class TestApplicationHistoryManagerOnTimelineStore {
 
   @Test
   public void testGetApplicationReport() throws Exception {
-    final ApplicationId appId = ApplicationId.newInstance(0, 1);
+    for (int i = 1; i <= 2; ++i) {
+      final ApplicationId appId = ApplicationId.newInstance(0, i);
       ApplicationReport app;
       if (callerUGI == null) {
         app = historyManager.getApplication(appId);
@@ -164,7 +169,13 @@ public class TestApplicationHistoryManagerOnTimelineStore {
       Assert.assertEquals(Integer.MAX_VALUE + 2L, app.getStartTime());
       Assert.assertEquals(Integer.MAX_VALUE + 3L, app.getFinishTime());
       Assert.assertTrue(Math.abs(app.getProgress() - 1.0F) < 0.0001);
-    if (callerUGI != null && callerUGI.getShortUserName().equals("user3")) {
+      // App 2 doesn't have the ACLs, such that the default ACLs " " will be used.
+      // Nobody except admin and owner has access to the details of the app.
+      if ((i ==  1 && callerUGI != null &&
+          callerUGI.getShortUserName().equals("user3")) ||
+          (i ==  2 && callerUGI != null &&
+          (callerUGI.getShortUserName().equals("user2") ||
+              callerUGI.getShortUserName().equals("user3")))) {
         Assert.assertEquals(ApplicationAttemptId.newInstance(appId, -1),
             app.getCurrentApplicationAttemptId());
         Assert.assertEquals(null, app.getHost());
@@ -187,6 +198,7 @@ public class TestApplicationHistoryManagerOnTimelineStore {
       Assert.assertEquals(YarnApplicationState.FINISHED,
           app.getYarnApplicationState());
     }
+  }
 
   @Test
   public void testGetApplicationAttemptReport() throws Exception {
@@ -396,7 +408,7 @@ public class TestApplicationHistoryManagerOnTimelineStore {
   }
 
   private static TimelineEntity createApplicationTimelineEntity(
-      ApplicationId appId) {
+      ApplicationId appId, boolean emptyACLs) {
     TimelineEntity entity = new TimelineEntity();
     entity.setEntityType(ApplicationMetricsConstants.ENTITY_TYPE);
     entity.setEntityId(appId.toString());
@@ -410,8 +422,12 @@ public class TestApplicationHistoryManagerOnTimelineStore {
     entityInfo.put(ApplicationMetricsConstants.QUEUE_ENTITY_INFO, "test queue");
     entityInfo.put(ApplicationMetricsConstants.SUBMITTED_TIME_ENTITY_INFO,
         Integer.MAX_VALUE + 1L);
+    if (emptyACLs) {
+      entityInfo.put(ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO, "");
+    } else {
       entityInfo.put(ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO,
           "user2");
+    }
     entity.setOtherInfo(entityInfo);
     TimelineEvent tEvent = new TimelineEvent();
     tEvent.setEventType(ApplicationMetricsConstants.CREATED_EVENT_TYPE);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/SystemMetricsPublisher.java

@@ -137,7 +137,7 @@ public class SystemMetricsPublisher extends CompositeService {
       dispatcher.getEventHandler().handle(
           new ApplicationACLsUpdatedEvent(
               app.getApplicationId(),
-              appViewACLs,
+              appViewACLs == null ? "" : appViewACLs,
               updatedTime));
     }
   }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java

@@ -95,11 +95,17 @@ public class TestSystemMetricsPublisher {
 
   @Test(timeout = 10000)
   public void testPublishApplicationMetrics() throws Exception {
-    ApplicationId appId = ApplicationId.newInstance(0, 1);
+    for (int i = 1; i <= 2; ++i) {
+      ApplicationId appId = ApplicationId.newInstance(0, i);
       RMApp app = createRMApp(appId);
       metricsPublisher.appCreated(app, app.getStartTime());
       metricsPublisher.appFinished(app, RMAppState.FINISHED, app.getFinishTime());
+      if (i == 1) {
         metricsPublisher.appACLsUpdated(app, "uers1,user2", 4L);
+      } else {
+        // in case user doesn't specify the ACLs
+        metricsPublisher.appACLsUpdated(app, null, 4L);
+      }
       TimelineEntity entity = null;
       do {
         entity =
@@ -134,9 +140,14 @@ public class TestSystemMetricsPublisher {
       Assert.assertEquals(app.getSubmitTime(),
           entity.getOtherInfo().get(
               ApplicationMetricsConstants.SUBMITTED_TIME_ENTITY_INFO));
+      if (i == 1) {
         Assert.assertEquals("uers1,user2",
             entity.getOtherInfo().get(
                 ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO));
+      } else {
+        Assert.assertEquals("", entity.getOtherInfo().get(
+            ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO));
+      }
       boolean hasCreatedEvent = false;
       boolean hasFinishedEvent = false;
       boolean hasACLsUpdatedEvent = false;
@@ -167,6 +178,7 @@ public class TestSystemMetricsPublisher {
       }
       Assert.assertTrue(hasCreatedEvent && hasFinishedEvent && hasACLsUpdatedEvent);
     }
+  }
 
   @Test(timeout = 10000)
   public void testPublishAppAttemptMetrics() throws Exception {