Return-Path: X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 51B9817E91 for ; Mon, 6 Apr 2015 17:46:50 +0000 (UTC) Received: (qmail 26294 invoked by uid 500); 6 Apr 2015 17:46:50 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 26250 invoked by uid 500); 6 Apr 2015 17:46:50 -0000 Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.apache.org Delivered-To: mailing list reviews@aurora.apache.org Received: (qmail 26221 invoked by uid 99); 6 Apr 2015 17:46:49 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 06 Apr 2015 17:46:49 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id E6C3C1C0369; Mon, 6 Apr 2015 17:46:47 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7913359117232876305==" MIME-Version: 1.0 Subject: Re: Review Request 32597: Improving async preemptor efficiency. From: "Zameer Manji" To: "Bill Farner" , "Zameer Manji" Cc: "Maxim Khutornenko" , "Aurora" Date: Mon, 06 Apr 2015 17:46:47 -0000 Message-ID: <20150406174647.5638.75927@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Zameer Manji" X-ReviewGroup: Aurora X-ReviewRequest-URL: https://reviews.apache.org/r/32597/ X-Sender: "Zameer Manji" References: <20150403033748.16792.18872@reviews.apache.org> In-Reply-To: <20150403033748.16792.18872@reviews.apache.org> Reply-To: "Zameer Manji" X-ReviewRequest-Repository: aurora --===============7913359117232876305== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On April 2, 2015, 8:37 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, line 68 > > > > > > What's the motivation behind the ordering? It also seems somewhat contradictory along with the class doc "fair evaluation order of all task groups regardless of their task count". > > Maxim Khutornenko wrote: > The idea was to give larger jobs a bigger chance to succeed. E.g. in case tasks from different groups equally qualify for a slave, a task from a larger job wins. I am not entirely sold on that idea though. Perhaps we can drop sorting alltogether for simplicity? +1 I think we should also drop this sorting for simplicity. I also don't think we prefer large jobs anywhere else in our scheduling so we should not prefer them here as well/ - Zameer ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32597/#review78759 ----------------------------------------------------------- On April 3, 2015, 1:04 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32597/ > ----------------------------------------------------------- > > (Updated April 3, 2015, 1:04 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-1219 > https://issues.apache.org/jira/browse/AURORA-1219 > > > Repository: aurora > > > Description > ------- > > This is addressing the preemption efficiency loss due to making preemptor async. Summary: > - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first) > - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found > - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor. > > The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 > src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 > src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 > > Diff: https://reviews.apache.org/r/32597/diff/ > > > Testing > ------- > > ./gradlew -Pq build > Manual testing in vagrant > > > Thanks, > > Maxim Khutornenko > > --===============7913359117232876305==--