Wangda Tan commented on YARN-3318:

bq. Well, I'm certainly open to other name options, but I do prefer 
SchedulerProcess to SchedulableEntity ... 
I don't like generic "Entity" as well, I cannot get a better name now, so just 
keep it as-is now and see if anybody has better suggestions, renaming should be 
simple :)

bq. Not all event handling must be asynchronous... *My goal was precisely to 
avoid needing to have implementer add a new method implementation every time a 
new event comes into play which may not be of interest to it*
This is one thing I don't agree. CS implemnts EventHandler is that CS will be 
called by other modules in asynchronized way, it handle such events one by one, 
but dispatcher in RM will queuing events for CS. I think SchedulerProcess will 
likely to be used in a sycnhronized way, making EventHandler can make less 
changes for interfaces but also reduces readability. I think this is why all 
scheduler internal classes choose to use setters instead of event handler

bq. SerialEpoch..
I read FairShareComparator and also FifoComparator again, how about keep an 
ID() only, it's a string and should contains everything we need to compare 
"nature" ordering of two SchedulerProcess, instead of policies in FairScheduler 
confusing different fields to compare nature order. We will keep a note that, 
we will use lexicographical order of id() to compare. submitTime should be an 
important property that need to be kept in SchedulerProcess even if we don't 
use it for now.

bq. I looked at switching to using the pending value of the 
attemptResourceUsage in the SchedulerApplicationAttempt but I don't see that it 
is being updated anywhere, am I missing it?
Oh I should mention that, such fields are set in YARN-3361, beyond requirements 
of fair policy, these pending values are used when doing normal scheduling as 
well as preemption.

bq. I think there needs to be a "getCopyPending" or the like, for usecases 
where you get the value and then start to use it.
Good suggestion, I think we should do that to avoid resource being modified 
while using by another reader. I think we can make internal resources in 
ResourceUsage to be "copy-on-write", to make sure resources returned by getters 
is just a snapshot.

bq. I think the current factoring is correct. First, you'll notice that the 
return type of getSchedulerProcess() is a Collection
The thing I'm worrying about is, this assumes caller can modify the collection. 
It is very possible returned collection is just a copy or immutable collection 
to avoid race condition according to different SchedulerProcess impl. So I 
think it's better to make callers can only use limited interfaces to get/set 
internal structure.
And also, there could have some operations need to do when add/remove object in 
the internal data structure, caller shouldn't silently modify it.

bq. ...Simplify configuration...
I suggest you can take a look at YARN-2986, which is very related to this 
We don't need to manually set comparator for most cases, instead, it comes with 
policy. I don't want to expose "comparator" to end user since it's too specific 
to internal logics. Instead, user need just specify queue's policy="fair" and 
set some "policy-options" like application-priority-enabled=true, etc.
"policy=fair" assumes use comparator=fair+fifo, when 
application-priority-enabled, it assumes "priority comparator" added to 
"CompoundComparator" -- they're all internal logics.
The configuration in my mind is (it's hierarchy config, also we can make it to 
be plain config)
  <policy name="fair">

> 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, YARN-3318.34.patch, YARN-3318.35.patch, YARN-3318.36.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

Reply via email to