Wangda Tan commented on YARN-1651:

Thanks review! [~jianhe].
bq. I think for now we can fail the allocate call explicitly on those very 
clear situations in checkAndNormalizeContainerChangeRequest ?, e.g. the 
situation that rmContainer doesn't exist That's more explicit to users. Digging 
through logs is not an easy thing for application writer.
Done, now we will check it in both AMS/Scheduler, exception will be thrown in 
AMS. Doing both check because AMS doesn't acquire scheduler lock, so it is 
still possbile that RMContainer state changed when adding to scheduler.

bq. RMNodeImpl#toBeDecreasedContainers - no need to be a map, it can be a list 
? and therefore NodeHeartBeatResponse and Impl change is not needed; similarly 
nmReportedIncreasedContainers can be a list.
This is to avoid AM decrease same container multiple times between same NM 
heartbeats, this is a rare edge case. Similar for NM reports 
increasedContainers, if we decouple NM heartbeat and scheduler allocation, we 
could have container increased multiple times between scheduler looks at NM.

bq. When decreasing a container, should it send RMNodeDecreaseContainerEvent 
too ?
Done, added test to confirm this as well.

bq. looks like when decreasing reservedIncreasedContainer, it will unreserve 
the whole extra reserved resource, should it only unreserve the extra resources 
being decresed ?
Decrease container is decrease resource of a container to lower than confirmed 
resource. If a container is 2G, AM asks to increase to 4G, it can only decrease 
it to less than 2G before increase issued. So I think we need to unreserve the 
whole extra reserved resource.

bq. In general, I think we should be able to decrease/increase a regular 
reserved container or a increasedReservedContainer ?
Container reservation is an internal state of scheduler, AM doesn't know about 
the reserved container at know, so far I think we don't need to expose that to 

bq. allocate call is specifically marked as noLock, but now every allocate call 
holds the global scheduler lock which is too expensive. we can move 
decreaseContainer to application itself.
DecreaseContainer is as same as completedContainer, both acquire scheduler lock 
and queue lock. I think we can optimize it in the future, which we can add them 
to something like "pendingReleased" list, and will be traversed periodically.
I added comments to CS#allocate to explain about this, the "NoLock" is not 100% 

And addressed all other comments.

Comment addressed.

> CapacityScheduler side changes to support increase/decrease container 
> resource.
> -------------------------------------------------------------------------------
>                 Key: YARN-1651
>                 URL: https://issues.apache.org/jira/browse/YARN-1651
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager, scheduler
>            Reporter: Wangda Tan
>            Assignee: Wangda Tan
>         Attachments: YARN-1651-1.YARN-1197.patch, 
> YARN-1651-2.YARN-1197.patch, YARN-1651-3.YARN-1197.patch, 
> YARN-1651-4.YARN-1197.patch, YARN-1651-5.YARN-1197.patch, 
> YARN-1651-6.YARN-1197.patch

This message was sent by Atlassian JIRA

Reply via email to