[
https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Eric Payne updated YARN-2932:
-----------------------------
Attachment: YARN-2932.v2.txt
Thanks very much, [~leftnoteasy], for your thorough review of this patch and
for your helpful comments.
{quote}
1) Since the QUEUE_PREEMPTION_DISABLED is an option for CS, I suggest to make
it as a member of CapacitySchedulerConfiguration, like
getUserLimitFactor/setUserLimit, etc. This will void some String operations.
{quote}
This is a good idea. I added {{isQueuePreemptable}} and
{{setQueuePreemptable}}. For {{isQueuePreemptable}}, I needed to add a default
value parameter because the default for the queue at a particular level should
be whatever its parent's value is.
{quote}
2) Rename {{context}} in {{AbstractCSQueue}} to name like {{csContext}} since
we have {{rmContext}}
{quote}
Renamed.
{quote}
3) I suggest to add a member var like {{preemptable}} to {{AbstractCSQueue}},
instead of calling:
{code}
+ @Private
+ public boolean isPreemptable() {
+ return context.getConfiguration().isPreemptable(getQueuePath());
+ }
{code}
The implementation of {{CSConfiguration.isPreemptable(..)}} seems too complex
to me. {{CSConfiguration}} should only care about value of configuration file,
such logic should put to {{AbstractCSQueue.setupQueueConfigs(...)}}
{quote}
I moved the logic to {{AbstractCSQueue.setupQueueConfigs(...)}}, and you are
right. It is much cleaner that way. Thanks!
{quote}
4) It's better to web UI name (preemptable) and configuration name
(disable_preemption) consistent. I prefer "preemptable" personally.
{quote}
Yes, it is less confusing that way. In this patch, the only things that worry
about the {{disable_preemption}} property are the internals of the
{{CSConfiguration}} methods. The APIs are now all asking whether or not the
queue is preemptable.
{quote}
5) {{testIsPreemptable}} should be a part of {{TestCapacityScheduler}} instead
of putting it to {{TestProportionalCapacityPreemptionPolicy}}.
{quote}
Thanks. I moved the test to {{testIsPreemptable}}. However, since the interface
for changing a queue's preemptability changed, there were also several changes
to {{TestProportionalCapacityPreemptionPolicy}}.
{quote}
6) In {{ProportionalCapacityPreemptionPolicy.cloneQueues}}, preemptable field
should get from Queue instead of getting from configuration.
{quote}
Done.
> Add entry for preemption setting to queue status screen and startup/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-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)
