From 600022ae697cf2a46e7cc75236be07074ab17b5d Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Mon, 28 Sep 2015 22:55:20 +0000 Subject: [PATCH] YARN-4141. Runtime Application Priority change should not throw exception for applications at finishing states. Contributed by Sunil G (cherry picked from commit 9f53a95ff624f66a774fe3defeea4a3454f4c4af) --- hadoop-yarn-project/CHANGES.txt | 3 ++ .../resourcemanager/ClientRMService.java | 30 +++++++++++----- .../resourcemanager/TestClientRMService.java | 36 ++++++++++--------- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index c5586932e12..11dc43c3131 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -852,6 +852,9 @@ Release 2.8.0 - UNRELEASED YARN-4204. ConcurrentModificationException in FairSchedulerQueueInfo. (adhoot) + YARN-4141. Runtime Application Priority change should not throw exception + for applications at finishing states (Sunil G via jlowe) + Release 2.7.2 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java index 43d3de7c7c5..0148234c877 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java @@ -184,6 +184,12 @@ public class ClientRMService extends AbstractService implements private ReservationSystem reservationSystem; private ReservationInputValidator rValidator; + private static final EnumSet COMPLETED_APP_STATES = EnumSet.of( + RMAppState.FINISHED, RMAppState.FINISHING, RMAppState.FAILED, + RMAppState.KILLED, RMAppState.FINAL_SAVING, RMAppState.KILLING); + private static final EnumSet ACTIVE_APP_STATES = EnumSet.of( + RMAppState.ACCEPTED, RMAppState.RUNNING); + public ClientRMService(RMContext rmContext, YarnScheduler scheduler, RMAppManager rmAppManager, ApplicationACLsManager applicationACLsManager, QueueACLsManager queueACLsManager, @@ -1333,7 +1339,8 @@ public class ClientRMService extends AbstractService implements AuditConstants.UPDATE_APP_PRIORITY, "UNKNOWN", "ClientRMService", "Trying to update priority of an absent application", applicationId); throw new ApplicationNotFoundException( - "Trying to update priority o an absent application " + applicationId); + "Trying to update priority of an absent application " + + applicationId); } if (!checkAccess(callerUGI, application.getUser(), @@ -1348,12 +1355,20 @@ public class ClientRMService extends AbstractService implements + ApplicationAccessType.MODIFY_APP.name() + " on " + applicationId)); } + UpdateApplicationPriorityResponse response = recordFactory + .newRecordInstance(UpdateApplicationPriorityResponse.class); // Update priority only when app is tracked by the scheduler - if (!EnumSet.of(RMAppState.ACCEPTED, RMAppState.RUNNING).contains( - application.getState())) { - String msg = - "Application in " + application.getState() - + " state cannot be update priority."; + if (!ACTIVE_APP_STATES.contains(application.getState())) { + if (COMPLETED_APP_STATES.contains(application.getState())) { + // If Application is in any of the final states, change priority + // can be skipped rather throwing exception. + RMAuditLogger.logSuccess(callerUGI.getShortUserName(), + AuditConstants.UPDATE_APP_PRIORITY, "ClientRMService", + applicationId); + return response; + } + String msg = "Application in " + application.getState() + + " state cannot update priority."; RMAuditLogger .logFailure(callerUGI.getShortUserName(), AuditConstants.UPDATE_APP_PRIORITY, "UNKNOWN", "ClientRMService", @@ -1373,9 +1388,6 @@ public class ClientRMService extends AbstractService implements RMAuditLogger.logSuccess(callerUGI.getShortUserName(), AuditConstants.UPDATE_APP_PRIORITY, "ClientRMService", applicationId); - UpdateApplicationPriorityResponse response = - recordFactory - .newRecordInstance(UpdateApplicationPriorityResponse.class); return response; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java index 39964da9b8d..49b5b550c2e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java @@ -1335,7 +1335,7 @@ public class TestClientRMService { @Test(timeout = 120000) public void testUpdateApplicationPriorityRequest() throws Exception { int maxPriority = 10; - int appPriorty = 5; + int appPriority = 5; YarnConfiguration conf = new YarnConfiguration(); conf.setInt(YarnConfiguration.MAX_CLUSTER_LEVEL_APPLICATION_PRIORITY, maxPriority); @@ -1344,43 +1344,47 @@ public class TestClientRMService { rm.start(); // Start app1 with appPriority 5 - RMApp app1 = rm.submitApp(1024, Priority.newInstance(appPriorty)); + RMApp app1 = rm.submitApp(1024, Priority.newInstance(appPriority)); Assert.assertEquals("Incorrect priority has been set to application", - appPriorty, app1.getApplicationSubmissionContext().getPriority() + appPriority, app1.getApplicationSubmissionContext().getPriority() .getPriority()); - appPriorty = 9; + appPriority = 9; ClientRMService rmService = rm.getClientRMService(); UpdateApplicationPriorityRequest updateRequest = UpdateApplicationPriorityRequest.newInstance(app1.getApplicationId(), - Priority.newInstance(appPriorty)); + Priority.newInstance(appPriority)); rmService.updateApplicationPriority(updateRequest); Assert.assertEquals("Incorrect priority has been set to application", - appPriorty, app1.getApplicationSubmissionContext().getPriority() + appPriority, app1.getApplicationSubmissionContext().getPriority() .getPriority()); rm.killApp(app1.getApplicationId()); rm.waitForState(app1.getApplicationId(), RMAppState.KILLED); + appPriority = 8; + UpdateApplicationPriorityRequest updateRequestNew = + UpdateApplicationPriorityRequest.newInstance(app1.getApplicationId(), + Priority.newInstance(appPriority)); // Update priority request for application in KILLED state - try { - rmService.updateApplicationPriority(updateRequest); - Assert.fail("Can not update priority for an application in KILLED state"); - } catch (YarnException e) { - String msg = - "Application in " + app1.getState() - + " state cannot be update priority."; - Assert.assertTrue("", msg.contains(e.getMessage())); - } + rmService.updateApplicationPriority(updateRequestNew); + + // Hence new priority should not be updated + Assert.assertNotEquals("Priority should not be updated as app is in KILLED state", + appPriority, app1.getApplicationSubmissionContext().getPriority() + .getPriority()); + Assert.assertEquals("Priority should be same as old one before update", + 9, app1.getApplicationSubmissionContext().getPriority() + .getPriority()); // Update priority request for invalid application id. ApplicationId invalidAppId = ApplicationId.newInstance(123456789L, 3); updateRequest = UpdateApplicationPriorityRequest.newInstance(invalidAppId, - Priority.newInstance(appPriorty)); + Priority.newInstance(appPriority)); try { rmService.updateApplicationPriority(updateRequest); Assert