MENG DING commented on YARN-1643:

Thanks [~jianhe] for the review:

bq. Here, memory * 2^20, but it gets reverted later on at info.pmemLimit >> 20, 
we can just use the original value ?
Will do.

bq. Do you think we can change the trackingContainers to be concurrentHashMap 
and update the ptInfo directly ? Also the getter and setter of ptInfo can 
synchronize on the ptInfo object
Yes, we can make {{trackingContainers}} a {{ConcurrentHashMap}}, and add setter 
to {{ProcessTreeInfo}} for vemLimit, pmemLimit, and cpuVcores, then have the 
getter and setter synchronized on the object. 

IIUC, the main benefit is that we don't need to synchronize on the 
{{enforceResourceLimits}} call, which can be heavy, right? If that is the case, 
we probably also need to have proper synchronization for 
{{ResourceCalculatorProcessTree}}, e.g., 
{{ProcfsBasedProcessTree}}/{{WindowsBasedProcessTree}}? These objects could be 
updated by multiple threads as well. I was afraid that the code change may be 
too much?

For other objects like {{containersToBeChanged}}, {{containersToBeAdded}}, 
{{containersToBeRemoved}}, I think we still need to synchronize on the entire 
map like the way it is right now, because we are calling  functions like 


> Make ContainersMonitor can support change monitoring size of an allocated 
> container in NM side
> ----------------------------------------------------------------------------------------------
>                 Key: YARN-1643
>                 URL: https://issues.apache.org/jira/browse/YARN-1643
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>            Reporter: Wangda Tan
>            Assignee: MENG DING
>         Attachments: YARN-1643-YARN-1197.4.patch, 
> YARN-1643-YARN-1197.5.patch, YARN-1643-YARN-1197.6.patch, YARN-1643.1.patch, 
> YARN-1643.2.patch, YARN-1643.3.patch

This message was sent by Atlassian JIRA

Reply via email to