From e259a818adb9c95bf2966f0898defc4a4d41bb01 Mon Sep 17 00:00:00 2001 From: Karthik Kambatla Date: Thu, 9 Feb 2017 14:45:04 -0800 Subject: [PATCH] YARN-6144. FairScheduler: preempted resources can become negative. (Miklos Szegedi via kasha) (cherry picked from commit 08f93978f3ec724b24a93d7ef538f158da75802f) --- .../scheduler/fair/FSAppAttempt.java | 16 +++++++++------- .../scheduler/fair/TestFairScheduler.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) 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/FSAppAttempt.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/FSAppAttempt.java index 5f5d14cecdc..9333caacfa9 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/FSAppAttempt.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/FSAppAttempt.java @@ -542,18 +542,20 @@ void resetMinshareStarvation() { } void trackContainerForPreemption(RMContainer container) { - containersToPreempt.add(container); - synchronized (preemptedResources) { - Resources.addTo(preemptedResources, container.getAllocatedResource()); + if (containersToPreempt.add(container)) { + synchronized (preemptedResources) { + Resources.addTo(preemptedResources, container.getAllocatedResource()); + } } } private void untrackContainerForPreemption(RMContainer container) { - synchronized (preemptedResources) { - Resources.subtractFrom(preemptedResources, - container.getAllocatedResource()); + if (containersToPreempt.remove(container)) { + synchronized (preemptedResources) { + Resources.subtractFrom(preemptedResources, + container.getAllocatedResource()); + } } - containersToPreempt.remove(container); } Set getPreemptionContainers() { 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/TestFairScheduler.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/TestFairScheduler.java index 5c86f98d47e..cc68b36f291 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/TestFairScheduler.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/TestFairScheduler.java @@ -3640,6 +3640,8 @@ public void testQueueMaxAMShare() throws Exception { scheduler.handle(updateEvent); assertEquals("Application1's AM should be finished", 0, app1.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app1.getResourceUsage()); assertEquals("Application3's AM should be running", 1, app3.getLiveContainers().size()); assertEquals("Application3's AM requests 1024 MB memory", @@ -3658,6 +3660,8 @@ public void testQueueMaxAMShare() throws Exception { 0, app4.getAMResource().getMemorySize()); assertEquals("Application4's AM should not be running", 0, app4.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app4.getResourceUsage()); assertEquals("Queue1's AM resource usage should be 2048 MB memory", 2048, queue1.getAmResourceUsage().getMemorySize()); @@ -3672,6 +3676,8 @@ public void testQueueMaxAMShare() throws Exception { 0, app5.getAMResource().getMemorySize()); assertEquals("Application5's AM should not be running", 0, app5.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app5.getResourceUsage()); assertEquals("Queue1's AM resource usage should be 2048 MB memory", 2048, queue1.getAmResourceUsage().getMemorySize()); @@ -3683,6 +3689,8 @@ public void testQueueMaxAMShare() throws Exception { scheduler.handle(updateEvent); assertEquals("Application5's AM should not be running", 0, app5.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app5.getResourceUsage()); assertEquals("Queue1's AM resource usage should be 2048 MB memory", 2048, queue1.getAmResourceUsage().getMemorySize()); @@ -3697,8 +3705,12 @@ public void testQueueMaxAMShare() throws Exception { scheduler.handle(updateEvent); assertEquals("Application2's AM should be finished", 0, app2.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app2.getResourceUsage()); assertEquals("Application3's AM should be finished", 0, app3.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app3.getResourceUsage()); assertEquals("Application5's AM should be running", 1, app5.getLiveContainers().size()); assertEquals("Application5's AM requests 2048 MB memory", @@ -3718,6 +3730,8 @@ public void testQueueMaxAMShare() throws Exception { scheduler.handle(containerExpired); assertEquals("Application5's AM should have 0 container", 0, app5.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app5.getResourceUsage()); assertEquals("Queue1's AM resource usage should be 2048 MB memory", 2048, queue1.getAmResourceUsage().getMemorySize()); scheduler.update(); @@ -3740,6 +3754,8 @@ public void testQueueMaxAMShare() throws Exception { scheduler.handle(updateEvent); assertEquals("Application6's AM should not be running", 0, app6.getLiveContainers().size()); + assertEquals("Finished application usage should be none", + Resources.none(), app6.getResourceUsage()); assertEquals("Application6's AM resource shouldn't be updated", 0, app6.getAMResource().getMemorySize()); assertEquals("Queue1's AM resource usage should be 2048 MB memory",