Junping Du commented on YARN-3304:

Thanks [~adhoot] for comments.
bq. Overall the API looks usable and will fix the issues.
Thanks for confirmation on this. That is important.

bq. But the implementation has to do a lot of duplication of code to implement 
the boolean api. The CPU isAvailable is straightforward but the memory 
calculations seem to rely on duplicating code that does the actual calculation.
I admit that there are some duplicate code (with getting CPU/Memory values) 
here. The reason I didn't do more refactor work is less risk of changing 
existing logic at this moment. I think we can refactor it later (in next 
release) to reuse logic for boolean API and get values. Anyway, this belongs to 
implementation not API and we don't have to finalize it now. 

bq. Wouldn't using it as -1 be easier instead of duplicating code to keep it as 
First, replacing default memory resource from 0 to -1 also take extra effort 
and extra checking resource value is not -1 in calculating resource.
Second, like I mentioned above, the consumer side (ContainerMetrics, MR 
resource counter, new TimelineService, etc.) need to handle -1 explicitly and 
existing MR resource counter never have negative value for memory resource 
Last but not the least, as a public API, negative resource value is always more 
misleading than boolean API for resource available.

> ResourceCalculatorProcessTree#getCpuUsagePercent default return value is 
> inconsistent with other getters
> --------------------------------------------------------------------------------------------------------
>                 Key: YARN-3304
>                 URL: https://issues.apache.org/jira/browse/YARN-3304
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager
>            Reporter: Junping Du
>            Assignee: Karthik Kambatla
>            Priority: Blocker
>         Attachments: YARN-3304-v2.patch, YARN-3304-v3.patch, YARN-3304.patch
> Per discussions in YARN-3296, getCpuUsagePercent() will return -1 for 
> unavailable case while other resource metrics are return 0 in the same case 
> which sounds inconsistent.

This message was sent by Atlassian JIRA

Reply via email to