> On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java, line 181 > > <https://reviews.apache.org/r/16232/diff/5/?file=398501#file398501line181> > > > > revert
done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java, line 247 > > <https://reviews.apache.org/r/16232/diff/5/?file=398501#file398501line247> > > > > remove extra newline done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java, line 250 > > <https://reviews.apache.org/r/16232/diff/5/?file=398501#file398501line250> > > > > when you said "i'll fix formatting later" in person, i had a hunch it > > wouldn't happen :-) done. One day I am going to get IntelliJ to do this for me. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java, line 256 > > <https://reviews.apache.org/r/16232/diff/5/?file=398501#file398501line256> > > > > put the line break before .toinstance rather than here done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/AsyncModule.java, line 259 > > <https://reviews.apache.org/r/16232/diff/5/?file=398501#file398501line259> > > > > remove this comment, it's the other one that stands out as strange done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java, line 176 > > <https://reviews.apache.org/r/16232/diff/5/?file=398503#file398503line176> > > > > Style nit: i prefer to leave an empty line between switch case blocks: > > > > switch (result) { > > case SUCCESS: > > ... > > break; > > > > case TRY_LATER: > > ... done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java, line 82 > > <https://reviews.apache.org/r/16232/diff/5/?file=398504#file398504line82> > > > > s/public //. everything declared in an interface is implicitly public done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java, line > > 181 > > <https://reviews.apache.org/r/16232/diff/5/?file=398504#file398504line181> > > > > How about "maybePreemptFor"? done. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java, line > > 210 > > <https://reviews.apache.org/r/16232/diff/5/?file=398504#file398504line210> > > > > 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. It just makes the tests too hard to read IMHO. Even creating test helper methods just make it difficult to understand what is expected and what should not be happening. I also feel a serious attempt should be in another diff. > On Dec. 16, 2013, 3:23 p.m., Bill Farner wrote: > > src/main/java/com/twitter/aurora/scheduler/async/TaskScheduler.java, line > > 249 > > <https://reviews.apache.org/r/16232/diff/5/?file=398504#file398504line249> > > > > s/this.// done. - Zameer ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16232/#review30498 ----------------------------------------------------------- On Dec. 16, 2013, 3: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, 3: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 > >
