Vinod Kumar Vavilapalli commented on YARN-3318:

bq. I think it is useful to split off CS changes into their own JIRA. We can 
strictly focus on the policy framework here.
You missed this, let's please do this.

bq. well, I'd actually talked Wangda Tan into SchedulerProcess So, we can chew 
on this a bit more & see where we go
SchedulerProcess is definitely misleading. It seems to point to a process that 
is doing scheduling. What you need is a Schedulable  / SchedulableEntity / 
Consumer etc. You could also say SchedulableProcess, but Process is way too 

bq.  The goal is to make this available quickly but iteratively, keeping the 
changes small but making them available for use and feedback. (..) We should 
grow it organically, gradually, iteratively, think of it is a facet of the 
policy framework hooked up and available but with more to follow
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.

bq.     You add/remove applications to/from LeafQueue's policy but 
addition/removal of containers is an event...
bq. This has been factored differently along Wangda Tan's suggestion, it should 
now be consistent
It's a bit better now. Although we are hard-coding Containers. Can revisit this 

Other comments
 - SchedulerApplicationAttempt.getDemand() should be private.
 - SchedulerProcess
    -- updateCaches() -> updateState() / updateSchedulingState() as that is 
what it is doing?
    -- getCachedConsumption() / getCachedDemand(): simply getCurrent*() ?
 - SchedulerComparator
  -- We aren't comparing Schedulers. Given the current name, it should have 
been SchedulerProcessComparator, but SchedulerProcess itself should be renamed 
as mentioned before.
  -- What is the need for reorderOnContainerAllocate () / 
 - Move all the comparator related classed into their own package.
 - SchedulerComparatorPolicy
  -- This is really a ComparatorBasedOrderingPolicy. Do we really see 
non-comparator based ordering-policy. We are unnecessarily adding two 
abstractions - adding policies and comparators.
  -- Use className.getName() instead of hardcoded strings like 

> 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, YARN-3318.39.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