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

Daniel Templeton commented on YARN-5783:
----------------------------------------

Thanks for the patch, [~kasha].  Questions and comments:

* In {{FSContext}}, what's the reason behind always creating the 
{{FSStarvedApps}}?
* Will javadocs be coming in another JIRA?
* Unrelated to this patch, the {{FSPreemptionThread.run()}} method exits on a 
{{RuntimeException}} preemption will just stop working.  We should probably set 
up a default exception handler that reacts more strongly.
* In {{FSStarvedApps}}, should {{apps}} be renamed to {{appsToBeProcessed}} now?
* In {{TestFSAppStarvation}}:
** I think {{ALLOC_FILE}} would be better as a {{File}}.
** {{rmNodes}} should be {{final}}.
** The tests should copy {{conf}} instead of modifying it directly.
** In {{testPreemptionEnabled()}}:
*** Is the sleep/update fool proof?  Should there be a loop?  Should the sleep 
be a yield?
*** The assert message {{"Incorrect starved applications"}} is a little weirdly 
worded and not very informational.
** Would it make sense to test with thresholds other than 0 and 1.1?
** In {{sendNodeUpdateEvents()}}, you should explain why you're sending the 
update 4 times.
** In {{setupStarvedCluster()}}, the for loop to add nodes is overkill. Since 
the whole thing is built around exactly two nodes, I'd hard-code it.

> Unit tests to verify the identification of starved applications
> ---------------------------------------------------------------
>
>                 Key: YARN-5783
>                 URL: https://issues.apache.org/jira/browse/YARN-5783
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: fairscheduler
>    Affects Versions: 2.8.0
>            Reporter: Karthik Kambatla
>            Assignee: Karthik Kambatla
>              Labels: oct16-medium
>         Attachments: yarn-5783.YARN-4752.1.patch, yarn-5783.YARN-4752.1.patch
>
>
> JIRA to track unit tests to verify the identification of starved 
> applications. An application should be marked starved only when:
> # Cluster allocation is over the configured threshold for preemption.
> # Preemption is enabled for a queue and any of the following:
> ## The queue is under its minshare for longer than minsharePreemptionTimeout
> ## One of the queue’s applications is under its fairshare for longer than 
> fairsharePreemptionTimeout.



--
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

Reply via email to