From 90cbdfaf9df46c7d00fd79fa6260c1f8a9ddeba7 Mon Sep 17 00:00:00 2001
From: Sanford Ryza <sandy@apache.org>
Date: Tue, 18 Feb 2014 17:52:30 +0000
Subject: [PATCH] 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/branches/branch-2@1569445 13f79535-47bb-0310-9956-ffa450edef68
---
 hadoop-yarn-project/CHANGES.txt               |  3 ++
 .../scheduler/fair/FairScheduler.java         | 40 ++++++++++---------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index b706a3df3db..22f970a0229 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -274,6 +274,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
diff --git a/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 b/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
index e057e740fb8..939d82fefb4 100644
--- a/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
+++ b/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.");
+    // 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();
     }
-    if (targetQueue == oldQueue) {
-      return oldQueue.getQueueName();
-    }
-    
-    if (oldQueue.getRunnableAppSchedulables().contains(
-        attempt.getAppSchedulable())) {
-      verifyMoveDoesNotViolateConstraints(attempt, oldQueue, targetQueue);
-    }
-    
-    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) {