[
https://issues.apache.org/jira/browse/YARN-3318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486256#comment-14486256
]
Wangda Tan commented on YARN-3318:
----------------------------------
Hi Craig,
Thanks for updating, some comments:
1) Regarding {{OrderingPolicy}} and {{SchedulingOrder}} responsibilities:
If I understand correctly, SchedulingOrder plays a bridge role between
scheduler and policy, but do we really need this additional layer? Such layer
is helpful when we need support different OrderingPolicy interface, since we're
defining new interfaces, I don't think that will be a target.
Looking at methods of OrderingPolicy, most of them are just pass through
parameters to OrderingPolicy, and rest of them are instantiation
OrderingPolicies.
So I suggest to remove the SchedulingOrder class to reduce complexity, and
create a OrderingPolicyFactory class to encapsulate all instantiation methods.
2) OrderingPolicy should be a per-queue instance or global library?
I think it's better to make Policy per queue since it needs some internal
fields which are different between each queues. SchedulingOrder class is to
store per-queue states, I think we can merge them to OrderingPolicy.
3) Suggestion about OrderingPolicy interface design (if you agree with 1/2):
- reorderForContainerAllocation is not needed, instead, it's better to call it
inform/notifyContainerAllocated(RMContainer r). You don't need to keep
SchedulableProcess, it's unclear that if the "Schedulable" is from
app/user/leaf-queue. OrderingPolicy will figure it out.
- As same as reorderForContainerRelease
- SchedulingComparator is actually implementation detail, the usaful thing is
getAssignmentIterator/getPreemptionIterator.
- add/remove/addAllSchedulableProcess should be a part of OrderingPolicy
4) CompoundOrderingPolicy is implemenation detail for FairOrderingPolicy, don't
need put in the patch.
5) SchedulableProcess:
- "Process" is easily to be confused with "process in the system", I prefer
SchedulableEntity even if "Entity" and "Process" are all overloaded words, at
least "Entity" is not so confusing to me :)
- About spliting SchedulableProcess to App and Queue, I think we need consider
queue's requirements since we're defining new interfaces. FairScheduler has
property to say a "depth" of a policy to avoid mis-applying queue/app's policy,
which I don't like. Separating App/Queue is not equals to "instanceof" need add
everywhere. Just check once when adding them is enough.
- As I mentioned before, use {{ResourceUsage}} is much better, it can avoid
defining too much methods like getDemand, etc. and respect node-labels.
SchedulerApplicationAttempt and all CSQueues has {{ResourceUsage}}. We should
make node-label as first-class citizen. Your concern about ResourceUsage is
tracked by YARN-3440.
> Create Initial OrderingPolicy Framework and FifoOrderingPolicy
> --------------------------------------------------------------
>
> 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, YARN-3318.34.patch, YARN-3318.35.patch,
> YARN-3318.36.patch, YARN-3318.39.patch, YARN-3318.45.patch,
> YARN-3318.47.patch, YARN-3318.48.patch
>
>
> Create the initial framework required for using OrderingPolicies and an
> initial FifoOrderingPolicy
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
