YARN-1721. When moving app between queues in Fair Scheduler, grab lock on FSSchedulerApp (Sandy Ryza)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1569443 13f79535-47bb-0310-9956-ffa450edef68

hadoop-yarn-project/CHANGES.txt

@@ -289,6 +289,9 @@ Release 2.4.0 - UNRELEASED
     instead rely on the http policy framework. And also fix some bugs related
     to https handling in YARN web-apps. (Haohui Mai via vinodkv)
 
+    YARN-1721. When moving app between queues in Fair Scheduler, grab lock on
+    FSSchedulerApp (Sandy Ryza)
+
 Release 2.3.1 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java

@@ -1366,24 +1366,26 @@ public class FairScheduler extends AbstractYarnScheduler {
       throw new YarnException("App to be moved " + appId + " not found.");
     }
     FSSchedulerApp attempt = (FSSchedulerApp) app.getCurrentAppAttempt();
-    
-    FSLeafQueue oldQueue = (FSLeafQueue) app.getQueue();
-    FSLeafQueue targetQueue = queueMgr.getLeafQueue(queueName, false);
-    if (targetQueue == null) {
-      throw new YarnException("Target queue " + queueName
-          + " not found or is not a leaf queue.");
-    }
-    if (targetQueue == oldQueue) {
-      return oldQueue.getQueueName();
-    }
-    
-    if (oldQueue.getRunnableAppSchedulables().contains(
-        attempt.getAppSchedulable())) {
-      verifyMoveDoesNotViolateConstraints(attempt, oldQueue, targetQueue);
+    // To serialize with FairScheduler#allocate, synchronize on app attempt
+    synchronized (attempt) {
+      FSLeafQueue oldQueue = (FSLeafQueue) app.getQueue();
+      FSLeafQueue targetQueue = queueMgr.getLeafQueue(queueName, false);
+      if (targetQueue == null) {
+        throw new YarnException("Target queue " + queueName
+            + " not found or is not a leaf queue.");
+      }
+      if (targetQueue == oldQueue) {
+        return oldQueue.getQueueName();
+      }
+      
+      if (oldQueue.getRunnableAppSchedulables().contains(
+          attempt.getAppSchedulable())) {
+        verifyMoveDoesNotViolateConstraints(attempt, oldQueue, targetQueue);
+      }
+      
+      executeMove(app, attempt, oldQueue, targetQueue);
+      return targetQueue.getQueueName();
     }
-    
-    executeMove(app, attempt, oldQueue, targetQueue);
-    return targetQueue.getQueueName();
   }
   
   private void verifyMoveDoesNotViolateConstraints(FSSchedulerApp app,
@@ -1420,8 +1422,8 @@ public class FairScheduler extends AbstractYarnScheduler {
   }
   
   /**
-   * Helper for moveApplication, which is synchronized, so all operations will
-   * be atomic.
+   * Helper for moveApplication, which has appropriate synchronization, so all
+   * operations will be atomic.
    */
   private void executeMove(SchedulerApplication app, FSSchedulerApp attempt,
       FSLeafQueue oldQueue, FSLeafQueue newQueue) {