Sunil G commented on YARN-4537:

Thanks [~rohithsharma] for sharing the patch.
Generally patch looks good.

Few minor nits:
private CompoundComparator fifoComparator;
I feel this variable is not needed in {{FifoOrderingPolicy}}

2. Also in {{PriorityComparator}} if needed we can remove the temporary 
variable {{res}}.
3. {{TestFifoOrderingPolicy}}, could u pls add a case where priority is null 
too. Since we have a comparator, its good we have a case for all corner cases.

> Pull out priority comparison from fifocomparator and use compound comparator 
> for FifoOrdering policy
> ----------------------------------------------------------------------------------------------------
>                 Key: YARN-4537
>                 URL: https://issues.apache.org/jira/browse/YARN-4537
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: capacity scheduler
>            Reporter: Rohith Sharma K S
>            Assignee: Rohith Sharma K S
>         Attachments: 0001-YARN-4537.patch
> Currently, priority comparison is integrated with FifoComparator. There 
> should be a separate comparator defined for priority comparison so that down 
> the line if any new ordering policy wants to integrate priority, they can use 
> compound comparator where priority will be high preference. 
> The following changes are expected to be done as part of this JIRA
> # Pull out priority comparison from FifoComparator
> # Define new priority comparator
> # Use compound comparator for FifoOrderingPolicy. Oder of preference is   
> Priority,FIFO

This message was sent by Atlassian JIRA

Reply via email to