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

Íñigo Goiri commented on YARN-8827:
-----------------------------------

Thanks [~asuresh] for the patch.
I think we can fix the findbugs, checkstyles and so on from Yetus.

Other minor comments:
* I think that ContainersMonitorImpl#run is single threaded and all the 
accesses to {{perAppResourceUtilization}} are made here, no need for the 
thread-safe comment; nothing is here.
* In ContainersMonitorImpl, we are doing the change of units twice (#825), can 
we refactor that into a proper ResourceUtilization and use it in both places?
* ContainersMonitorImpl#1106, what does it mean?
* TestResourceUtilizationAggregator#line66 fits in one single line. (There are 
a few more places like #171, #174, etc).
* TestResourceUtilizationAggregator for values like 8192, you can do 8*GB as 
the rest of the class.
* Can we do a more informed wait in TestResourceUtilizationAggregator instead 
of sleep(1000)?
* mkmap() and e() are an OK pattern but I would probably call the second one 
mkmapentry and add javadoc for the two explaining the purpose.
* Can we use an static import for {{Assert.assertEquals()}}?
* The code in {{testResourceUtilizationAggregation}} is a little repetitive. 
Given that this class is only used for this, I would make the NMs part of the 
class and trigger registrations and so on as part of the class. I would also 
add a couple more comments throughout explaining how the apps are submitting 
requests, etc. Specially the numbers after #184.
* TestResourceUtilizationAggregator#184 can it be made calculated from some 
assignment we track?

> Plumb per app, per user and per queue resource utilization from the NM to RM
> ----------------------------------------------------------------------------
>
>                 Key: YARN-8827
>                 URL: https://issues.apache.org/jira/browse/YARN-8827
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Arun Suresh
>            Assignee: Arun Suresh
>            Priority: Major
>         Attachments: YARN-8827-YARN-1011.01.patch
>
>
> Opportunistic Containers for OverAllocation need to be allocated to pending 
> applications in some fair manner. Rather than evaluating queue and user 
> resource usage (allocated resource usage) and comparing against queue and 
> user limits to decide the allocation, it might  make more sense to use a 
> snapshot of actual resource utilization of the queue and user.
> To facilitate this, this JIRA proposes to aggregate per user, per app (and 
> maybe per queue) resource utilization in addition to aggregated Container and 
> Node Utilization and send it along with the NM heartbeat. It should be fairly 
> inexpensive to aggregate - since it can be performed in the same loop of the 
> {{ContainersMonitorImpl}}'s Monitoring thread.
> A snapshot aggregate can be made every couple of seconds in the RM. This 
> instantaneous resource utilization should be used to decide if Opportunistic 
> containers can be allocated to an App, Queue or User.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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