Craig Welch commented on YARN-3318:

bq. Beyond SchedulerApplicationAttempt which is pending YARN-3361, Few comments 
on latest patch:

I think you misunderstood, the patch doesn't depend on 3361, but after 3361 is 
in some things should be removed from this patch.  In any case, I decided that 
it really belonged in the integration patch, [YARN-3463], so I've dropped it 
from here and it will be committed there

bq. 1) CACHED_USED/CACHED_PENDING don't used by anybody, are they pending 
YARN-3361 as well? 

No, that was a miss during the ResourceUsage usage changes!  Something which 
could affect functionality!  Amazing, fixed.

bq. 2) AbstractComparatorOrderingPolicy doesn't handle locks, I suggest to add 
synchronized lock to all methods if you think it will only be used in 
single-thread scenario

Since the api returns iterators which must be externally synchronized, 
OrderingPolicy makes it clear in documentation that the burden for 
synchronization rests with the user (the schedulers).  That's the threading 
model, so synchronizing here would be pointless

bq. 3) FifoComparator, it will be used by FairOrderingPolicy as well? If so, 
better to make it to a separated class

sure, done

bq. 4) How about call getInfo to getStatusMessage, since the "info" is too 
generic. And add a comment to indicate it will be used for logger printing.

sure, done

bq. 5) getComparator of AbstractComparatorOrderingPolicy is @VisibleForTest?

sure, done

> 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, YARN-3318.52.patch, 
> YARN-3318.53.patch, YARN-3318.56.patch, YARN-3318.57.patch, 
> YARN-3318.58.patch, YARN-3318.59.patch, YARN-3318.60.patch
> Create the initial framework required for using OrderingPolicies and an 
> initial FifoOrderingPolicy

This message was sent by Atlassian JIRA

Reply via email to