Re: Review Request 56395: Change Resource Validation in ConfigurationManager so that it validates the Resource Set instead of deprecated fields

2017-02-13 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56395/#review165471 ---

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
> On Feb. 14, 2017, 5:44 a.m., Santhosh Kumar Shanmugham wrote: > > LGTM modulo my pervious comment, > > > > "Since this command has the potential to hurt the Scheduler, should we make > > sure that atleast one of the filters is provided? Or force a minimum > > default limit to avoid

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165460 --- Ship it! Master (40d91fe) is green with this patch.

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165456 --- Ship it! LGTM modulo my pervious comment, "Since this command

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165450 --- Ship it! Master (40d91fe) is green with this patch.

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165448 --- Add entry in RELEASE-NOTES. - Santhosh Kumar Shanmugham On

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165447 ---

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
> On Feb. 13, 2017, 5:54 p.m., Reza Motamedi wrote: > > LGTM! > > > > But, should we test this in the test cluster? At least to check the effect > > of _limit_? > > David McLaughlin wrote: > I did test this in Vagrant. All the options work correctly. > > David McLaughlin wrote: >

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- (Updated Feb. 14, 2017, 5:06 a.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
> On Feb. 13, 2017, 8:48 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > lines 1128-1129 > > > > > > Depending on th query this can be

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165440 ---

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
> On Feb. 14, 2017, 1:54 a.m., Reza Motamedi wrote: > > LGTM! > > > > But, should we test this in the test cluster? At least to check the effect > > of _limit_? > > David McLaughlin wrote: > I did test this in Vagrant. All the options work correctly. Weird, verifying the StateManager

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- (Updated Feb. 14, 2017, 4:28 a.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- (Updated Feb. 14, 2017, 4:27 a.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
> On Feb. 14, 2017, 3:48 a.m., Mehrdad Nurolahzade wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 1135 > > > > > > We are skipping task delete through

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- (Updated Feb. 14, 2017, 4:12 a.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Mehrdad Nurolahzade
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165434 ---

Re: Review Request 53333: Add DSL and E2E changes for per task volume mounts.

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5/#review165431 --- Ship it! Master (40d91fe) is green with this patch.

Re: Review Request 53333: Add DSL and E2E changes for per task volume mounts.

2017-02-13 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5/ --- (Updated Feb. 13, 2017, 6:26 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 53333: Add DSL and E2E changes for per task volume mounts.

2017-02-13 Thread Zameer Manji
> On Oct. 31, 2016, 8:14 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/config/schema/base.py, lines 137-138 > > > > > > I'd say we should mirror the names from mesos.proto here? > > `container_path`

Re: Review Request 53333: Add DSL and E2E changes for per task volume mounts.

2017-02-13 Thread Zameer Manji
> On Nov. 2, 2016, 8:59 a.m., Stephan Erb wrote: > > Ups, I forgot. The following seems to be missing: > > > > * Changelog entry > > * Enduser documentation (at least the configuration reference) Done. - Zameer --- This is an

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165428 --- Master (40d91fe) is red with this patch.

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
> On Feb. 14, 2017, 1:54 a.m., Reza Motamedi wrote: > > LGTM! > > > > But, should we test this in the test cluster? At least to check the effect > > of _limit_? I did test this in Vagrant. All the options work correctly. - David ---

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- (Updated Feb. 14, 2017, 2:01 a.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165425 --- LGTM! But, should we test this in the test cluster? At least to

Re: Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165421 --- Master (40d91fe) is red with this patch.

Review Request 56629: Expose task pruning endpoint in aurora_admin

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/ --- Review request for Aurora, Santhosh Kumar Shanmugham, Stephan Erb, and Zameer

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Mehrdad Nurolahzade
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165419 --- After testing this patch on our test clusters, I can see that

Re: Review Request 56577: Add basic test scripts for RPM and DEB packages

2017-02-13 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56577/#review165397 --- Ship it! LGTM. I always thought we had automated tests, but

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread David McLaughlin
> On Feb. 13, 2017, 8:24 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 188 > > > > > > If the goal is to reduce GC pressure, then what we want

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165388 ---

Re: Review Request 56523: Displaying update id after 'Killed for job update' message for the update that resulted in the task getting killed

2017-02-13 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56523/#review165387 --- This is now on master. - Zameer Manji On Feb. 13, 2017, 12:09

Re: Review Request 56523: Displaying update id after 'Killed for job update' message for the update that resulted in the task getting killed

2017-02-13 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56523/#review165385 --- Ship it! LGTM, thanks for your contribution. - Zameer Manji

Re: Review Request 56523: Displaying update id after 'Killed for job update' message for the update that resulted in the task getting killed

2017-02-13 Thread Abhishek Jain
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56523/ --- (Updated Feb. 13, 2017, 12:09 p.m.) Review request for Aurora, Stephan Erb and

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Santhosh Kumar Shanmugham
> On Feb. 13, 2017, 9:49 a.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 187-188 > > > > > > I am still not convinced this is really needed. For

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Santhosh Kumar Shanmugham
> On Feb. 12, 2017, 2:59 p.m., Mehrdad Nurolahzade wrote: > > To further manage task history pruning heap/workload pressure, we can > > introduce a new configuation parameter in `PruningModule` for specifying > > the max number of tasks to be pruned per round. It can default to -1 > >

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165356 --- Fix it, then Ship it! LGTM in general, +- the sleep below.

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165357 --- Master (ad3377a) is red with this patch.

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Mehrdad Nurolahzade
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/ --- (Updated Feb. 13, 2017, 9:30 a.m.) Review request for Aurora, David

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Mehrdad Nurolahzade
> On Feb. 12, 2017, 11:54 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 174-182 > > > > > > Shouldn't all this be done on the `expiredTasks` rather

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-13 Thread Stephan Erb
> On Feb. 12, 2017, 11:59 p.m., Mehrdad Nurolahzade wrote: > > To further manage task history pruning heap/workload pressure, we can > > introduce a new configuation parameter in `PruningModule` for specifying > > the max number of tasks to be pruned per round. It can default to -1 > >