Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============5383833942900404116=="
MIME-Version: 1.0
Subject: Re: Review Request 32352: Making preemptor asynchronous. Part
 3(final) - background service.
From: "Stephan Erb" <stephan@dev.static-void.de>
To: "Bill Farner" <wfarner@apache.org>, "Zameer Manji" <zmanji@apache.org>
Cc: "Stephan Erb" <stephan@dev.static-void.de>,
 "Maxim Khutornenko" <maxim@apache.org>,
 "Aurora" <reviews@aurora.incubator.apache.org>
Date: Mon, 23 Mar 2015 22:00:26 -0000
Message-ID: <20150323220026.32590.12102@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Stephan Erb" <noreply@reviews.apache.org>
References: <20150322100440.32591.99454@reviews.apache.org>
In-Reply-To: <20150322100440.32591.99454@reviews.apache.org>
Reply-To: "Stephan Erb" <stephan@dev.static-void.de>

--===============5383833942900404116==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On March 22, 2015, 11:04 a.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 107
> > <https://reviews.apache.org/r/32352/diff/1/?file=902121#file902121line107>
> >
> >     Slotes computed here might have overlapping victims. A simple way to mitigate this problem could be to properly randomize the slave traversal order in `findPreemptionSlotFor`
> 
> Maxim Khutornenko wrote:
>     Thanks for bringing this up! The existing algorithm is optimized to work for a single pending task in a sequence with victim preemption. By splitting apart slot search and actual preemption we definitely lose some efficiency. 
>     
>     I am afraid randomizing slave traversal will not address the problem completely and will bring some unpredictability in execution order. We need to optimize the N(pendingTasks) x M(slaves) problem in a more structured fashion, possibly reversing the traversal from tasks->slaves to slaves->tasks to further improve efficiency (as slave count is expected to be >> pending task count under normal conditions). I have already left a TODO to optimize slot search for multiple pending tasks. Will file a ticket to capture all these points.

Your plan sounds good to me.


- Stephan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32352/#review77348
-----------------------------------------------------------


On March 21, 2015, 3:19 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32352/
> -----------------------------------------------------------
> 
> (Updated March 21, 2015, 3:19 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1158
>     https://issues.apache.org/jira/browse/AURORA-1158
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Summary of changes:
> - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction.
> - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots.
> - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots.
> - preemption_delay is reduced from 10 to 3 minutes.
> 
> Benchmark refactoring will be addressed separately.
> 
> 
> Diffs
> -----
> 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 
> 
> Diff: https://reviews.apache.org/r/32352/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


--===============5383833942900404116==--