YARN-1618. Fix invalid RMApp transition from NEW to FINAL_SAVING (kasha)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1562532 13f79535-47bb-0310-9956-ffa450edef68

hadoop-yarn-project/CHANGES.txt

@@ -453,6 +453,8 @@ Release 2.4.0 - UNRELEASED
 
     YARN-1629. IndexOutOfBoundsException in MaxRunningAppsEnforcer (Sandy Ryza)
 
+    YARN-1618. Fix invalid RMApp transition from NEW to FINAL_SAVING (kasha)
+
 Release 2.3.0 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/RMAppEventType.java

@@ -24,8 +24,10 @@ public enum RMAppEventType {
   RECOVER,
   KILL,
 
-  // Source: Scheduler
+  // Source: Scheduler and RMAppManager
   APP_REJECTED,
+
+  // Source: Scheduler
   APP_ACCEPTED,
 
   // Source: RMAppAttempt

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/RMAppImpl.java

@@ -144,13 +144,10 @@ public class RMAppImpl implements RMApp, Recoverable {
             RMAppState.ACCEPTED, RMAppState.FINISHED, RMAppState.FAILED,
             RMAppState.KILLED, RMAppState.FINAL_SAVING),
         RMAppEventType.RECOVER, new RMAppRecoveredTransition())
-    .addTransition(RMAppState.NEW, RMAppState.FINAL_SAVING, RMAppEventType.KILL,
+    .addTransition(RMAppState.NEW, RMAppState.KILLED, RMAppEventType.KILL,
-        new FinalSavingTransition(
+        new AppKilledTransition())
-          new AppKilledTransition(), RMAppState.KILLED))
+    .addTransition(RMAppState.NEW, RMAppState.FAILED,
-    .addTransition(RMAppState.NEW, RMAppState.FINAL_SAVING,
+        RMAppEventType.APP_REJECTED, new AppRejectedTransition())
-        RMAppEventType.APP_REJECTED,
-        new FinalSavingTransition(
-          new AppRejectedTransition(), RMAppState.FAILED))
 
     // Transitions from NEW_SAVING state
     .addTransition(RMAppState.NEW_SAVING, RMAppState.NEW_SAVING,

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/TestRMAppTransitions.java

@@ -295,6 +295,10 @@ public class TestRMAppTransitions {
     verify(store, times(1)).updateApplicationState(any(ApplicationState.class));
   }
 
+  private void assertAppFinalStateNotSaved(RMApp application){
+    verify(store, times(0)).updateApplicationState(any(ApplicationState.class));
+  }
+
   private void assertKilled(RMApp application) {
     assertTimesAtFinish(application);
     assertAppState(RMAppState.KILLED, application);
@@ -321,7 +325,6 @@ public class TestRMAppTransitions {
     StringBuilder diag = application.getDiagnostics();
     Assert.assertTrue("application diagnostics is not correct",
         diag.toString().matches(regex));
-    assertAppFinalStateSaved(application);
   }
 
   private void sendAppUpdateSavedEvent(RMApp application) {
@@ -477,6 +480,7 @@ public class TestRMAppTransitions {
     sendAppUpdateSavedEvent(application);
     assertFailed(application,
         ".*Unmanaged application.*Failing the application.*");
+    assertAppFinalStateSaved(application);
   }
   
   @Test
@@ -506,6 +510,7 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertKilled(application);
+    assertAppFinalStateNotSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }
 
@@ -522,6 +527,7 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertFailed(application, rejectedText);
+    assertAppFinalStateNotSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }
 
@@ -553,6 +559,7 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertFailed(application, rejectedText);
+    assertAppFinalStateSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }
 
@@ -569,6 +576,7 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertFailed(application, rejectedText);
+    assertAppFinalStateSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }
 
@@ -618,6 +626,7 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertFailed(application, ".*" + message + ".*Failing the application.*");
+    assertAppFinalStateSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }
 
@@ -703,12 +712,14 @@ public class TestRMAppTransitions {
     rmDispatcher.await();
     sendAppUpdateSavedEvent(application);
     assertFailed(application, ".*Failing the application.*");
+    assertAppFinalStateSaved(application);
 
     // FAILED => FAILED event RMAppEventType.KILL
     event = new RMAppEvent(application.getApplicationId(), RMAppEventType.KILL);
     application.handle(event);
     rmDispatcher.await();
     assertFailed(application, ".*Failing the application.*");
+    assertAppFinalStateSaved(application);
     verify(writer).applicationFinished(any(RMApp.class));
   }