[jira] [Commented] (YARN-9124) Resolve contradiction in ResourceUtils: addMandatoryResources / checkMandatoryResources work differently

["Adam Antal (JIRA)" <ji...@apache.org>]

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

Adam Antal commented on YARN-9124:
----------------------------------

So I read through the area and I believe this is intended.

There are two occurrences of that function in {{ResourceUtils}}.
Also, {{checkMandatoryResources}} is called right before {{addMandatoryResources}}.
The purpose of {{checkMandatoryResources}} is to ensure that:
1) no legacy (memory, memory-mb, vcores) resource types have been added accidentally either by node-resources.xml or by the yarn.resource-types configuration
2) checking whether the "mandatory" resources (memory, vcores, gpu and fpga) are not misconfigured (checking their unit and type). For e.g. fpga's unit set to Mb makes no sense.

After that operation {{addMandatoryResources}} ensures that all "mandatory" (here memory and vcores) resources are added. The mandatory term is used a bit different here, it only concerns the memory and the cpu.
Also since since the 1) item in {{checkMandatoryResources}}, this will always add both of these resources.

There is no bad code here - or at least I'd rather not start refactoring around this area. I'd be more happy with some extra javadoc's, and as you originally suggested some renames.

> Resolve contradiction in ResourceUtils: addMandatoryResources / checkMandatoryResources work differently
> --------------------------------------------------------------------------------------------------------
>
>                 Key: YARN-9124
>                 URL: https://issues.apache.org/jira/browse/YARN-9124
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Adam Antal
>            Priority: Minor
>
> {{ResourceUtils#addMandatoryResources}}: Adds only memory and vcores as mandatory resources.
> {{ResourceUtils#checkMandatoryResources}}: YARN-6620 added some code to this. This method not only checks memory and vcores, but all the resources referred in ResourceInformation#MANDATORY_RESOURCES.
> I think it would be good to call {{MANDATORY_RESOURCES}} as {{PREDEFINED_RESOURCES}} or something like that and use a similar name for {{checkMandatoryResources}}.



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org