> On March 19, 2015, 9:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java, > > line 45 > > <https://reviews.apache.org/r/32220/diff/2/?file=899548#file899548line45> > > > > This should refer to the parameter, not the guice binding annotation.
Done. > On March 19, 2015, 9:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java, > > line 87 > > <https://reviews.apache.org/r/32220/diff/2/?file=899548#file899548line87> > > > > `synchronized` should be unnecessary, `Cache` is expected to be thread > > safe, and we're not doing multiple operations per method here. Yeah, I used synchronized when I was not sure about the final functionality but now it's clearly redundant. dropped. > On March 19, 2015, 9:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, > > line 124 > > <https://reviews.apache.org/r/32220/diff/2/?file=899549#file899549line124> > > > > "Validates that a previously-found" Done. > On March 19, 2015, 9:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java, > > line 84 > > <https://reviews.apache.org/r/32220/diff/2/?file=899552#file899552line84> > > > > Given that you probably want a singleton PreemptionSlotCache, don't > > forget to bind that as such here. Good idea. Done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/#review77106 ----------------------------------------------------------- On March 19, 2015, 12:29 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32220/ > ----------------------------------------------------------- > > (Updated March 19, 2015, 12:29 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 > ------- > > This diff makes preemption asynchronous wrt the scheduling loop. New flow > works as follows: > - TaskScheduler is unable to schedule a task and calls into > PreemptorImpl.attemptPreemptionFor() to acquire a reservation. > - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, > fires an async slot search request and replies back with empty result. > - A search is finished and a slot (if found) is added into internal slot > cache. > - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, > validates a slot is still valid and preempts tasks. A reservation for a slave > is created. > > This is still an intermediate milestone on the way to a fully independent > background preemptor. > > Benchmark refactoring will be addressed in a separate diff. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/async/OfferManager.java > 7d2cb46aa86dd4c3c6d53848725eed1542307ebd > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java > e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java > 1808b71546423dfe80ccb1902e8cebd545674a27 > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java > 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java > dbfebf99bc6028faf433a69db4308a239ff61290 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java > cfbc1a039262d92481ded2733d50ac51293a5b91 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java > e329358f70028f52a807cd987378cbc002af36a9 > > Diff: https://reviews.apache.org/r/32220/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
