[ https://issues.apache.org/jira/browse/YARN-3318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14494606#comment-14494606 ]
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 (v6.3.4#6332)