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

Wangda Tan commented on YARN-2932:
----------------------------------

[~eepayne],
Thanks for update, after looked at your latest patch, I think ver.3 is more 
clear to me comparing to ver.5. It looks bad to check the {{null}} at 
{{getQueuePreemptable}}. And ver.3 supports queue reinitialize, it was my 
misunderstanding about logics in your patch, sorry for adding unnecessary work 
to you.

Two nits on ver.3: 
1) In {{isQueuePathHierarchyPreemptable}}
You can use:
parentQ.preemptable instead of isQueuePathHierarchyPreemptable(parentQ).

Instead of
{code}
+    return csConf.isQueuePreemptable(q.getQueuePath(),
+                                     isQueuePathHierarchyPreemptable(parentQ));
{code}

The latter one is a recursive method call and potentially makes debug harder. 
CS.parseQueue assumes parent queue get initialized before children queues.

2) Still in {{isQueuePathHierarchyPreemptable}}
Use {{DEFAULT_RM_SCHEDULER_ENABLE_MONITORS}} instead of hard coded {{false}} 
when getting system-wide-preemptable. It is possible in the future we turn on 
preemption policy by default, only one place we will need to edit in that case. 

Besides the two nits, patch LGTM, could you rebase ver.3 against latest trunk 
and kick Jenkins?

Thanks,
Wangda

> Add entry for "preemptable" status to scheduler web UI and queue 
> initialize/refresh logging
> -------------------------------------------------------------------------------------------
>
>                 Key: YARN-2932
>                 URL: https://issues.apache.org/jira/browse/YARN-2932
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 3.0.0, 2.7.0
>            Reporter: Eric Payne
>            Assignee: Eric Payne
>         Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt, YARN-2932.v3.txt, 
> YARN-2932.v4.txt, YARN-2932.v5.txt
>
>
> YARN-2056 enables the ability to turn preemption on or off on a per-queue 
> level. This JIRA will provide the preemption status for each queue in the 
> {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue 
> refresh.



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

Reply via email to