hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Craig Welch (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3318) Create Initial OrderingPolicy Framework and FifoOrderingPolicy
Date Sat, 04 Apr 2015 03:38:33 GMT

    [ https://issues.apache.org/jira/browse/YARN-3318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14395508#comment-14395508
] 

Craig Welch commented on YARN-3318:
-----------------------------------

[~vinodkv]

bq. ...We can strictly focus on the policy framework here...

Sure, limited patch to "framework"

bq. ...You could also say SchedulableProcess...

SchedulableProcess it is, done

bq. I agree to this, but we are not in a position to support the APIs, CLI, config names in
a supportable manner yet. They may or may not change depending on how parent queue policies,
limit policies evolve. For that reason alone, I am saying that (1) Don't make the configurations
public yet, or put a warning saying that they are unstable and (2) don't expose them in CLI
, REST APIs yet. It's okay to put in the web UI, web UI scraping is not a contract.

You can't see it, because it's part of "Capacity Scheduler Integration", but removed CLI and
proto related change.  There was no rest api change, the web UI change is still present. 
Will warn unstable when added to config files in the scheduler integration patch

bq. SchedulerApplicationAttempt.getDemand() should be private

Done

bq. updateCaches() -> updateState() / updateSchedulingState() as that is what it is doing?
 getCachedConsumption() / getCachedDemand(): simply getCurrent*() ? What is the need for reorderOnContainerAllocate
() / reorderOnContainerRelease()?

Is now getSchedulingConsumption(); getSchedulingDemand(); updateSchedulingState();

This is needed because mutable values which are used for ordering cannot be allowed to change
for an item in the tree, else it will not be found in some cases during the delete before
reinsert process which occurs when a schedulable's mutable values used in comparison change
(for fairness, changes to consumption and potentially demand)  Not all OrderingPolicies require
reordering on these events, for efficiency they get to decide if they do or not, hence the
"reorderOn".  The "reorderOn" are now   reorderForContainerAllocation  reorderForContainerRelease

bq. Move all the comparator related classed into their own package
No longer needed as comparators are now just a property of policies, see below for details

bq. This is really a ComparatorBasedOrderingPolicy. Do we really see non-comparator based
ordering-policy. We are unnecessarily adding two abstractions - adding policies and comparators

Originally, there was a perceived need to be able to support a more flexible interface than
the comparator one, but also a desire to build up a simpler, composible abstraction to be
used with an instance of the former which had "most of the hard stuff done".  Given that all
of the policies we've contemplated building fit the latter abstraction and the level of flexibility
does not appear to actually be that different, I think it's fair to say that we only need
what was previously the "SchedulerComparator" abstraction as a plugin-point.  Given that,
a slightly refactored version of the "SchedulerComparator" abstraction is now the only plugin
point and is now what goes by the name of "OrderingPolicy".  What was previously the "OrderingPolicy"
is now a single concrete class implementing the surrounding logic, meant to be usable from
any scheduler, named "SchedulingOrder".  So, one abstraction, a comparator-based ordering-policy.
 If we really do find we need a flexibility we don't have some day, the SchedulingOrder class
could be abstracted to provide that higher level abstraction - but as we see no need for it
now, and it appears probably never will, there's no reason to do so at present

bq. ...Use className.getName()...

Done

[~leftnoteasy]

bq. ...I prefer what Vinod suggested, split "SchedulerProcess" to be "QueueSchedulable" and
"AppSchedulable" ...

I don't see that he has suggested that.  In any case, with the removal of "*Serial*" and the
move to compareInputOrderTo() I don't at present see a need to have separate subtypes for
app and queue to avoid "dangling properties".  And, I think if we do it right we won't end
up introducing them.  By splitting in the suggested way we commit ourselves to either multiple
comparators (to use the differing functionality) or awkward testing of subtype/etc logic in
one comparator - so it basically moves the complexity/awkwardness, it doesn't eliminate it.
 I've refactored such that the Policy now provides a Comparator as opposed to extending it,
so there is now room for it to provide multiple comparators and handle subtypes if need be,
but I think we should wait until we see that we must do that before doing so, as I don't believe
we will end up needing to (but if we do, existing code should need little change, and implementing
what you suggest should be essentially additive...)

bq. ...About inherit relationships between interfaces/classes...

Policies will be composed to achieve combined capabilities yet the collection of SchedulableProcesses
& as such must reside in one instance, so attempting to place that logic in the multiple-instances
(where multiple policies are constructed, each containing a SchedulerProcess collection via
inheritance) is a non starter.  Before the latest refactor this was handled by having a single
SchedulerComparatorOrderingPolicy with multiple Comparators, post the latest refactor the
SchedulingOrder is a single concrete type containing the SchedulableProcess collection, and
using one or more OrderingPolicies which possess the policy specific logic (including, but
not limited to, comparator logic).  As it happens, they do not appear to need to share logic,
so there is no super class, just a shared interface implementation - were that to change we
could certainly adjust the implementation, but there's no need to now, and it's an implementation
not an api issue anyway.

bq. ...Regarding relationship between OrderingPolicy and comparator...

There's no longer this duality, there are now just OrderingPolicies

bq. ...Regarding configuration...

There's now just one type of configuration, for policy, and will handle "friendly policy labels".
 Only the fifo is available now, and generalized composition logic, it's in SchedulingOrder












> 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
>
>
> Create the initial framework required for using OrderingPolicies and an initial FifoOrderingPolicy



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message