From bbff96be48119774688981d04baf444639135977 Mon Sep 17 00:00:00 2001 From: Jian He Date: Tue, 30 Sep 2014 16:39:25 -0700 Subject: [PATCH] YARN-2602. Fixed possible NPE in ApplicationHistoryManagerOnTimelineStore. Contributed by Zhijie Shen --- hadoop-yarn-project/CHANGES.txt | 3 + ...licationHistoryManagerOnTimelineStore.java | 4 +- ...licationHistoryManagerOnTimelineStore.java | 110 +++++++------ .../metrics/SystemMetricsPublisher.java | 2 +- .../metrics/TestSystemMetricsPublisher.java | 150 ++++++++++-------- 5 files changed, 151 insertions(+), 118 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 95fba2395c..bfaaa90d08 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/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 diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java index f00ec9cac9..5381bd6cb2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java @@ -227,7 +227,9 @@ private static ApplicationReportExt convertToApplicationReport( if (entityInfo.containsKey(ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO)) { String appViewACLsStr = entityInfo.get( ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO).toString(); - appViewACLs.put(ApplicationAccessType.VIEW_APP, appViewACLsStr); + if (appViewACLsStr.length() > 0) { + appViewACLs.put(ApplicationAccessType.VIEW_APP, appViewACLsStr); + } } if (field == ApplicationReportField.USER_AND_ACLS) { return new ApplicationReportExt(ApplicationReport.newInstance( diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java index e6bfcd9399..49386c5b3c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java +++ b/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 @@ private static void prepareTimelineStore(TimelineStore store) 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,50 +146,58 @@ private static void prepareTimelineStore(TimelineStore store) @Test public void testGetApplicationReport() throws Exception { - final ApplicationId appId = ApplicationId.newInstance(0, 1); - ApplicationReport app; - if (callerUGI == null) { - app = historyManager.getApplication(appId); - } else { - app = - callerUGI.doAs(new PrivilegedExceptionAction () { - @Override - public ApplicationReport run() throws Exception { - return historyManager.getApplication(appId); - } - }); + for (int i = 1; i <= 2; ++i) { + final ApplicationId appId = ApplicationId.newInstance(0, i); + ApplicationReport app; + if (callerUGI == null) { + app = historyManager.getApplication(appId); + } else { + app = + callerUGI.doAs(new PrivilegedExceptionAction () { + @Override + public ApplicationReport run() throws Exception { + return historyManager.getApplication(appId); + } + }); + } + Assert.assertNotNull(app); + Assert.assertEquals(appId, app.getApplicationId()); + Assert.assertEquals("test app", app.getName()); + Assert.assertEquals("test app type", app.getApplicationType()); + Assert.assertEquals("user1", app.getUser()); + Assert.assertEquals("test queue", app.getQueue()); + 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); + // 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()); + Assert.assertEquals(-1, app.getRpcPort()); + Assert.assertEquals(null, app.getTrackingUrl()); + Assert.assertEquals(null, app.getOriginalTrackingUrl()); + Assert.assertEquals(null, app.getDiagnostics()); + } else { + Assert.assertEquals(ApplicationAttemptId.newInstance(appId, 1), + app.getCurrentApplicationAttemptId()); + Assert.assertEquals("test host", app.getHost()); + Assert.assertEquals(-100, app.getRpcPort()); + Assert.assertEquals("test tracking url", app.getTrackingUrl()); + Assert.assertEquals("test original tracking url", + app.getOriginalTrackingUrl()); + Assert.assertEquals("test diagnostics info", app.getDiagnostics()); + } + Assert.assertEquals(FinalApplicationStatus.UNDEFINED, + app.getFinalApplicationStatus()); + Assert.assertEquals(YarnApplicationState.FINISHED, + app.getYarnApplicationState()); } - Assert.assertNotNull(app); - Assert.assertEquals(appId, app.getApplicationId()); - Assert.assertEquals("test app", app.getName()); - Assert.assertEquals("test app type", app.getApplicationType()); - Assert.assertEquals("user1", app.getUser()); - Assert.assertEquals("test queue", app.getQueue()); - 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")) { - Assert.assertEquals(ApplicationAttemptId.newInstance(appId, -1), - app.getCurrentApplicationAttemptId()); - Assert.assertEquals(null, app.getHost()); - Assert.assertEquals(-1, app.getRpcPort()); - Assert.assertEquals(null, app.getTrackingUrl()); - Assert.assertEquals(null, app.getOriginalTrackingUrl()); - Assert.assertEquals(null, app.getDiagnostics()); - } else { - Assert.assertEquals(ApplicationAttemptId.newInstance(appId, 1), - app.getCurrentApplicationAttemptId()); - Assert.assertEquals("test host", app.getHost()); - Assert.assertEquals(-100, app.getRpcPort()); - Assert.assertEquals("test tracking url", app.getTrackingUrl()); - Assert.assertEquals("test original tracking url", - app.getOriginalTrackingUrl()); - Assert.assertEquals("test diagnostics info", app.getDiagnostics()); - } - Assert.assertEquals(FinalApplicationStatus.UNDEFINED, - app.getFinalApplicationStatus()); - Assert.assertEquals(YarnApplicationState.FINISHED, - app.getYarnApplicationState()); } @Test @@ -396,7 +408,7 @@ public ContainerReport run() throws Exception { } 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 @@ private static TimelineEntity createApplicationTimelineEntity( entityInfo.put(ApplicationMetricsConstants.QUEUE_ENTITY_INFO, "test queue"); entityInfo.put(ApplicationMetricsConstants.SUBMITTED_TIME_ENTITY_INFO, Integer.MAX_VALUE + 1L); - entityInfo.put(ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO, - "user2"); + 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); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/SystemMetricsPublisher.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/SystemMetricsPublisher.java index 5da006c009..e2ecf9a83d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/SystemMetricsPublisher.java +++ b/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 void appACLsUpdated(RMApp app, String appViewACLs, dispatcher.getEventHandler().handle( new ApplicationACLsUpdatedEvent( app.getApplicationId(), - appViewACLs, + appViewACLs == null ? "" : appViewACLs, updatedTime)); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java index 63343e9521..52faf12f50 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java +++ b/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,77 +95,89 @@ public static void tearDown() throws Exception { @Test(timeout = 10000) public void testPublishApplicationMetrics() throws Exception { - ApplicationId appId = ApplicationId.newInstance(0, 1); - RMApp app = createRMApp(appId); - metricsPublisher.appCreated(app, app.getStartTime()); - metricsPublisher.appFinished(app, RMAppState.FINISHED, app.getFinishTime()); - metricsPublisher.appACLsUpdated(app, "uers1,user2", 4L); - TimelineEntity entity = null; - do { - entity = - store.getEntity(appId.toString(), - ApplicationMetricsConstants.ENTITY_TYPE, - EnumSet.allOf(Field.class)); - // ensure three events are both published before leaving the loop - } while (entity == null || entity.getEvents().size() < 3); - // verify all the fields - Assert.assertEquals(ApplicationMetricsConstants.ENTITY_TYPE, - entity.getEntityType()); - Assert - .assertEquals(app.getApplicationId().toString(), entity.getEntityId()); - Assert - .assertEquals( - app.getName(), - entity.getOtherInfo().get( - ApplicationMetricsConstants.NAME_ENTITY_INFO)); - Assert.assertEquals(app.getQueue(), - entity.getOtherInfo() - .get(ApplicationMetricsConstants.QUEUE_ENTITY_INFO)); - Assert - .assertEquals( - app.getUser(), - entity.getOtherInfo().get( - ApplicationMetricsConstants.USER_ENTITY_INFO)); - Assert - .assertEquals( - app.getApplicationType(), - entity.getOtherInfo().get( - ApplicationMetricsConstants.TYPE_ENTITY_INFO)); - Assert.assertEquals(app.getSubmitTime(), - entity.getOtherInfo().get( - ApplicationMetricsConstants.SUBMITTED_TIME_ENTITY_INFO)); - Assert.assertEquals("uers1,user2", - entity.getOtherInfo().get( - ApplicationMetricsConstants.APP_VIEW_ACLS_ENTITY_INFO)); - boolean hasCreatedEvent = false; - boolean hasFinishedEvent = false; - boolean hasACLsUpdatedEvent = false; - for (TimelineEvent event : entity.getEvents()) { - if (event.getEventType().equals( - ApplicationMetricsConstants.CREATED_EVENT_TYPE)) { - hasCreatedEvent = true; - Assert.assertEquals(app.getStartTime(), event.getTimestamp()); - } else if (event.getEventType().equals( - ApplicationMetricsConstants.FINISHED_EVENT_TYPE)) { - hasFinishedEvent = true; - Assert.assertEquals(app.getFinishTime(), event.getTimestamp()); - Assert.assertEquals( - app.getDiagnostics().toString(), - event.getEventInfo().get( - ApplicationMetricsConstants.DIAGNOSTICS_INFO_EVENT_INFO)); - Assert.assertEquals( - app.getFinalApplicationStatus().toString(), - event.getEventInfo().get( - ApplicationMetricsConstants.FINAL_STATUS_EVENT_INFO)); - Assert.assertEquals(YarnApplicationState.FINISHED.toString(), event - .getEventInfo().get(ApplicationMetricsConstants.STATE_EVENT_INFO)); - } else if (event.getEventType().equals( - ApplicationMetricsConstants.ACLS_UPDATED_EVENT_TYPE)) { - hasACLsUpdatedEvent = true; - Assert.assertEquals(4L, event.getTimestamp()); + 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 = + store.getEntity(appId.toString(), + ApplicationMetricsConstants.ENTITY_TYPE, + EnumSet.allOf(Field.class)); + // ensure three events are both published before leaving the loop + } while (entity == null || entity.getEvents().size() < 3); + // verify all the fields + Assert.assertEquals(ApplicationMetricsConstants.ENTITY_TYPE, + entity.getEntityType()); + Assert + .assertEquals(app.getApplicationId().toString(), entity.getEntityId()); + Assert + .assertEquals( + app.getName(), + entity.getOtherInfo().get( + ApplicationMetricsConstants.NAME_ENTITY_INFO)); + Assert.assertEquals(app.getQueue(), + entity.getOtherInfo() + .get(ApplicationMetricsConstants.QUEUE_ENTITY_INFO)); + Assert + .assertEquals( + app.getUser(), + entity.getOtherInfo().get( + ApplicationMetricsConstants.USER_ENTITY_INFO)); + Assert + .assertEquals( + app.getApplicationType(), + entity.getOtherInfo().get( + ApplicationMetricsConstants.TYPE_ENTITY_INFO)); + 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; + for (TimelineEvent event : entity.getEvents()) { + if (event.getEventType().equals( + ApplicationMetricsConstants.CREATED_EVENT_TYPE)) { + hasCreatedEvent = true; + Assert.assertEquals(app.getStartTime(), event.getTimestamp()); + } else if (event.getEventType().equals( + ApplicationMetricsConstants.FINISHED_EVENT_TYPE)) { + hasFinishedEvent = true; + Assert.assertEquals(app.getFinishTime(), event.getTimestamp()); + Assert.assertEquals( + app.getDiagnostics().toString(), + event.getEventInfo().get( + ApplicationMetricsConstants.DIAGNOSTICS_INFO_EVENT_INFO)); + Assert.assertEquals( + app.getFinalApplicationStatus().toString(), + event.getEventInfo().get( + ApplicationMetricsConstants.FINAL_STATUS_EVENT_INFO)); + Assert.assertEquals(YarnApplicationState.FINISHED.toString(), event + .getEventInfo().get(ApplicationMetricsConstants.STATE_EVENT_INFO)); + } else if (event.getEventType().equals( + ApplicationMetricsConstants.ACLS_UPDATED_EVENT_TYPE)) { + hasACLsUpdatedEvent = true; + Assert.assertEquals(4L, event.getTimestamp()); + } + } + Assert.assertTrue(hasCreatedEvent && hasFinishedEvent && hasACLsUpdatedEvent); } - Assert.assertTrue(hasCreatedEvent && hasFinishedEvent && hasACLsUpdatedEvent); } @Test(timeout = 10000)