[
https://issues.apache.org/jira/browse/YARN-8842?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16644473#comment-16644473
]
Wilfred Spiegelenburg commented on YARN-8842:
---------------------------------------------
Thank you [~snemeth]. I am glad we gave this its own jira separate from the
pre-emption changes. It has become quite large on its own.
The patch overall looks good. I have a couple of remarks:
* {{getMaxAllocationUtilization}} is new and not used outside of the patch, why
would we add it if we're only going to leverage it in YARN-8059?
* With that simplification we also do not need the tests for
{{getMaxAllocationUtilization}} in the {{TestQueueMetricsForCustomResources}}
* There is a possible NPE in {{getAllocatedResources}} combined with the fact
that I do not understand the need for the {{isThereAnyAllocatedResource}}, the
way the newInstance is created we can pass in an empty list or even a null. I
would expect the method to look like this:
{code}
if (queueMetricsForCustomResources != null) {
return Resource.newInstance(allocatedMB.value(), allocatedVCores.value(),
queueMetricsForCustomResources.getAllocatedValues());
}
return Resource.newInstance(allocatedMB.value(), allocatedVCores.value());
{code}
That does bring the {{QueueMetricsAllocatedCustomResources}} class back in line
with all other custom resource metrics classes, just extend the abstract.
* Please look at the checkstyle issues mentioned.
A nit: on my system a number of indent inconsistencies showed up (mainly in
TestQueueMetricsForCustomResources, QueueMetricsTestcase & TestQueueMetrics)
> Update QueueMetrics with custom resource values
> ------------------------------------------------
>
> Key: YARN-8842
> URL: https://issues.apache.org/jira/browse/YARN-8842
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Szilard Nemeth
> Assignee: Szilard Nemeth
> Priority: Major
> Attachments: YARN-8842.001.patch, YARN-8842.002.patch,
> YARN-8842.003.patch, YARN-8842.004.patch, YARN-8842.005.patch,
> YARN-8842.006.patch, YARN-8842.007.patch, YARN-8842.008.patch
>
>
> This is the 2nd dependent jira of YARN-8059.
> As updating the metrics is an independent step from handling preemption, this
> jira only deals with the queue metrics update of custom resources.
> The following metrics should be updated:
> * allocated resources
> * available resources
> * pending resources
> * reserved resources
> * aggregate seconds preempted
--
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
