I did a fairly quick pass on the uber-patch, and it looks good to me. The only small annoyance is the trivial overrides of equals() and hashCode() just to keep findbugs quiet. I've seen this before in other patches, and I just don't understand why findbugs requires you to create no-op overrides when you have no intention of changing the base behavior. Perhaps we should file a minor JIRA to fix findbugs rule on this?
On Thu, Nov 10, 2016 at 6:19 PM, Karthik Kambatla <[email protected]> wrote: > Forgot to mention > > - All the patches were reviewed before getting committed to the branch. > - The changes are strictly internal to the scheduler. Most of it is > limited to FairScheduler with minimal changes adding helper methods to > common scheduler code. > > > > On Thu, Nov 10, 2016 at 4:46 PM, Karthik Kambatla <[email protected]> > wrote: > > > Hi folks > > > > We have been working on overhauling FairScheduler preemption on branch > > YARN-4572. It is close to being ready for merge to trunk: > > > > 1. Preemption considers individual ResourceRequests to satisfy. > > (YARN-5605) > > 2. Preemption now works within a leaf queue and across sibling leaf > > queues. (YARN-5605) > > 3. Comprehensive unit tests for app starvation and preemption - > > minshare and fairshare. (YARN-5783 and YARN-5819, the latter is close > to > > commit) > > 4. TODO: Clean up the TODOs to remove my initials and replace them > > with appropriate JIRAs. > > > > There are some unresolved sub-tasks in the umbrella JIRA, but none of > them > > are regressions in the new implementation. > > > > I just uploaded the cumulative patch to YARN-4752 for Jenkins > verification > > and will follow up on any issues that come up. > > > > Would like to hear your thoughts on the merge. > > > > Thanks > > Karthik > > > > PS: Post facto, I feel a feature branch was unnecessary for this work. > > Github PRs with multiple commits for ease of review would have been > enough. > > >
