Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: yarn-issues@hadoop.apache.org
Date: Tue, 9 Sep 2014 05:22:29 +0000 (UTC)
From: "Wangda Tan (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.12714119.1400048094000.25151.1410240149295@Atlassian.JIRA>
In-Reply-To: <JIRA.12714119.1400048094000@Atlassian.JIRA>
References: <JIRA.12714119.1400048094000@Atlassian.JIRA>
 <JIRA.12714119.1400048094445@arcas>
Subject: [jira] [Commented] (YARN-2056) Disable preemption at Queue level
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/YARN-2056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14126621#comment-14126621 ] 

Wangda Tan commented on YARN-2056:
----------------------------------

Hi [~eepayne],
Thanks for update the patch. Sorry for late, I've jus take a look at your patch, I think the new added test looks good to me, only a couple of minor comments,
1.
{code}
+    // verify capacity taken from queueB, not queueE despite queueE being far
+    // over its absolute guaranteed capacity
{code}
queueE isn't preempted because its parent queue still under satisfied, as you know, it's internal mechanism of preemption policy. I think it's better to add it to the comment, can save some time when people looking at the test. 

2.
{code}
+    ApplicationAttemptId expectedAttemptOnQueueB = 
+        ApplicationAttemptId.newInstance(
+            appA.getApplicationId(), appA.getAttemptId());
+    assertTrue("appA should be running on queueB",
+        mCS.getAppsInQueue("queueB").contains(expectedAttemptOnQueueB));
{code}
It's better to remove such assertion, it's unrelated to preemption policy. I guess you added it here because you want to check if mockQueue/mockApp is correct. I suggest you can add a separated test to verify mock nest queue/app.

Also some similar checks on queueC/E 

3. failed test TestCapacitySchedulerQueueACLs should not related to this change, but it's better to re-kick jenkins to make sure it.

Thanks,
Wangda


> Disable preemption at Queue level
> ---------------------------------
>
>                 Key: YARN-2056
>                 URL: https://issues.apache.org/jira/browse/YARN-2056
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>    Affects Versions: 2.4.0
>            Reporter: Mayank Bansal
>            Assignee: Eric Payne
>         Attachments: YARN-2056.201408202039.txt, YARN-2056.201408260128.txt, YARN-2056.201408310117.txt, YARN-2056.201409022208.txt
>
>
> We need to be able to disable preemption at individual queue level


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)