[
https://issues.apache.org/jira/browse/YARN-7237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16193135#comment-16193135
]
Daniel Templeton commented on YARN-7237:
----------------------------------------
The changes made look good. On the open topics:
bq. {quote}Rather than creating our own minimum and maximum profiles based on
the usual minimums and maximums, I rather get rid of the minimum and maximum
profiles completely and go back to the original code for enforcing minimums and
maximums.{quote} I would prefer to keep it so that we can get a completed list
of profiles including minimum and maximum.
What do we get from having a completed list of profiles? Before resource
profiles were added, we dealt with the configured min and max directly. I
don't see where making it a profile makes the code any cleaner or easier. In
fact, adding another layer of indirection here adds complication without any
clear benefit. As I'm sure you've noticed by now, I'm very interested in
keeping things as simple as possible and only adding complication when it adds
value. (Or in this case, removing complication if it doesn't add value.)
If you insist on keeping the minimum and maximum profiles, then you need tests
to validate that the profile names are actually disallowed.
bq. {quote}testResourceProfiles() looks like it tests more than just the
minimum and maximum profiles. Are you sure you should remove it?{quote}I'm not
quite understand this comment, could you explain?
In the patch you removed {{testResourceProfiles()}}, but it looks like the test
was testing more than just the minimum and maximum profiles. My question is
whether it might not be better to just remove the minimum and maximum profile
portions of the test rather than removing the whole test.
> Cleanup usages of ResourceProfiles
> ----------------------------------
>
> Key: YARN-7237
> URL: https://issues.apache.org/jira/browse/YARN-7237
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, resourcemanager
> Reporter: Wangda Tan
> Assignee: Wangda Tan
> Priority: Critical
> Attachments: YARN-7237.001.patch, YARN-7237.002.patch,
> YARN-7237.003.patch, YARN-7237.004.patch, YARN-7237.005.patch
>
>
> While doing tests, there're a couple of issues:
> 1) When use {{ProfileCapability#getProfileCapabilityOverride}}, it does
> overwrite of whatever specified in resource-profiles.json when value >= 0.
> Which is different from javadocs of {{ProfileCapability}}
> bq. For example, if you have a resource profile "small" that maps to <4096M,
> 2 cores, 1 gpu> and you set the capability override to <8192M, 0 cores, 0
> gpu>, then the actual resource allocation on the ResourceManager will be
> <8192M, 2 cores, 1 gpu>
> To me, the correct behavior should do overwrite when value > 0. The reason
> is, by default resource value will be set to 0, For example, assume we have a
> profile {{"a" = (mem=3, vcore=5, res_1=7)}}, and create a
> capability-overwrite (capability = new resource(8). The final result should
> be (mem=8, vcore=5, res_1=7), instead of (mem=8, vcore=0, res_1=0).
> 2) ResourceProfileManager now loads minimum/maximum profile from config file
> (resource-profiles.json), to me this is not correct because minimum/maximum
> allocation for each resource types are already specified inside
> {{resource-types.xml}}. We should always use
> {{ResourceUtils#getResourceTypesMinimum/MaximumAllocation}} to get from
> resource-types.xml and yarn-site.xml. This value will be added to profiles so
> client can get these configs.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
