[
https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15645799#comment-15645799
]
Daniel Templeton commented on YARN-5819:
----------------------------------------
This is just a first-pass review. After we settle on YARN-5783, I'll take a
closer look.
Maybe I'm old, but I don't like non-incremental _for_ loops:
{code}
- FSQueue queue = getQueue();
- while (!queue.getQueueName().equals("root")) {
+ for (FSQueue queue = getQueue();
+ !queue.getQueueName().equals("root");
+ queue = queue.getParent()) {
{code}
If you want to be safer about the _while_ loop, create an {{Iterator}}.
In {{FSPremeptionThread}} you left some dead code:
{code}
// boolean preemptable = app.canContainerBePreempted(container);
{code}
In {{TestFairSchedulerPreemption}}, {{app1}} and {{app2}} would be nicer with
more meaningful names, e.g. {{greedyApp}} and {{starvingApp}}.
Is it worth it to combine the two config file writing methods into a single one
that takes a boolean parameter. It's more succinct, but I'm not if sure the
hit to readability is worth it.
There's an extra space after the cast in {{setupCluster()}}:
{code}
scheduler = (FairScheduler) resourceManager.getResourceScheduler();
{code}
Any way to share the {{addNode()}} and {{sendNodeUpdateEvents()}} methods with
{{TestFSAppStarvation}}?
Drop the hyphen from the {{@throws}} message in {{submitApps()}}. Might be
nice to put the parameters references in {{@code}} tags.
{{preemptable}} and {{nonpreemptable}} might be better names than {{allowed}}
and {{disallowed}}. It took me a little while to figure out what was allowed
and disallowed.
> Verify fairshare and minshare preemption
> ----------------------------------------
>
> Key: YARN-5819
> URL: https://issues.apache.org/jira/browse/YARN-5819
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: fairscheduler
> Affects Versions: 2.9.0
> Reporter: Karthik Kambatla
> Assignee: Karthik Kambatla
> Attachments: yarn-5819.YARN-4752.1.patch
>
>
> JIRA to track the unit test(s) verifying both fairshare and minshare
> preemption. The tests should verify:
> # preemption within a single leaf queue
> # preemption between sibling leaf queues
> # preemption between non-sibling leaf queues
> # {{allowPreemption = false}} should disallow preemption from a queue
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
