Zian Chen commented on YARN-8524:

[~snemeth] thanks for working on this issue and provide the patch. I have some 
concerns here,

1. Is it better to give some code comments here to clarify this is for creating 
all resources with the same value? Also, I think there are still some 
confusions about the logic here,
ConfigurableResource(long value) {

The logic here remains the same as the original code, which is setting all 
resource types with the same value as the input "value" here, but what if the 
user is explicitly want to just set memory and vcore with the input value but 
leave other resource types as blank? I think is better to distinguish all the 
possible intentions here, such as,

     1) user only want to set memory, vcore with the same value.

          i) if there is only memory and vcore involves, no other resource types

          ii) if there are other resources other than memory and vcore, but 
user just wants to leave other resources blank.

     2) user want to set memory, vcore with the same value, but other resources 
with different value. (actually this can be covered by 
Resource#newInstance(long, int, java.util.Map<java.lang.String,java.lang.Long>) 
really easy)


2. For the newly added UT TestResources#testCreateResourceWithSameValue, we can 
add one more test case to give input with int value, like 

Resource res = Resources.createResourceWithSameValue(11);

to make sure int value is also acceptable.


> Single parameter Resource / LightWeightResource constructor looks confusing
> ---------------------------------------------------------------------------
>                 Key: YARN-8524
>                 URL: https://issues.apache.org/jira/browse/YARN-8524
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: api
>            Reporter: Szilard Nemeth
>            Assignee: Szilard Nemeth
>            Priority: Major
>         Attachments: YARN-8524.001.patch
> The single parameter (long) constructor in Resource / LightWeightResource 
> sets all resource components to the same value.
> Since there are other constructors in these classes with (long, int) 
> parameters where the semantics are different, it could be confusing for the 
> users.
> The perfect place to create such a resource would be in the Resources class, 
> with a method named like "createResourceWithSameValue".

This message was sent by Atlassian JIRA

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