----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16232/#review30498 -----------------------------------------------------------
src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java <https://reviews.apache.org/r/16232/#comment58344> revert src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java <https://reviews.apache.org/r/16232/#comment58345> remove extra newline src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java <https://reviews.apache.org/r/16232/#comment58347> when you said "i'll fix formatting later" in person, i had a hunch it wouldn't happen :-) src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java <https://reviews.apache.org/r/16232/#comment58348> put the line break before .toinstance rather than here src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java <https://reviews.apache.org/r/16232/#comment58349> remove this comment, it's the other one that stands out as strange src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java <https://reviews.apache.org/r/16232/#comment58350> Style nit: i prefer to leave an empty line between switch case blocks: switch (result) { case SUCCESS: ... break; case TRY_LATER: ... src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java <https://reviews.apache.org/r/16232/#comment58351> s/public //. everything declared in an interface is implicitly public src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java <https://reviews.apache.org/r/16232/#comment58353> How about "maybePreemptFor"? src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java <https://reviews.apache.org/r/16232/#comment58352> Since the plumbing seems to be in place to avoid exposing this, can you try to make it private? This has the added benefit of sidestepping exposing TaskSchedulerImpl in AsyncModule. src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java <https://reviews.apache.org/r/16232/#comment58354> s/this.// - Bill Farner On Dec. 16, 2013, 11:04 p.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16232/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2013, 11:04 p.m.) > > > Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. > > > Bugs: AURORA-11 > https://issues.apache.org/jira/browse/AURORA-11 > > > Repository: aurora > > > Description > ------- > > This patch adds a reservation system the preemption flow. > > The reservation associates a slave id with a task id for a fixed duration. If > the task attempts to schedule itself during that time period and an offer is > available from that slave then it will be scheduled. If another task attempts > to schedule itself then it will not use the reserved offer. > > > Diffs > ----- > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java > db07841543e554e269f6fe7b36d7f7232af21140 > src/main/java/com/twitter/aurora/scheduler/async/Preemptor.java > e5aeb8321e22c51eb3a5dad3d3dd1e26b7121b7d > src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java > f95f719c5a444b0f8faa4330852e251dd5de528e > src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java > fbd82ff70235294cfd27c242f141a585d6bb2396 > src/test/java/com/twitter/aurora/scheduler/async/TaskSchedulerImplTest.java > PRE-CREATION > src/test/java/com/twitter/aurora/scheduler/async/TaskSchedulerTest.java > a747f2b1ecbad7263931aeec3b12711096d2469d > src/test/java/com/twitter/aurora/scheduler/state/PubsubTestUtil.java > f9d7fb64728008d0ea6eb424283b58e956e1d50a > > Diff: https://reviews.apache.org/r/16232/diff/ > > > Testing > ------- > > ./gradlew clean build > > > Thanks, > > Zameer Manji > >
