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

Konstantinos Karanasos commented on YARN-2883:
----------------------------------------------

Thanks for the feedback, [~chris.douglas] and [~kasha]!

I am in the process of addressing Chris' comments -- will upload a new patch 
soon.

Regarding Karthik's comments:
bq. Any reason we use a map instead of a queue to store the queued containers?
I am using a Map only to track the allocated containers; for the queued 
containers, I am using a queue, as you suggest.

bq. I like that QueuingContainerManagerImpl extends ContainerManagerImpl - 
while we harden the queuing side of things, it will help keep the code clean. 
In the longer run, we might want to default to Queuing implementation and play 
with the queue length, but we can cross that bridge when we get there.
Agreed, that was exactly our intention too.

bq. IIUC, the intent is to use queueing for all opportunistic containers. The 
ContainerManagerImpl implementation seems to depend on whether queuing is 
enabled - wouldn't that affect all containers and not just opportunistic 
containers?
In most cases (including distributed scheduling and resource over-commitment), 
queues will indeed only be used for opportunistic containers.
However, as long as queuing is enabled, guaranteed containers might need to be 
queued momentarily until the opportunistic containers that block their 
execution get killed.
That's the reason you see guaranteed containers going through the same 
code-path too. But again, this will not break any semantics of the guaranteed 
containers.

bq. The patch has the author's name left against a TODO. Also, we don't want to 
leave orphaned TODOs - let us go ahead and file a JIRA
True, I will make sure I remove any TODOs and author names.

bq. The ResourceUtilization changes are not strictly related to this patch, do 
they?
This is correct. I put them in this JIRA because they are just a couple of 
methods. Do you think I should create a separate JIRA for this?

bq. TestQueuingContainerMgr: We typically don't wrap imports at 80 chars.
Yep, will fix that.

> Queuing of container requests in the NM
> ---------------------------------------
>
>                 Key: YARN-2883
>                 URL: https://issues.apache.org/jira/browse/YARN-2883
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, resourcemanager
>            Reporter: Konstantinos Karanasos
>            Assignee: Konstantinos Karanasos
>         Attachments: YARN-2883-trunk.004.patch, 
> YARN-2883-yarn-2877.001.patch, YARN-2883-yarn-2877.002.patch, 
> YARN-2883-yarn-2877.003.patch, YARN-2883-yarn-2877.004.patch
>
>
> We propose to add a queue in each NM, where queueable container requests can 
> be held.
> Based on the available resources in the node and the containers in the queue, 
> the NM will decide when to allow the execution of a queued container.
> In order to ensure the instantaneous start of a guaranteed-start container, 
> the NM may decide to pre-empt/kill running queueable containers.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to