[
https://issues.apache.org/jira/browse/YARN-1012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14614442#comment-14614442
]
Karthik Kambatla commented on YARN-1012:
----------------------------------------
Thanks for updating the patch, Inigo. Looks better, few minor comments:
# ResourceUtilization of containers is calculated only when container
monitoring is enabled. Given the low overhead of sending this across, I think
it is okay to not add another config to choose whether to send this information
across or not. However, if container-monitoring is not enabled, we should
probably not send it across. Accordingly, would it make sense to not modify
{{NodeStatus#newInstance}} and just call the setter when required.
# ResourceUtilization
## javadoc has an error
{code}
*
* @see Resource
*/
{code}
## Now that we have moved it to be server-side only, does it need to Public?
How about changing it to Private-Evolving
## We don't need to add annotations for individual methods if it is the same as
the enclosing class.
# ContainersMonitorImpl - Extra space below
{code}
// Save the aggregated utilization of the containers
setContainersUtilization(trackedContainersUtilization );
{code}
# NodeStatus doesn't need to ResourceUtilization
# NodeStatusUpdaterImpl doesn't need to import Records
> Report NM aggregated container resource utilization in heartbeat
> ----------------------------------------------------------------
>
> Key: YARN-1012
> URL: https://issues.apache.org/jira/browse/YARN-1012
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager
> Affects Versions: 2.7.0
> Reporter: Arun C Murthy
> Assignee: Inigo Goiri
> Attachments: YARN-1012-1.patch, YARN-1012-2.patch, YARN-1012-3.patch,
> YARN-1012-4.patch, YARN-1012-5.patch, YARN-1012-6.patch, YARN-1012-7.patch,
> YARN-1012-8.patch, YARN-1012-9.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
