Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
> On Jan. 21, 2016, 4:20 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 164 > > > > > > I was envisioning put and take, no schedules, no batching. T

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
> On Jan. 21, 2016, 4:20 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 164 > > > > > > I was envisioning put and take, no schedules, no batching. T

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
> On Jan. 21, 2016, 3:50 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 108 > > > > > > Why BlockingQueue and not something like ConcurrentLinke

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115749 --- Ship it! Ship It! - John Sirois On Jan. 21, 2016, 4:43 p.m., Za

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115739 --- Ship it! Ship It! - Maxim Khutornenko On Jan. 21, 2016, 11:43 p

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115738 --- Ship it! Master (a2c7ccc) is green with this patch. ./build-supp

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
> On Jan. 21, 2016, 2:50 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 108 > > > > > > Why BlockingQueue and not something like ConcurrentLinke

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 21, 2016, 3:43 p.m.) Review request for Aurora, John Sirois and M

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
> On Jan. 21, 2016, 3:20 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 164 > > > > > > I was envisioning put and take, no schedules, no batching. T

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Maxim Khutornenko
> On Jan. 21, 2016, 10:50 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 108 > > > > > > Why BlockingQueue and not something like ConcurrentLink

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115730 --- src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrune

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115729 --- Ship it! Master (b2cc604) is green with this patch. ./build-supp

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 21, 2016, 2:55 p.m.) Review request for Aurora, John Sirois and M

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
> On Jan. 21, 2016, 2:50 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 108 > > > > > > Why BlockingQueue and not something like ConcurrentLinke

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115720 --- src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrune

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115721 --- Ship it! Master (8d3fb24) is green with this patch. ./build-supp

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 21, 2016, 2:22 p.m.) Review request for Aurora, John Sirois and M

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Maxim Khutornenko
> On Jan. 21, 2016, 6:51 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 152-161 > > > > > > Am I reading this as a busy loop consuming 100% thr

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Zameer Manji
> On Jan. 21, 2016, 10:51 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 152-161 > > > > > > Am I reading this as a busy loop consuming 100% th

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
> On Jan. 20, 2016, 4:39 p.m., John Sirois wrote: > > Ship It! As Maxim points out below, the current `run` will consume a thread, so ship-it retracted! - John --- This is an automatically generated e-mail. To reply, visit: https://rev

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread John Sirois
> On Jan. 21, 2016, 11:51 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 152-161 > > > > > > Am I reading this as a busy loop consuming 100% th

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115663 --- src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrune

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread John Sirois
> On Jan. 20, 2016, 4:26 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 154 > > > > > > I think this same concept can be cleaned up usefully by: > >

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115524 --- Ship it! Ship It! - John Sirois On Jan. 20, 2016, 3:39 p.m., Za

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread Zameer Manji
> On Jan. 20, 2016, 3:26 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 154 > > > > > > I think this same concept can be cleaned up usefully by: > >

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115517 --- LGTM, but let me know what you think about the suggestion below.

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115513 --- Ship it! Master (8d3fb24) is green with this patch. ./build-supp

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 20, 2016, 2:39 p.m.) Review request for Aurora, John Sirois and M

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 20, 2016, 2:38 p.m.) Review request for Aurora, John Sirois and M

Re: Review Request 42332: Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.

2016-01-20 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/ --- (Updated Jan. 20, 2016, 2:36 p.m.) Review request for Aurora, John Sirois and M