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/FSLeafQueue.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/FSLeafQueue.java index a0388873278..3deddee5d6e 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/FSLeafQueue.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/FSLeafQueue.java @@ -457,6 +457,20 @@ public int getNumAssignedApps() { } } + @Override + public boolean isEmpty() { + readLock.lock(); + try { + if (runnableApps.size() > 0 || nonRunnableApps.size() > 0 || + assignedApps.size() > 0) { + return false; + } + } finally { + readLock.unlock(); + } + return true; + } + /** * TODO: Based on how frequently this is called, we might want to club * counting pending and active apps in the same method. 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/FSParentQueue.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/FSParentQueue.java index e9f4af67450..9a52d37efeb 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/FSParentQueue.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/FSParentQueue.java @@ -263,6 +263,21 @@ public int getNumRunnableApps() { } } + @Override + public boolean isEmpty() { + readLock.lock(); + try { + for (FSQueue queue: childQueues) { + if (!queue.isEmpty()) { + return false; + } + } + } finally { + readLock.unlock(); + } + return true; + } + @Override public void collectSchedulerApplications( Collection apps) { 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/FSQueue.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/FSQueue.java index 6217f550f8b..b64f783af4d 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/FSQueue.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/FSQueue.java @@ -601,4 +601,6 @@ public boolean isDynamic() { public void setDynamic(boolean dynamic) { this.isDynamic = dynamic; } + + public abstract boolean isEmpty(); } 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/QueueManager.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/QueueManager.java index 2ca32c3018b..a9c06780069 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/QueueManager.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/QueueManager.java @@ -498,7 +498,7 @@ public void removeEmptyDynamicQueues() { } while (!parentQueuesToCheck.isEmpty()) { FSParentQueue queue = parentQueuesToCheck.iterator().next(); - if (queue.getChildQueues().isEmpty()) { + if (queue.isEmpty()) { removeQueue(queue); if (queue.getParent().isDynamic()) { parentQueuesToCheck.add(queue.getParent()); @@ -528,7 +528,7 @@ public void removePendingIncompatibleQueues() { * @return true if removed, false otherwise */ private boolean removeQueueIfEmpty(FSQueue queue) { - if (isEmpty(queue)) { + if (queue.isEmpty()) { removeQueue(queue); return true; } @@ -553,26 +553,6 @@ private void removeQueue(FSQueue queue) { } } - /** - * Returns true if there are no applications, running or not, in the given - * queue or any of its descendents. - */ - protected boolean isEmpty(FSQueue queue) { - if (queue instanceof FSLeafQueue) { - FSLeafQueue leafQueue = (FSLeafQueue)queue; - return queue.getNumRunnableApps() == 0 && - leafQueue.getNumNonRunnableApps() == 0 && - leafQueue.getNumAssignedApps() == 0; - } else { - for (FSQueue child : queue.getChildQueues()) { - if (!isEmpty(child)) { - return false; - } - } - return true; - } - } - /** * Gets a queue by name. */ diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFSParentQueue.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFSParentQueue.java index 671b94be9ed..c6ff5aaa33d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFSParentQueue.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFSParentQueue.java @@ -23,9 +23,6 @@ import org.junit.Before; import org.junit.Test; -import java.util.HashSet; -import java.util.Set; - import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -34,7 +31,6 @@ private FairSchedulerConfiguration conf; private QueueManager queueManager; - private Set notEmptyQueues; @Before public void setUp() throws Exception { @@ -47,13 +43,7 @@ public void setUp() throws Exception { new DefaultResourceCalculator()); SystemClock clock = SystemClock.getInstance(); when(scheduler.getClock()).thenReturn(clock); - notEmptyQueues = new HashSet(); - queueManager = new QueueManager(scheduler) { - @Override - public boolean isEmpty(FSQueue queue) { - return !notEmptyQueues.contains(queue); - } - }; + queueManager = new QueueManager(scheduler); FSQueueMetrics.forQueue("root", null, true, conf); queueManager.initialize(conf); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java index 3674ffb40b6..fdb8566a02e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java @@ -21,7 +21,6 @@ import static org.mockito.Mockito.*; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import org.apache.hadoop.yarn.api.records.ApplicationAttemptId; @@ -41,7 +40,6 @@ public class TestQueueManager { private FairSchedulerConfiguration conf; private QueueManager queueManager; - private Set notEmptyQueues; private FairScheduler scheduler; @Before @@ -64,13 +62,7 @@ public void setUp() throws Exception { SystemClock clock = SystemClock.getInstance(); when(scheduler.getClock()).thenReturn(clock); - notEmptyQueues = new HashSet<>(); - queueManager = new QueueManager(scheduler) { - @Override - public boolean isEmpty(FSQueue queue) { - return !notEmptyQueues.contains(queue); - } - }; + queueManager = new QueueManager(scheduler); FSQueueMetrics.forQueue("root", null, true, conf); @@ -95,16 +87,19 @@ public void testReloadTurnsLeafQueueIntoParent() throws Exception { // When apps exist in leaf queue, we shouldn't be able to create // children under it, but things should work otherwise. - notEmptyQueues.add(queueManager.getLeafQueue("queue1", false)); + FSLeafQueue q1 = queueManager.getLeafQueue("queue1", false); + ApplicationId appId = ApplicationId.newInstance(0,0); + q1.addAssignedApp(appId); updateConfiguredLeafQueues(queueManager, "queue1.queue2"); assertNull(queueManager.getLeafQueue("queue1.queue2", false)); assertNotNull(queueManager.getLeafQueue("queue1", false)); // When apps exist in leaf queues under a parent queue, shouldn't be // able to turn it into a leaf queue, but things should work otherwise. - notEmptyQueues.clear(); + q1.removeAssignedApp(appId); updateConfiguredLeafQueues(queueManager, "queue1.queue2"); - notEmptyQueues.add(queueManager.getQueue("root.queue1")); + FSLeafQueue q2 = queueManager.getLeafQueue("queue1.queue2", false); + q2.addAssignedApp(appId); updateConfiguredLeafQueues(queueManager, "queue1"); assertNotNull(queueManager.getLeafQueue("queue1.queue2", false)); assertNull(queueManager.getLeafQueue("queue1", false)); @@ -126,7 +121,9 @@ public void testReloadTurnsLeafToParentWithNoLeaf() { // Lets say later on admin makes queue1 a parent queue by // specifying "type=parent" in the alloc xml and lets say apps running in // queue1 - notEmptyQueues.add(queueManager.getLeafQueue("root.queue1", false)); + FSLeafQueue q1 = queueManager.getLeafQueue("queue1", false); + ApplicationId appId = ApplicationId.newInstance(0,0); + q1.addAssignedApp(appId); allocConf = new AllocationConfiguration(conf); allocConf.configuredQueues.get(FSQueueType.PARENT) .add("root.queue1"); @@ -137,7 +134,7 @@ public void testReloadTurnsLeafToParentWithNoLeaf() { assertNull(queueManager.getParentQueue("root.queue1", false)); // Now lets assume apps completed and there are no apps in queue1 - notEmptyQueues.clear(); + q1.removeAssignedApp(appId); // We should see queue1 transform from leaf queue to parent queue. queueManager.updateAllocationConfiguration(allocConf); assertNull(queueManager.getLeafQueue("root.queue1", false)); @@ -319,7 +316,7 @@ public void testRemovalOfDynamicLeafQueue() { queueManager.updateAllocationConfiguration(allocConf); - FSQueue q1 = queueManager.getLeafQueue("root.test.childB.dynamic1", true); + FSLeafQueue q1 = queueManager.getLeafQueue("root.test.childB.dynamic1", true); assertNotNull("Queue root.test.childB.dynamic1 was not created", q1); assertEquals("createQueue() returned wrong queue", @@ -328,7 +325,8 @@ public void testRemovalOfDynamicLeafQueue() { q1.isDynamic()); // an application is submitted to root.test.childB.dynamic1 - notEmptyQueues.add(q1); + ApplicationId appId = ApplicationId.newInstance(0,0); + q1.addAssignedApp(appId); // root.test.childB.dynamic1 is not empty and should not be removed queueManager.removePendingIncompatibleQueues(); @@ -338,7 +336,7 @@ public void testRemovalOfDynamicLeafQueue() { // the application finishes, the next removeEmptyDynamicQueues() should // clean root.test.childB.dynamic1 up, but keep its static parent - notEmptyQueues.remove(q1); + q1.removeAssignedApp(appId); queueManager.removePendingIncompatibleQueues(); queueManager.removeEmptyDynamicQueues(); @@ -388,7 +386,8 @@ public void testNonEmptyDynamicQueueBecomingStaticQueue() { assertTrue("root.leaf1 is not a dynamic queue", q1.isDynamic()); // pretend that we submitted an app to the queue - notEmptyQueues.add(q1); + ApplicationId appId = ApplicationId.newInstance(0,0); + q1.addAssignedApp(appId); // non-empty queues should not be deleted queueManager.removePendingIncompatibleQueues(); @@ -406,7 +405,7 @@ public void testNonEmptyDynamicQueueBecomingStaticQueue() { // application finished now and the queue is empty, but since leaf1 is a // static queue at this point, hence not affected by // removeEmptyDynamicQueues() - notEmptyQueues.clear(); + q1.removeAssignedApp(appId); queueManager.removePendingIncompatibleQueues(); queueManager.removeEmptyDynamicQueues(); q1 = queueManager.getLeafQueue("root.leaf1", false); @@ -427,7 +426,8 @@ public void testNonEmptyStaticQueueBecomingDynamicQueue() { assertFalse("root.test.childA is not a static queue", q1.isDynamic()); // we submitted an app to the queue - notEmptyQueues.add(q1); + ApplicationId appId = ApplicationId.newInstance(0,0); + q1.addAssignedApp(appId); // the next removeEmptyDynamicQueues() call should not modify // root.test.childA @@ -451,7 +451,7 @@ public void testNonEmptyStaticQueueBecomingDynamicQueue() { // application finished - the queue does not have runnable app // the next removeEmptyDynamicQueues() call should remove the queues - notEmptyQueues.remove(q1); + q1.removeAssignedApp(appId); queueManager.removePendingIncompatibleQueues(); queueManager.removeEmptyDynamicQueues(); @@ -549,20 +549,20 @@ public void testApplicationAssignmentPreventsRemovalOfDynamicQueue() FSLeafQueue q = queueManager.getLeafQueue("root.leaf1", true); assertNotNull("root.leaf1 does not exist", q); - assertTrue("root.leaf1 is not empty", queueManager.isEmpty(q)); + assertTrue("root.leaf1 is not empty", q.isEmpty()); // assigning an application (without an appAttempt so far) to the queue // removeEmptyDynamicQueues() should not remove the queue ApplicationId applicationId = ApplicationId.newInstance(1L, 0); q.addAssignedApp(applicationId); q = queueManager.getLeafQueue("root.leaf1", false); - assertFalse("root.leaf1 is empty", queueManager.isEmpty(q)); + assertFalse("root.leaf1 is empty", q.isEmpty()); queueManager.removePendingIncompatibleQueues(); queueManager.removeEmptyDynamicQueues(); q = queueManager.getLeafQueue("root.leaf1", false); assertNotNull("root.leaf1 has been removed", q); - assertFalse("root.leaf1 is empty", queueManager.isEmpty(q)); + assertFalse("root.leaf1 is empty", q.isEmpty()); ApplicationAttemptId applicationAttemptId = ApplicationAttemptId.newInstance(applicationId, 0); @@ -578,12 +578,12 @@ public void testApplicationAssignmentPreventsRemovalOfDynamicQueue() queueManager.removeEmptyDynamicQueues(); q = queueManager.getLeafQueue("root.leaf1", false); assertNotNull("root.leaf1 has been removed", q); - assertFalse("root.leaf1 is empty", queueManager.isEmpty(q)); + assertFalse("root.leaf1 is empty", q.isEmpty()); // the appAttempt finished, the queue should be empty q.removeApp(appAttempt); q = queueManager.getLeafQueue("root.leaf1", false); - assertTrue("root.leaf1 is not empty", queueManager.isEmpty(q)); + assertTrue("root.leaf1 is not empty", q.isEmpty()); // removeEmptyDynamicQueues() should remove the queue queueManager.removePendingIncompatibleQueues(); @@ -593,8 +593,7 @@ public void testApplicationAssignmentPreventsRemovalOfDynamicQueue() } @Test - public void testRemovalOfIncompatibleNonEmptyQueue() - throws Exception { + public void testRemovalOfIncompatibleNonEmptyQueue() { AllocationConfiguration allocConf = scheduler.getAllocationConfiguration(); allocConf.configuredQueues.get(FSQueueType.LEAF).add("root.a"); scheduler.allocConf = allocConf; @@ -602,13 +601,15 @@ public void testRemovalOfIncompatibleNonEmptyQueue() FSLeafQueue q = queueManager.getLeafQueue("root.a", true); assertNotNull("root.a does not exist", q); - assertTrue("root.a is not empty", queueManager.isEmpty(q)); + assertTrue("root.a is not empty", q.isEmpty()); // we start to run an application on root.a - notEmptyQueues.add(q); + ApplicationId appId = ApplicationId.newInstance(0,0); + q.addAssignedApp(appId); + q = queueManager.getLeafQueue("root.a", false); assertNotNull("root.a does not exist", q); - assertFalse("root.a is empty", queueManager.isEmpty(q)); + assertFalse("root.a is empty", q.isEmpty()); // root.a should not be removed by removeEmptyDynamicQueues or by // removePendingIncompatibleQueues @@ -626,20 +627,90 @@ public void testRemovalOfIncompatibleNonEmptyQueue() // since root.a has running applications, it should be still a leaf queue q = queueManager.getLeafQueue("root.a", false); assertNotNull("root.a has been removed", q); - assertFalse("root.a is empty", queueManager.isEmpty(q)); + assertFalse("root.a is empty", q.isEmpty()); // removePendingIncompatibleQueues should still keep root.a as a leaf queue queueManager.removePendingIncompatibleQueues(); q = queueManager.getLeafQueue("root.a", false); assertNotNull("root.a has been removed", q); - assertFalse("root.a is empty", queueManager.isEmpty(q)); + assertFalse("root.a is empty", q.isEmpty()); // when the application finishes, root.a should be a parent queue - notEmptyQueues.clear(); + q.removeAssignedApp(appId); queueManager.removePendingIncompatibleQueues(); queueManager.removeEmptyDynamicQueues(); FSParentQueue p = queueManager.getParentQueue("root.a", false); assertNotNull("root.a does not exist", p); } + @Test + public void testRemoveDeepHierarchy() throws Exception { + AllocationConfiguration allocConf = scheduler.getAllocationConfiguration(); + queueManager = new QueueManager(scheduler); + queueManager.initialize(conf); + queueManager.updateAllocationConfiguration(allocConf); + + // create a deeper queue hierarchy + FSLeafQueue q = queueManager.getLeafQueue("root.p1.p2.p3.leaf", true); + assertNotNull("root.p1.p2.p3.leaf does not exist", q); + assertTrue("root.p1.p2.p3.leaf is not empty", q.isEmpty()); + + // Add an application to make the queue not empty + ApplicationId appId = ApplicationId.newInstance(0,0); + q.addAssignedApp(appId); + + // remove should not remove the queues + queueManager.removePendingIncompatibleQueues(); + queueManager.removeEmptyDynamicQueues(); + q = queueManager.getLeafQueue("root.p1.p2.p3.leaf", false); + assertNotNull("root.p1.p2.p3.leaf does not exist", q); + + // Remove the application + q.removeAssignedApp(appId); + // Cleanup should remove the whole tree + queueManager.removeEmptyDynamicQueues(); + q = queueManager.getLeafQueue("root.p1.p2.p3.leaf", false); + assertNull("root.p1.p2.p3.leaf does not exist", q); + FSParentQueue p = queueManager.getParentQueue("root.p1", false); + } + + @Test + public void testRemoveSplitHierarchy() throws Exception { + AllocationConfiguration allocConf = scheduler.getAllocationConfiguration(); + queueManager = new QueueManager(scheduler); + queueManager.initialize(conf); + queueManager.updateAllocationConfiguration(allocConf); + + // create a deeper queue hierarchy + FSLeafQueue leaf1 = queueManager.getLeafQueue("root.p1.p2-1.leaf-1", true); + assertNotNull("root.p1.p2-1.leaf-1 does not exist", leaf1); + assertTrue("root.p1.p2-1.leaf1 is not empty", leaf1.isEmpty()); + // Create a split below the first level + FSLeafQueue leaf2 = queueManager.getLeafQueue("root.p1.p2-2.leaf-2", true); + assertNotNull("root.p1.p2-2.leaf2 does not exist", leaf2); + assertTrue("root.p1.p2-2.leaf2 is not empty", leaf2.isEmpty()); + + // Add an application to make one of the queues not empty + ApplicationId appId = ApplicationId.newInstance(0,0); + leaf1.addAssignedApp(appId); + + // remove should not remove the non empty split + queueManager.removePendingIncompatibleQueues(); + queueManager.removeEmptyDynamicQueues(); + leaf1 = queueManager.getLeafQueue("root.p1.p2-1.leaf-1", false); + assertNotNull("root.p1.p2-1.leaf-1 does not exist", leaf1); + leaf2 = queueManager.getLeafQueue("root.p1.p2-2.leaf-2", false); + assertNull("root.p1.p2-2.leaf2 does exist", leaf2); + FSParentQueue p = queueManager.getParentQueue("root.p1.p2-2", false); + assertNull("root.p1.p2-2 does exist", p); + + // Remove the application + leaf1.removeAssignedApp(appId); + // Cleanup should remove the whole tree + queueManager.removeEmptyDynamicQueues(); + leaf1 = queueManager.getLeafQueue("root.p1.p2-1.leaf-1", false); + assertNull("root.p1.p2-1.leaf-1 does exist", leaf1); + p = queueManager.getParentQueue("root.p1", false); + assertNull("root.p1 does exist", p); + } }