[
https://issues.apache.org/jira/browse/YARN-7612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16295858#comment-16295858
]
Konstantinos Karanasos commented on YARN-7612:
----------------------------------------------
I looked at the latest patch and had an offline discussion with [~asuresh].
First, we agreed to split the current JIRA in three parts to better review it:
* API of the processor framework
* Implementation of the processor framework
* Coupling with Capacity Scheduler
Some first comments in the meantime:
* I know it might not be that easy, but let's try to remove the
isConstraintedAllocation from the RMContainer. It will simplify a lot the code
of the CapacityScheduler adn the FicaSchedulerApp.
* One downside of the current implementation is that it relies on the commit
API that is there only in the CapacityScheduler... This is not a blocker for
now, but we should see in another JIRA what it will take to make this work for
the Fair Scheduler too.
* Do we need the placeApplication in the RMAppImpl?
* CapacityScheduler:
** Can we unify the two createResourceCommitRequest in the CapacityScheduler
that seem to duplicate a lot of code?
** In the createResourceCommitRequest, since it assumes we request a single
container in the SchedulingRequest, shouldn't we add a check for that?
** The SchedulerContainer is confusing... Looks like an inner class to me, for
sure it deserves a better name.
* Does the NodeCandidateSelector belong to the constraint/processor package?
* ApplicationMasterService: since biggest part of the serviceInit is now the
AMS-processor-chain related stuff, let's put all that initialization in a
separate amsProcessorInit method.
Also some minor comments:
* YarnConfiguration: placement.algorithm -> constraint-placement.algorithm
* yarn_protos.proto:
** RR prefix reminds me of ResourceRequest, instead of RejectionReason. Same in
ProtoUtils. Maybe we can do sth like PRR (PlacementRR).
** COULD_NOT_SCHEDULE_ON_PLACED_NODE -> COULD_NOT_SCHEDULE_ON_NODE
* In the ApplicationMasterServiceUtils, I would put the
setRejectedSchedulingRequests inside the first if clause, assuming most
responses will not have a rejection.
* Typo in RMActiveServiceContext, RMContext, and RMContextImpl:
PlacementConstriantsManager
* RMContainerImpl: isConstraintAllocation, is ConstraintPlacement… Make
consistent
* ResourceScheduler:
** True is -> true if
** tryAllocate -> attemptAllocation? Or better attemptAllocationOnNode?
** "Propose a SchedulerRequest for the Scheduler to try allocate" -> "attempt
to place a SchedulerRequest"
* constraint/api/Algorithm: rename to sth like PlacementAlgorithm
> Add Placement Processor Framework
> ---------------------------------
>
> Key: YARN-7612
> URL: https://issues.apache.org/jira/browse/YARN-7612
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Arun Suresh
> Assignee: Arun Suresh
> Attachments: YARN-7612-YARN-6592.001.patch,
> YARN-7612-YARN-6592.002.patch, YARN-7612-YARN-6592.003.patch,
> YARN-7612-YARN-6592.004.patch, YARN-7612-YARN-6592.005.patch,
> YARN-7612-YARN-6592.006.patch, YARN-7612-v2.wip.patch, YARN-7612.wip.patch
>
>
> This introduces a Placement Processor and a Planning algorithm framework to
> handle placement constraints and scheduling requests from an app and places
> them on nodes.
> The actual planning algorithm(s) will be handled in a YARN-7613.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
