[
https://issues.apache.org/jira/browse/YARN-3318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14387544#comment-14387544
]
Wangda Tan commented on YARN-3318:
----------------------------------
Hi Craig,
Thanks for working on the patch, I just took a look, some overall comments for
interface definitions:
*1) {{SchedulerProcess}}*
1.1. It's name seems not very straightforward to me, many processes can run for
an application. How about call it SchedulableEntity (to not confuse with fair
scheduler's Schedulable)?
1.2. I found {{SchedulerProcessEvent}} defined in SchedulerProcess, is there
any specific reason to make SchedulerProcess to be asynchronized? As far as I
can imagine, fair/capacity(fifo)/priority, all of them need synchronized call
(or synchronized calls are enough). The reason is, if you don't want to write a
brand-new scheduler, now all schedulers assume scheduler event need handle
under scheduler lock.
1.3. SerialEpoch and Serial not straight forward to me also, how about call
them clusterTimeStamp and submissionId?
1.4. currentConsumption is not enough to make choice,
demand(pending-resource)/used and priority/weight are basic fields of a
Schedulable, do you think so? I suggest you can take a look at ResourceUsage
class, now it becomes field of
CSQueue/LeafQueue.User/SchedulerApplicationAttempt already, it should have
enough resource-related fields to make scheduling decisions. And it considered
node-label as well.
*2) {{OrderingPolicy}}*
2.1 addSchedulerProcess/removeSchedulerProcess/addAllSchedulerProcesses should
become interface of Orderingpolicy. Now doing getSchedulerProcess().add(..)
assumes internal implementation is treeSet. It's better not depend on internal
Collection<> behavior.
2.2 {{handleSchedulerProcessEvent}} makes the OrderingPolicy mixed
synchronized/asynchronized writer interface, I suggest to make it to be,
containerAllocated/Preempted(SchedulerProcess, RMContainer)
2.3 Rename it to be {{SchedulerProcessOrderingPolicy}}?
*3) {{SchedulerComparatorPolicy}}*
I understand this class is to make different comparators can share same policy
implementation, and admins can combine different comparators via configuration.
But it seems a little hard to use for me.
For example, now admin need configure following options to enable fair
schedling per queue:
{code}
policy=SchedulerComparatorPolicy
comparator=CompoundComparator, and its comparators are {fair and fifo}
{code}
Instead, I think we need hide internal implementation of policy/comparator, we
can predefine policy=fair means using CompoundComparator (fair+fifo) and policy
= capacity means using fifo.
We can still leave admin flexibility to configure their custom
policy/comparator, but we need reduce configuration complexity for most common
usecases.
Will include review for tests once we have consent on interfaces.
> Create Initial OrderingPolicy Framework, integrate with CapacityScheduler
> LeafQueue supporting present behavior
> ---------------------------------------------------------------------------------------------------------------
>
> Key: YARN-3318
> URL: https://issues.apache.org/jira/browse/YARN-3318
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: scheduler
> Reporter: Craig Welch
> Assignee: Craig Welch
> Attachments: YARN-3318.13.patch, YARN-3318.14.patch,
> YARN-3318.17.patch
>
>
> Create the initial framework required for using OrderingPolicies with
> SchedulerApplicaitonAttempts and integrate with the CapacityScheduler. This
> will include an implementation which is compatible with current FIFO behavior.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
