Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144693 --- Master (a071af3) is red with this patch. ./build-support/jenkins

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/ --- (Updated Aug. 3, 2016, 3:01 p.m.) Review request for Aurora, Joshua Cohen and M

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Joshua Cohen
> On Aug. 3, 2016, 2:36 a.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle
> On Aug. 2, 2016, 7:36 p.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle
> On Aug. 2, 2016, 7:36 p.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Joshua Cohen
> On Aug. 3, 2016, 2:36 a.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle
> On Aug. 2, 2016, 7:36 p.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144579 --- I was about to push this change, but in the process of running e2e

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144578 --- Ship it! Ship It! - Joshua Cohen On Aug. 2, 2016, 11:38 p.m.

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144566 --- Master (b912e17) is red with this patch. ./build-support/jenkins

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/ --- (Updated Aug. 2, 2016, 4:38 p.m.) Review request for Aurora, Joshua Cohen and M

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144549 --- Ship it! LGTM (barring minor change in the last review cycle).

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Maxim Khutornenko
> On Aug. 1, 2016, 6:28 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java, > > lines 117-121 > > > > > > This will add CPU overhead of a revocable t

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144392 --- Ship it! Master (b912e17) is green with this patch. ./build-s

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/ --- (Updated Aug. 1, 2016, 2:54 p.m.) Review request for Aurora, Joshua Cohen and M

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Renan DelValle
> On Aug. 1, 2016, 11:28 a.m., Maxim Khutornenko wrote: > > docs/operations/configuration.md, line 154 > > > > > > drop trailing space Fixed. > On Aug. 1, 2016, 11:28 a.m., Maxim Khutornenko wrote: > > src/main/

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144368 --- docs/operations/configuration.md (line 154)

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144025 --- Ship it! Master (dde2c92) is green with this patch. ./build-s

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/ --- (Updated July 28, 2016, 3:35 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle
> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java, > > line 158 > > > > > > Are we guaranteed that the executor config exists here? The pr

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle
> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java, > > lines 205-208 > > > > > > I'd drop this in favor of EMPTY overhead alr

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-27 Thread Renan DelValle
> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote: > > CHANGELOG, lines 1-4 > > > > > > This file is updated automatically by our release script. You can > > revert these changes. Got it. Was a bit confused because l

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-27 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review143749 --- src/main/java/org/apache/aurora/scheduler/configuration/executor/

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-26 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review143664 --- CHANGELOG (lines 1 - 4)

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-26 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review143653 --- Ship it! Master (08792d4) is green with this patch. ./build-s