[
https://issues.apache.org/jira/browse/YARN-5938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15762880#comment-15762880
]
Arun Suresh commented on YARN-5938:
-----------------------------------
Thanks for the review [~leftnoteasy]
bq. OpportunisticContainerAllocator#allocateContainers, why change from
AllocateRequest to several separated param? Is it possible that we need some
other fields in AllocateRequest?
So, Firslty, I wanted to decouple the OpportunisticContainerAllocator from the
AllocateRequest object and Secondly.. we were doing
1. allocateRequest.setAskList(<all opportunistic requests>)
2. then OpportunisticContainerAllocator::allocate()
3. then doing a allocateRequest.setAskList(<all guaranteed requests>)
4. super.allocate()
The above sequence seemed a bit "hacky"... and made a code flow a bit difficult
to follow.. the refactoring fixes that.
It is possible we might need other things later from the AllocateRequest, but
we can address that as and when we need to (there are better ways like updating
the OpportunisticContainerContext etc.)
Will update the patch addressing your remaining comments shortly..
> Minor refactoring to OpportunisticContainerAllocatorAMService
> -------------------------------------------------------------
>
> Key: YARN-5938
> URL: https://issues.apache.org/jira/browse/YARN-5938
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Arun Suresh
> Assignee: Arun Suresh
> Attachments: YARN-5938-YARN-5085.001.patch,
> YARN-5938-YARN-5085.002.patch, YARN-5938-YARN-5085.003.patch,
> YARN-5938.001.patch
>
>
> Minor code re-organization to do the following:
> # The OpportunisticContainerAllocatorAMService currently allocates outside
> the ApplicationAttempt lock maintained by the ApplicationMasterService. This
> should happen inside the lock.
> # Refactored out some code to simplify the allocate() method.
> # Removed some unused fields inside the OpportunisticContainerAllocator.
> # Re-organized some of the code in the
> OpportunisticContainerAllocatorAMService::allocate method to make it a bit
> more readable.
> # Moved SchedulerRequestKey to a new package, so it can be used by the
> OpportunisticContainerAllocator/Context.
> # Moved all usages of Priority in the OpportunisticContainerAllocator ->
> SchedulerRequestKey.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
