> On March 7, 2015, 5:30 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, > > line 326 > > <https://reviews.apache.org/r/31821/diff/1/?file=888148#file888148line326> > > > > Not yours, but it seems odd that we would call a function and > > internally suppress its behavior based on configuration. Seems like the > > caller should avoid the call if the behavior is not desired. If you agree, > > please TODO. > > Maxim Khutornenko wrote: > Not sure I share your concern. This is a predicate intended to answer a > simple question, which it does quite well. I doubt the alternative would gain > us anything here.
Perhaps i highlighted a line that created confusion. I'm suggesting that this predicate does not belong in this class, as it is used to determine whether to execute the 'body' of the preemptor. I suggest this predicate live in the caller. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75637 ----------------------------------------------------------- On March 9, 2015, 11:34 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31821/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 11:34 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-1158 > https://issues.apache.org/jira/browse/AURORA-1158 > > > Repository: aurora > > > Description > ------- > > Extracting PreemptorSlotFinder to be reused for slot validation in later > stages. The changes are very minimal and mostly around metric handling and > test code. > > Also added missing test coverage. > > > Diffs > ----- > > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 701b9052696337766cb233c865cb9fbb4907071e > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java > e093ca54521ffb9399bb97ce60f510331af70853 > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java > 80c2023f46b63753dcec6a555dba626720a1925a > src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java > bddb9647493b3e7a58c40d4b477a06161c1388a2 > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java > ae56d1e09322869eedd7a27586cd6f96edd64e0a > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java > 85b3874a36ed07c684f26da172952c932cff707a > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java > 58733bdc4dd6de29ccead5cb0a267286e8dc0656 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java > 891cc098cca99e84ba014b7131106ceb0b429b5f > > src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java > 7207867813b0d096772dbc7f92fc1c76937e9831 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java > b0380b3fabb45be8ace55cfcf38ce15ef8040188 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java > 83680769611878886da04e1794b321aa1986e678 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java > 020b67187a18bba64d9b562c3a6c0969fc85d469 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/31821/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
