+1 from me too. minor nit: * In FSAppAttempt, maybe rename addPreemption(container) / removePreemption(container) to something better.
Also, had an offline discussion with Karthik. Suggested to maybe experiment with replacing the PreemptionThread with a synchronous preemption approach. For eg. Maybe keep adding to the FSStarvedApps and when it reaches a threshold, trigger a preemption. It would likely avoid race conditions and reduce synchronization costs.. but am ok with trying this out and experimenting in follow up JIRAs. Cheers -Arun On Tue, Nov 15, 2016 at 1:43 PM, Daniel Templeton <[email protected]> wrote: > +1 from me, but no surprise since I was the branch reviewer. > > Aside from being functionally superior to the existing code, this branch > is also cleaner and better tested code. We did our best to make sure that > the patches were high quality and all the testing bases were covered. > > To add to Karthik's points, YARN-5819 was committed early this morning, > and the JIRA for his TODO bullet is YARN-5885. > > Daniel > > > On 11/10/16 6:19 PM, Karthik Kambatla 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. >>> >>> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
