YARN-4141. Runtime Application Priority change should not throw excep…

…tion for applications at finishing states. Contributed by Sunil G
apache · Sep 28, 2015 · 9f53a95 · 9f53a95
1 parent 3abbdc9
commit 9f53a95
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 25 deletions.
diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
@@ -904,6 +904,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/...emanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java b/...emanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java
@@ -185,6 +185,12 @@ public class ClientRMService extends AbstractService implements
   private ReservationSystem reservationSystem;
   private ReservationInputValidator rValidator;
 
+  private static final EnumSet<RMAppState> COMPLETED_APP_STATES = EnumSet.of(
+      RMAppState.FINISHED, RMAppState.FINISHING, RMAppState.FAILED,
+      RMAppState.KILLED, RMAppState.FINAL_SAVING, RMAppState.KILLING);
+  private static final EnumSet<RMAppState> ACTIVE_APP_STATES = EnumSet.of(
+      RMAppState.ACCEPTED, RMAppState.RUNNING);
+
   public ClientRMService(RMContext rmContext, YarnScheduler scheduler,
       RMAppManager rmAppManager, ApplicationACLsManager applicationACLsManager,
       QueueACLsManager queueACLsManager,
@@ -1334,7 +1340,8 @@ public UpdateApplicationPriorityResponse updateApplicationPriority(
           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(),
@@ -1349,12 +1356,20 @@ public UpdateApplicationPriorityResponse updateApplicationPriority(
           + 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",
@@ -1374,9 +1389,6 @@ public UpdateApplicationPriorityResponse updateApplicationPriority(
 
     RMAuditLogger.logSuccess(callerUGI.getShortUserName(),
         AuditConstants.UPDATE_APP_PRIORITY, "ClientRMService", applicationId);
-    UpdateApplicationPriorityResponse response =
-        recordFactory
-            .newRecordInstance(UpdateApplicationPriorityResponse.class);
     return response;
   }
 

diff --git a/...ager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java b/...ager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java
@@ -1335,7 +1335,7 @@ protected ClientRMService createClientRMService() {
   @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 void testUpdateApplicationPriorityRequest() throws Exception {
     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