Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/#review195060 --- Ship it! Master (8e5e08e) is green with this patch.

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Bill Farner
> On Jan. 9, 2018, 10:44 a.m., Jordan Ly wrote: > > src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java > > Lines 369-372 (original), 356-358 (patched) > > > > > > nit: can you just expect

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/#review195054 --- Ship it! Thanks for the benchmarks! LGTM

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/ --- (Updated Jan. 9, 2018, 10:32 a.m.) Review request for Aurora, Daniel Knightly

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Bill Farner
> On Jan. 4, 2018, 4:46 p.m., Jordan Ly wrote: > > Nice patch! Do you have any benchmarks comparing this refactor to the > > current master? Scheduling benchmarks suggest that this does not affect performance. While i expected as much, it's good to see there are no surprises! master: ```

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Bill Farner
> On Jan. 4, 2018, 4:46 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java > > Lines 152-158 (original), 141-144 (patched) > > > > > > Is this check effectively

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Bill Farner
> On Jan. 4, 2018, 11:42 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/TierManager.java > > Line 105 (original), 106 (patched) > > > > > > This was a potential bug - `TierManager` relied on

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/#review194810 --- Nice patch! Do you have any benchmarks comparing this refactor to

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Jordan Ly
> On Jan. 4, 2018, 7:42 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/TierManager.java > > Line 105 (original), 106 (patched) > > > > > > This was a potential bug - `TierManager` relied on

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/#review194775 --- Reviewer notes

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/#review194779 --- Ship it! Master (8e5e08e) is green with this patch.

Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64954/ --- Review request for Aurora, Daniel Knightly and Jordan Ly. Repository: aurora