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

> 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%

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115663 ---

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

> 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:

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

> 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%

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/ --- (Updated Jan. 21, 2016, 12:25 p.m.) Review request for Aurora, John Sirois and

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/#review115684 --- Ship it! IIUC the MVCC capabilities in 1.4+ should prevent table

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

> On Jan. 21, 2016, 1:28 p.m., John Sirois wrote: > > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but > > DDL stuff. > > John Sirois wrote: > Hmm - more to the story: "If MVCC is enabled, changing the lock mode > (LOCK_MODE) has no effect." ... Maybe turning on

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/#review115693 --- Ship it! Ship It! - John Sirois On Jan. 21, 2016, 1:40 p.m.,

Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42614/ --- Review request for Aurora and Bill Farner. Repository: aurora Description

Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/ --- Review request for Aurora, John Sirois and Zameer Manji. Bugs: AURORA-1580

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/#review115688 --- Ship it! Ship It! - Maxim Khutornenko On Jan. 21, 2016, 8:25

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

> On Jan. 21, 2016, 12:28 p.m., John Sirois wrote: > > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but > > DDL stuff. > > John Sirois wrote: > Hmm - more to the story: "If MVCC is enabled, changing the lock mode > (LOCK_MODE) has no effect." ... Maybe turning on

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/ --- (Updated Jan. 21, 2016, 12:40 p.m.) Review request for Aurora, John Sirois and

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

> 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

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

--- 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

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115730 ---

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

--- 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.

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

> 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.

Re: Review Request 42387: working version of jessie builds

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42387/ --- (Updated Jan. 21, 2016, 10:20 p.m.) Review request for Aurora, Benjamin

Re: Review Request 42387: working version of jessie builds

> On Jan. 21, 2016, 1:26 a.m., Bill Farner wrote: > > build-artifact.sh, line 35 > > > > > > How about a selective `set -x`, `set +x` around the commands you wish > > to see echoed instead? Removed. Adding set -x,

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

> On Jan. 21, 2016, 10:09 p.m., Bill Farner wrote: > > Before submitting, please verify that end-to-end tests work with this patch. Verified. - Maxim --- This is an automatically generated e-mail. To reply, visit:

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

--- 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

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

--- 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

Re: Review Request 42387: working version of jessie builds

> On Jan. 20, 2016, 5:26 p.m., Bill Farner wrote: > > specs/debian/aurora-executor.thermos.init, lines 43-44 > > > > > > Is this necessary due to `app_daemonize` or is there more to it? > > Dmitriy Shirchenko wrote:

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

--- 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.

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

> 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

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

--- 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.

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42614/#review115712 --- Ship it! Before submitting, please verify that end-to-end tests

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

> On Jan. 21, 2016, 1:14 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java, > > line 34 > > > > > > The limit of 2 seems arbitrary, and this reflects in

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

> 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%

Re: Review Request 42387: working version of jessie builds

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42387/#review115719 --- Ship it! Ship It! - Bill Farner On Jan. 21, 2016, 2:20 p.m.,

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42614/#review115725 --- Ship it! Master (8d3fb24) is green with this patch.

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42614/#review115697 ---

Re: Review Request 42613: Enable READ COMMITTED transaction isolation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42613/#review115705 --- Ship it! Ship It! - Zameer Manji On Jan. 21, 2016, 12:40 p.m.,

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42332/#review115720 ---

Re: Review Request 42614: Allowing dual authorizing params to account for thrift API deprecations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42614/ --- (Updated Jan. 21, 2016, 10:05 p.m.) Review request for Aurora and Bill Farner.

Re: Review Request 42387: working version of jessie builds

> On Jan. 19, 2016, 4:12 p.m., John Sirois wrote: > > builder/deb/debian-jessie/Dockerfile, line 38 > > > > > > Its not exactly clear to me how this is any better than downloading > > gradle from gradle. Since this

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

--- 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

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

> 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

Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42639/ --- Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

> On Jan. 21, 2016, 7:58 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/GuavaUtils.java, line 63 > > > > > > Would not AbstractIdleService suffice your needs here? > >

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

> On Jan. 21, 2016, 7:52 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 169 > > > > > > Is it safe to call this method from multiple threads?

Re: Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/#review115756 --- Master (a2c7ccc) is red with this patch.

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42639/#review115774 --- Ship it! Master (c89fecb) is green with this patch.

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

> 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

Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/ --- Review request for Aurora, John Sirois and Maxim Khutornenko. Repository:

Re: Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/#review115763 --- Ship it! Master (c89fecb) is green with this patch.

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42639/#review115768 ---

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

> 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.

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

--- 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.,

Re: Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/#review115757 --- @ReviewBot retry - Bill Farner On Jan. 21, 2016, 5:25 p.m.,

Re: Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/#review115754 --- Related discussion, i would like to evaluate removing this

Re: Review Request 42628: Add storage API methods for fetching amd mutating a task by ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42628/#review115780 --- Ship it! Ship It! - John Sirois On Jan. 21, 2016, 6:25 p.m.,

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42639/#review115782 --- Ship it! Ship It! - Zameer Manji On Jan. 21, 2016, 6:47 p.m.,