Review Request 42979: Enable ping query to prevent use of invalid pooled connections.

[Zameer Manji]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Bugs: AURORA-1596
https://issues.apache.org/jira/browse/AURORA-1596


Repository: aurora


Description
---

Enable ping query to prevent use of invalid pooled connections.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
8b8c54a116726aa7d3400a54df6439fd368258e7 

Diff: https://reviews.apache.org/r/42979/diff/


Testing
---


Thanks,

Zameer Manji

Review Request 42985: Add a flag to configure H2 LOCK_TIMEOUT.

[Zameer Manji]

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Bugs: AURORA-1596
https://issues.apache.org/jira/browse/AURORA-1596


Repository: aurora


Description
---

Add a flag to configure H2 LOCK_TIMEOUT.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
8b8c54a116726aa7d3400a54df6439fd368258e7 

Diff: https://reviews.apache.org/r/42985/diff/


Testing
---


Thanks,

Zameer Manji

Re: Review Request 42964: Add header to allow bypassing the LeaderRedirectFilter.

[Zameer Manji]

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



I think you also need to add a flag/option to `aurora_admin` to let operators 
take advantage of this flag.


src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java (line 
45)
<https://reviews.apache.org/r/42964/#comment178076>

Since this is a custom header perhaps we should prefix it with `X-`?


- Zameer Manji


On Jan. 29, 2016, 12:54 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42964/
> ---
> 
> (Updated Jan. 29, 2016, 12:54 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1601
> https://issues.apache.org/jira/browse/AURORA-1601
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add header to allow bypassing the LeaderRedirectFilter.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
> 41b99846bff65c4d10d5d74ca8534768b0ce3fd3 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> 21371d951f8c6e44c40f97793d6acd26d3ca5614 
> 
> Diff: https://reviews.apache.org/r/42964/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 42922: Revert "Improving job update query performance."

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 28, 2016, 2:15 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42922/
> ---
> 
> (Updated Jan. 28, 2016, 2:15 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reverts commit fee5943a95c4f08e148dc5f1366486a8c23d5773.
> 
> We discovered a bug when deploying this commit that caused corruption of the 
> update store.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 
> 50044e1c38c712a52855dd96fa2a9de769d6c973 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> 92849d9a82dcd6d5fe6cc452061bcdca31415f40 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  8438f7287b9a95a2cecd72c996b84901cabed4e1 
> 
> Diff: https://reviews.apache.org/r/42922/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 42896: Remove timestamp from task IDs.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 27, 2016, 8:53 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42896/
> ---
> 
> (Updated Jan. 27, 2016, 8:53 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove timestamp from task IDs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
> 35675acfad8f98e833e0dba277ff65943a21db91 
> 
> Diff: https://reviews.apache.org/r/42896/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42882: Improving job update query performance.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 27, 2016, 4:25 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42882/
> ---
> 
> (Updated Jan. 27, 2016, 4:25 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1600
> https://issues.apache.org/jira/browse/AURORA-1600
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The solution is to do sub-selects instead of relying on a large join. For a 
> given update this approach reduced the overall number of rows returned by h2 
> to mybatis from 531441 to just 81.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 
> 0c9dbaee81d27bacee1b6bf51b33b51509e28a63 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> 1d8986ba117f32288510c299ea5634f90a9311e7 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  fba7d4f39dffb0f431e957993b9ae7c03eba2c60 
> 
> Diff: https://reviews.apache.org/r/42882/diff/
> 
> 
> Testing
> ---
> 
> Before this change:
> ```
> Benchmark   (instanceOverrides)   
> Mode  Cnt   ScoreError  Units
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run1  
> thrpt5  39.907 ±  4.694  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run   10  
> thrpt5  24.641 ±  4.187  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run  100  
> thrpt5   0.654 ±  0.080  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run 1000  
> thrpt5   0.007 ±  0.001  ops/s
> ```
> 
> After this change:
> ```
> Benchmark   (instanceOverrides)   
> Mode  Cnt   ScoreError  Units
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run1  
> thrpt5   41.838 ±  3.060  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run   10  
> thrpt5   41.812 ±  2.373  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run  100  
> thrpt5   38.281 ±  2.034  ops/s
> UpdateStoreBenchmarks.JobInstructionsBenchmark.run 1000  
> thrpt5   20.459 ±  2.886  ops/s
> ```
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42845: Enable H2 logging to slf4j.

[Zameer Manji]

> On Jan. 27, 2016, 10:41 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, lines 
> > 118-119
> > <https://reviews.apache.org/r/42845/diff/1/?file=1222489#file1222489line118>
> >
> > Please change this to explain that level 4 indicates intent to use 
> > slf4j, and change the link to
> > http://www.h2database.com/html/features.html#other_logging

Done.


> On Jan. 27, 2016, 10:41 a.m., Bill Farner wrote:
> > src/main/resources/logback.xml, line 21
> > <https://reviews.apache.org/r/42845/diff/1/?file=1222490#file1222490line21>
> >
> > This is really convenient!  For those curious, it generates this 
> > output, essentially echoing the configuration:
> > ```
> > 18:36:36,988 |-INFO in ch.qos.logback.classic.LoggerContext[default] - 
> > Found resource [logback.xml] at 
> > [jar:file:/home/vagrant/aurora/dist/install/aurora-scheduler/lib/aurora-0.12
> > .0-SNAPSHOT.jar!/logback.xml]
> > 18:36:37,074 |-INFO in 
> > ch.qos.logback.core.joran.spi.ConfigurationWatchList@25af5db5 - URL 
> > [jar:file:/home/vagrant/aurora/dist/install/aurora-scheduler/lib/aurora-0.12.0-SNAPSHOT.
> > jar!/logback.xml] is not of type file
> > 18:36:37,356 |-INFO in ch.qos.logback.core.joran.action.AppenderAction 
> > - About to instantiate appender of type 
> > [ch.qos.logback.core.ConsoleAppender]
> > 18:36:37,366 |-INFO in ch.qos.logback.core.joran.action.AppenderAction 
> > - Naming appender as [STDERR]
> > 18:36:37,508 |-INFO in 
> > ch.qos.logback.core.joran.action.NestedComplexPropertyIA - Assuming default 
> > type [ch.qos.logback.classic.encoder.PatternLayoutEncoder] for [encoder] 
> > propert
> > y
> > 18:36:37,665 |-INFO in 
> > ch.qos.logback.classic.joran.action.RootLoggerAction - Setting level of 
> > ROOT logger to INFO
> > 18:36:37,669 |-INFO in 
> > ch.qos.logback.core.joran.action.AppenderRefAction - Attaching appender 
> > named [STDERR] to Logger[ROOT]
> > 18:36:37,672 |-INFO in ch.qos.logback.classic.joran.action.LoggerAction 
> > - Setting level of logger [h2database] to INFO
> > 18:36:37,672 |-INFO in 
> > ch.qos.logback.classic.joran.action.ConfigurationAction - End of 
> > configuration.
> > ```

Normally this output is only there if there is something to warn us about (ie 
more than one logback.xml file), however it is also good at confirming the 
logging configuration. Putting it at the top of log output ensures operators 
can confirm logging is configured as desired.


> On Jan. 27, 2016, 10:41 a.m., Bill Farner wrote:
> > src/main/resources/logback.xml, line 32
> > <https://reviews.apache.org/r/42845/diff/1/?file=1222490#file1222490line32>
> >
> > I believe you can omit this line, as it will inherit the root logger's 
> > appenders.

Done.


- Zameer


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


On Jan. 26, 2016, 9:37 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42845/
> ---
> 
> (Updated Jan. 26, 2016, 9:37 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> On a test cluster with DbTaskStore enabled there are several lines in the log 
> that look like:
> 
> 2016-01-26 13:07:14 jdbc[15]: exception
> 
> There is no other information with these lines. This is a result of setting 
> `TRACE_LEVEL_SYSTEM_OUT` to `1` for H2. This will print out the error message 
> but not the associated throwable: 
> https://github.com/h2database/h2database/blob/a932268843ac84c7a665e427167ff2eb291d6b8e/h2/src/main/org/h2/message/TraceSystem.java#L228
> 
> The SLF4J implementation of tracing in H2 does not suffer from this 
> restriction.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 36c462ac86cc605bd86bfc31c2f962a315099a02 
>   src/main/resources/logback.xml faf0dbc94dea536a944be18810a7f330d4c94dee 
> 
> Diff: https://reviews.apache.org/r/42845/diff/
> 
> 
> Testing
> ---
> 
> Set the level to debug initially and observed extra output in Vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 42845: Enable H2 logging to slf4j.

[Zameer Manji]

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

(Updated Jan. 27, 2016, 1:38 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Bill's feedback.


Repository: aurora


Description
---

On a test cluster with DbTaskStore enabled there are several lines in the log 
that look like:

2016-01-26 13:07:14 jdbc[15]: exception

There is no other information with these lines. This is a result of setting 
`TRACE_LEVEL_SYSTEM_OUT` to `1` for H2. This will print out the error message 
but not the associated throwable: 
https://github.com/h2database/h2database/blob/a932268843ac84c7a665e427167ff2eb291d6b8e/h2/src/main/org/h2/message/TraceSystem.java#L228

The SLF4J implementation of tracing in H2 does not suffer from this restriction.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
36c462ac86cc605bd86bfc31c2f962a315099a02 
  src/main/resources/logback.xml faf0dbc94dea536a944be18810a7f330d4c94dee 

Diff: https://reviews.apache.org/r/42845/diff/


Testing
---

Set the level to debug initially and observed extra output in Vagrant.


Thanks,

Zameer Manji

Re: Review Request 42869: Fix stray printf style log replacement token when logging triggered cron jobs.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 27, 2016, 1:16 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42869/
> ---
> 
> (Updated Jan. 27, 2016, 1:16 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix stray printf style log replacement token when logging triggered cron jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> 7d41a53d01206f06badda16e0149e8ad37dc5a69 
> 
> Diff: https://reviews.apache.org/r/42869/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Review Request 42845: Enable H2 logging to slf4j.

[Zameer Manji]

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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

On a test cluster with DbTaskStore enabled there are several lines in the log 
that look like:

2016-01-26 13:07:14 jdbc[15]: exception

There is no other information with these lines. This is a result of setting 
`TRACE_LEVEL_SYSTEM_OUT` to `1` for H2. This will print out the error message 
but not the associated throwable: 
https://github.com/h2database/h2database/blob/a932268843ac84c7a665e427167ff2eb291d6b8e/h2/src/main/org/h2/message/TraceSystem.java#L228

The SLF4J implementation of tracing in H2 does not suffer from this restriction.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
36c462ac86cc605bd86bfc31c2f962a315099a02 
  src/main/resources/logback.xml faf0dbc94dea536a944be18810a7f330d4c94dee 

Diff: https://reviews.apache.org/r/42845/diff/


Testing
---

Set the level to debug initially and observed extra output in Vagrant.


Thanks,

Zameer Manji

Re: Review Request 42801: `TaskHistoryPruner` controls Lifecycle directly.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 26, 2016, 7 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42801/
> ---
> 
> (Updated Jan. 26, 2016, 7 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1593
> https://issues.apache.org/jira/browse/AURORA-1593
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This was the original idea in https://reviews.apache.org/r/42332.
> 
> Mixing the active scheduler `Service` lifecycle with the `EventBus`
> lifecycle proves tricky - prune events are fired before scheduler active
> services are started.  Instead of queueing up prune events to wait for
> service start or re-engineering service / event bus interaction, returns
> to the orignal behavior, manipulating the `Lifecycle` directly.
> 
> Also kill a confusing unused EventSink discovered during analyis of all
> pub-sub event sourcing that might interact with the `TaskHistoryPruner`.
> 
>  src/main/java/org/apache/aurora/GuavaUtils.java  
> | 18 --
>  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
> |  5 -
>  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> |  1 -
>  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> | 24 +---
>  src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java   
> | 17 +++--
>  src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java
> |  4 
>  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> | 24 
>  7 files changed, 36 insertions(+), 57 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 7d569e000c358e4f27397168843afdc0e9471ad6 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 5ba5e73464336014d5d49885a080dc98bebfeefb 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> efdfbdaa4ea00469faf2c33a8bc12051685f87ca 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 5441630522b3855a3b2036b5ff66fe980728ec68 
>   src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java 
> d91dc277efebf658947382614f77aff15b3a251b 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> ffeee1bebdb403be9179a8160e4d9a01aaf1f56b 
> 
> Diff: https://reviews.apache.org/r/42801/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> It's the latter - e2e (krb part) - that was the only automated testing
> revealing the problem previously.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 42727: Remove the --announcer-enable executor flag.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 25, 2016, 10:03 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42727/
> ---
> 
> (Updated Jan. 25, 2016, 10:03 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the --announcer-enable executor flag.
> 
> 
> Diffs
> -
> 
>   NEWS f2798f6a2d6841606e99a93677067e58dc5cf5cc 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 57fa312ca6c007731fd88ace1bea507320d14700 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 69025dd85011751b1036615af9944a4f7693bb15 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 6214882cd60211b18fa4ce6f43442184b2dccac8 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 12726936ee85d2ee3dba19eb497873d7422886e6 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-thermos-executor.json
>  8d4d90b18451d8a2cc7cbe2d25f64942d0045491 
> 
> Diff: https://reviews.apache.org/r/42727/diff/
> 
> 
> Testing
> ---
> 
> Running tests now.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[Zameer Manji]

> On Jan. 21, 2016, 6:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > <https://reviews.apache.org/r/42565/diff/3/?file=1203749#file1203749line80>
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
> `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
> 
> From
> ```
> mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
> ```
> 
> to
> ```
> mainClassName = 'com.my.package.CustomScheduler'
> ```
> 
> Maxim Khutornenko wrote:
> This means anyone who wants to add custom modules would be forced to fork 
> Aurora codebase instead of a purely pluggable model that we have now? I am 
> not sure I like it. This is error and merge conflict prone.
> 
> Perhaps having a well documented gradle option taking a path to 
> CustomScheduler would be a better middle ground? At least that would not 
> require forking Aurora.
> 
> John Sirois wrote:
> Wait - how do you extend aurora today?  My understanding is you need to 
> muck with start scripts to ammend the classpath.  Under that assumption, I 
> saw mucking with classpath + changing main class name as not an undue burden 
> over and above the status quo.
> 
> John Sirois wrote:
> Basically - if you actually want to claim the label pluggable, you need 
> an out-of-packaging way to ammend the classpath.  IE: an optional file in - 
> say ubuntu - /etc/default/aurora.d/my_plugins.conf - that adds a, say 
> 'my_extensions/*' entry to the claaspath.  The need to write a main could be 
> converted to using a requirement to implement a ServiceLoader interface that 
> would allow the standard scheduler main to scan for service implementations - 
> th

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 22, 2016, 10:47 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 10:47 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42668: Remove most direct uses of deprecated TaskConfig fields.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 22, 2016, 11:23 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42668/
> ---
> 
> (Updated Jan. 22, 2016, 11:23 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> After https://reviews.apache.org/r/42646/, i ventured towards removing 
> TaskConfig fields that have been marked as deprecated:
> 
> TaskConfig.owner.role
> TaskConfig.environment
> TaskConfig.jobName
> 
> Turns out we still cannot remove them, but i resolved to at least prepare a 
> lot of our code for the removal.  The majority of the work here was 
> converting unit tests to use `TaskTestUtil`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 440946bf6cb9dac21b64acdca3678b505ef37aff 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> 37700569c5b8d14dcc7f29ac564cb6546f73ea34 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> ebb4f9a20e382360032bfad3389349ef13117460 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 0a9dfe31b7680d92e8101abfad4e88a324f37a77 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> f0624976259762cc2c86a653e27e9e5ea1bc71f0 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  3b1766d67e5bfcb5b5d1bd1759d251b7f1732e9d 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 2b5a82db77827013f86880d497cde3758276878a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 066c6a3393955ca4dc67e5ddfc6ead9b451b9b4e 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> e1b539136fde48b95ab9f00d31b0e5674e27d37d 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 920e3e594330ed87c69c2cbf65607621478685b4 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java
>  9d21dcd47995d9955c3b4ecfb1a7c1f863022d81 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> 89162d12aee13bd70f340df9ea64eb86e555f8d3 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 7fb6278d75561c2d38fd7a92b168563d20e8c1d9 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 4a655f10b96b91e48b465a10569bbbc7a62c412f 
>   
> src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
>  092df8cdb7901e045682d0f252109a08afc50c01 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> d0a6cd6e48d6d5623b4b7a68f9e6d5336c61bbe7 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
>  fffbc36d034525e012fd19e95e91fd5bae152c5d 
> 
> Diff: https://reviews.apache.org/r/42668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

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

[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
> > <https://reviews.apache.org/r/42332/diff/3/?file=1203998#file1203998line152>
> >
> > Am I reading this as a busy loop consuming 100% thread CPU waiting for 
> > something that may never happen? I don't think this is an acceptable 
> > solution.
> > 
> > Perhaps it's time to refactor task prunner into an 
> > AbstractScheduledService? I always felt task prunner approach of holding on 
> > to task IDs for 2 days just to act once on them isn't very efficient. What 
> > if instead of acting on every particular task ID we have a periodic (say 
> > every 30 seconds) run loop to prune job keys instead?
> > 
> > Implementation-wise, it could be a Set of unique job keys that we 
> > populate on every TaskStateChange event. A runOneIteration() would poll 
> > that set and apply both latency and max_per_job thresholds for all related 
> > terminal tasks within the same iteration.
> > 
> > The only downside for the above is a somewhat increased history count 
> > between the cleanup runs but given that our current thresholds are chosen 
> > mostly arbitrarily I think that should be acceptable.
> 
> John Sirois wrote:
> I think my Future/Queue suggestion above solves the busy loop with no 
> liveness penalty.  That might allow your batching change suggestion to happen 
> in a seperate follow-up RB.

+1 to John here. I think we are overdue for a less complex and heavy pruner but 
I would prefer to keep this RB focused on failure propagation. I am open to a 
follow up ticket and RB. Maxim, if you agree, I can create a ticket that tracks 
the work you just proposed.

Right now, I think I will use the Future/Queue suggestion that John has to 
remove the busy loop.


- Zameer


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


On Jan. 20, 2016, 2:39 p.m., Zameer Manji wrote:
> 
> ---
> 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 Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is key to operating a large cluster and failure to prune should 
> trigger shutdown to prevent unbounded growth of storage. This patch turns 
> `TaskHistoryPruner` into a service which propagates failure from failed 
> pruning attempts towards the `ServiceManager`. Also completing a TODO which 
> removes a test for behaviour that is very awkward to test for.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> e2e tests
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

[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
> > <https://reviews.apache.org/r/42332/diff/4/?file=1204701#file1204701line108>
> >
> > Why BlockingQueue and not something like ConcurrentLinkedQueue? There 
> > is nothing to block here for.
> 
> Zameer Manji wrote:
> `BlockingQueue` has the `drainTo` method which I think makes the 
> iteration nicer.
> 
> Maxim Khutornenko wrote:
> The price for that is excessive locking and lower perf. You can do the 
> same with a faster ConcurrentLinkedQueue and avoid extra collection copying:
> 
> ```
>   FutureTask task;
>   while((task = q.poll()) != null) {
> task.get();
>   }
> ```

Done.


- Zameer


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


On Jan. 21, 2016, 2:55 p.m., Zameer Manji wrote:
> 
> ---
> 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 Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is key to operating a large cluster and failure to prune should 
> trigger shutdown to prevent unbounded growth of storage. This patch turns 
> `TaskHistoryPruner` into a service which propagates failure from failed 
> pruning attempts towards the `ServiceManager`. Also completing a TODO which 
> removes a test for behaviour that is very awkward to test for.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> e2e tests
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

[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 Maxim Khutornenko.


Changes
---

Maxim's feedback.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is key to operating a large cluster and failure to prune should 
trigger shutdown to prevent unbounded growth of storage. This patch turns 
`TaskHistoryPruner` into a service which propagates failure from failed pruning 
attempts towards the `ServiceManager`. Also completing a TODO which removes a 
test for behaviour that is very awkward to test for.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
735199ac1ab343c24471890aa330d6635c26 
  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq
e2e tests


Thanks,

Zameer Manji

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

[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
> > <https://reviews.apache.org/r/42332/diff/3-5/?file=1203998#file1203998line164>
> >
> > I was envisioning put and take, no schedules, no batching. The queue 
> > could even be a SynchronousQueue holding ~0 elements. Any reason not to 
> > simplify in that way?

The documentation for Guava's `AbstractExecutionThreadService` says: "You must 
override the run() method, and it must respond to stop requests.". If we just 
implemented `take`, the thread of the service would block for a long time if 
there was no TaskStateChanges, possibly causing this service to not respond to 
stop requests. I don't know what the implications are of not responding to stop 
requests, but I would like to avoid that case.

I decided that it would be easier to just use `AbstractScheduledService` 
instead and avoid this case.


- Zameer


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


On Jan. 21, 2016, 2:55 p.m., Zameer Manji wrote:
> 
> ---
> 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 Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is key to operating a large cluster and failure to prune should 
> trigger shutdown to prevent unbounded growth of storage. This patch turns 
> `TaskHistoryPruner` into a service which propagates failure from failed 
> pruning attempts towards the `ServiceManager`. Also completing a TODO which 
> removes a test for behaviour that is very awkward to test for.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> e2e tests
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

[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 Maxim Khutornenko.


Changes
---

Removed busy waiting for periodic checking.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is key to operating a large cluster and failure to prune should 
trigger shutdown to prevent unbounded growth of storage. This patch turns 
`TaskHistoryPruner` into a service which propagates failure from failed pruning 
attempts towards the `ServiceManager`. Also completing a TODO which removes a 
test for behaviour that is very awkward to test for.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
735199ac1ab343c24471890aa330d6635c26 
  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq
e2e tests


Thanks,

Zameer Manji

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

[Zameer Manji]

---
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., Bill Farner wrote:
> 
> ---
> 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 Zameer Manji.
> 
> 
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Proposing this as the minimal fix for AURORA-1580.  I think it's wise, 
> however, to look towards the alternative approach i described in that ticket 
> for simplicity and performance gains.
> 
> I'd like to note that after this change, it will be important to treat 
> long-running transactions carefully.  It will be important to avoid 
> transactions block others by writing (locking a table) and subsequently doing 
> other lengthy work.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2b3ee7bf6f7801c140f921b25f78daf6d320098a 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> e689cfeda39e1f065ed29c61a23376512df8f3aa 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 3dba286556ab6ea34b7fdd38508a164c902d9d17 
> 
> Diff: https://reviews.apache.org/r/42613/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

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

[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 Maxim Khutornenko.


Changes
---

Maxim's feedback.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is key to operating a large cluster and failure to prune should 
trigger shutdown to prevent unbounded growth of storage. This patch turns 
`TaskHistoryPruner` into a service which propagates failure from failed pruning 
attempts towards the `ServiceManager`. Also completing a TODO which removes a 
test for behaviour that is very awkward to test for.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
735199ac1ab343c24471890aa330d6635c26 
  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq
e2e tests


Thanks,

Zameer Manji

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

[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
> > <https://reviews.apache.org/r/42332/diff/4/?file=1204701#file1204701line108>
> >
> > Why BlockingQueue and not something like ConcurrentLinkedQueue? There 
> > is nothing to block here for.

`BlockingQueue` has the `drainTo` method which I think makes the iteration 
nicer.


> On Jan. 21, 2016, 2:50 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, 
> > line 140
> > <https://reviews.apache.org/r/42332/diff/4/?file=1204701#file1204701line140>
> >
> > runOneIteration() implies periodicity already, consider simplifying 
> > this comment

Simplified.


> On Jan. 21, 2016, 2:50 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, 
> > line 153
> > <https://reviews.apache.org/r/42332/diff/4/?file=1204701#file1204701line153>
> >
> > Does it have to run every second? This just checks for errors so a 
> > higher limit should suffice (e.g. 5 seconds).

No, it does not have to run every second. Changed it to every 5 seconds.


- Zameer


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


On Jan. 21, 2016, 2:22 p.m., Zameer Manji wrote:
> 
> ---
> 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 Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is key to operating a large cluster and failure to prune should 
> trigger shutdown to prevent unbounded growth of storage. This patch turns 
> `TaskHistoryPruner` into a service which propagates failure from failed 
> pruning attempts towards the `ServiceManager`. Also completing a TODO which 
> removes a test for behaviour that is very awkward to test for.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> e2e tests
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

[Zameer Manji]

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



src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java (line 
146)
<https://reviews.apache.org/r/42639/#comment176831>

Is it safe to call this method from multiple threads? Nothing in the Guava 
documentation indicates this is the case.


- Zameer Manji


On Jan. 21, 2016, 6:47 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42639/
> ---
> 
> (Updated Jan. 21, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This eliminates processing all futures to find the 1st failed one in
> favor of directly signalling a Service failure when a unit of async work
> fails.
> 
>  src/main/java/org/apache/aurora/GuavaUtils.java  | 
> 18 
>  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java | 
> 56 ++
>  src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java   | 
> 17 ++-
>  3 files changed, 40 insertions(+), 51 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 8c2ab5720b808e5eae4c6b93fea8b787839b8dbe 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2d4c58eaa930c561446320a77a559cd926318e8a 
>   src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java 
> 8d19c04a96fccdd9594ecf783fc797c2b76140c7 
> 
> Diff: https://reviews.apache.org/r/42639/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -P build`.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

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

[Zameer Manji]

---
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., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42639/
> ---
> 
> (Updated Jan. 21, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This eliminates processing all futures to find the 1st failed one in
> favor of directly signalling a Service failure when a unit of async work
> fails.
> 
>  src/main/java/org/apache/aurora/GuavaUtils.java  | 
> 18 
>  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java | 
> 56 ++
>  src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java   | 
> 17 ++-
>  3 files changed, 40 insertions(+), 51 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 8c2ab5720b808e5eae4c6b93fea8b787839b8dbe 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2d4c58eaa930c561446320a77a559cd926318e8a 
>   src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java 
> 8d19c04a96fccdd9594ecf783fc797c2b76140c7 
> 
> Diff: https://reviews.apache.org/r/42639/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -P build`.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

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

[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
> > <https://reviews.apache.org/r/42332/diff/3/?file=1203998#file1203998line154>
> >
> > I think this same concept can be cleaned up usefully by:
> > 1. wrap the executor in an `ExecutorCompletionService` up in the 
> > constructor.
> > 2. submit void work to the `CompletionService` 2x in 
> > `registerInactiveTask`
> > 3. in this run loop `take` void futures from it and `get` their void 
> > result - the exceptions will propagate naturally, 1st come, 1st serve, no 
> > trampling.

`ExecutorCompletionService` doesn't allow for submission of delayed tasks and 
line 186 is submitting a task with a delay so I don't think this is possible.


- Zameer


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


On Jan. 20, 2016, 2:39 p.m., Zameer Manji wrote:
> 
> ---
> 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 Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is key to operating a large cluster and failure to prune should 
> trigger shutdown to prevent unbounded growth of storage. This patch turns 
> `TaskHistoryPruner` into a service which propagates failure from failed 
> pruning attempts towards the `ServiceManager`. Also completing a TODO which 
> removes a test for behaviour that is very awkward to test for.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> e2e tests
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

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

[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 Maxim Khutornenko.


Summary (updated)
-

Turn TaskHistoryPruner into a service and trigger shutdown on pruning failure.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is critical to the operation of a large cluster. Failure to prune 
tasks can lead to storage growing in unexpected ways leading to scheduling 
slowdown. This patch adds shutdown on task pruning failure to prevent the 
scheduler from entering an undefined state.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
735199ac1ab343c24471890aa330d6635c26 
  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

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

[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 Maxim Khutornenko.


Changes
---

Remove TODO


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is key to operating a large cluster and failure to prune should 
trigger shutdown to prevent unbounded growth of storage. This patch turns 
`TaskHistoryPruner` into a service which propagates failure from failed pruning 
attempts towards the `ServiceManager`. Also completing a TODO which removes a 
test for behaviour that is very awkward to test for.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
735199ac1ab343c24471890aa330d6635c26 
  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq
e2e tests


Thanks,

Zameer Manji

Re: Review Request 41804: Shim interfaces to preface args system overhaul.

[Zameer Manji]

> On Jan. 19, 2016, 8:34 a.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java, 
> > line 67
> > <https://reviews.apache.org/r/41804/diff/2/?file=1200802#file1200802line67>
> >
> > The defaults aren't applied consistently here, not sometimes at all in 
> > the Params interfaces above.  Its not important to get this right until the 
> > RB that takes away the Arg fields though, so I'm fine with reading this 
> > change as providing some examples of how this will work.  My takeaway , 
> > based on optional options being - hopefully - the norm, is that The Params 
> > methods should either return Optional and have no default implementation or 
> > else have a default implementation.  Non-Optional pure abstract Params 
> > should be viewed with mild suspicion.
> 
> Bill Farner wrote:
> I was sleep-deprived on a plane when writing this, but i believe my 
> rationale was to use `default` methods minimally in this change.  I intended 
> to use them solely in places where tests would otherwise be forced to supply 
> a default (at risk of diverging from the true default).
> 
> You are right, though, all defaults will be applied this way in the 
> follow-up.  The `Optional`/`default` decision will be an interesting one, 
> though, so hold on to that thought!

+1 to the follow up change and I had the same concern about `Optional/default`.


- Zameer


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


On Jan. 19, 2016, 9:49 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> -------
> 
> (Updated Jan. 19, 2016, 9:49 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This begins to define a proposed replacement args API, from the perspective 
> of the code consuming args.  Args will be defined in interfaces, which the 
> eventual arg system will be responsible for implementing on the fly 
> (mechanism TBD).  So while what is seen here is a large net increase in code, 
> the eventual conclusion will be roughly equivalent in terms of lines of code 
> in `Module`s.
> 
> This begins to enact work described here: 
> http://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt 6b71fd233af0d137332bc69249d16e433aa198c7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 45ab76b9331a79699979c6386c93bbc763f64e2e 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> ddc0d0500b8788bc2c9dd67abb62a412465488b4 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> a25fa41f2cc0b1dd8e7915f576cd52bab77a2b21 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> da07df66b06cef6223119854032b4ca1c57a0859 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java 
> 155d702d68367b247dd066f773c662407f0e3b5b 
>   src/main/java/org/apache/aurora/scheduler/http/H2ConsoleModule.java 
> 01d6b5de0079d6f5709c29fe9a72829fbc8501de 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> df649ff4ffdc741afdbc4850c2dbf98bd3e218f1 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> cd5adf9655dc3368dbe06bfee15c65182176be2e 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>  43c38dcffd8e68c018217681cc5a3073d9fb1437 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  0f8bdbbf77dd23c6c370e26a7e1dbc3f1a4ebfe0 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 5daafa9234d20dfcfd9a6cc81508836efe39f1f0 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  7de8f4cebcda51953e00322caec6ad2789

Re: Review Request 41804: Shim interfaces to preface args system overhaul.

[Zameer Manji]

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

Ship it!


Looking forward to the followup change that collapses some of the code by using 
`default` methods.

- Zameer Manji


On Jan. 19, 2016, 9:49 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> ---
> 
> (Updated Jan. 19, 2016, 9:49 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This begins to define a proposed replacement args API, from the perspective 
> of the code consuming args.  Args will be defined in interfaces, which the 
> eventual arg system will be responsible for implementing on the fly 
> (mechanism TBD).  So while what is seen here is a large net increase in code, 
> the eventual conclusion will be roughly equivalent in terms of lines of code 
> in `Module`s.
> 
> This begins to enact work described here: 
> http://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt 6b71fd233af0d137332bc69249d16e433aa198c7 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 45ab76b9331a79699979c6386c93bbc763f64e2e 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> ddc0d0500b8788bc2c9dd67abb62a412465488b4 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> a25fa41f2cc0b1dd8e7915f576cd52bab77a2b21 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> da07df66b06cef6223119854032b4ca1c57a0859 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java 
> 155d702d68367b247dd066f773c662407f0e3b5b 
>   src/main/java/org/apache/aurora/scheduler/http/H2ConsoleModule.java 
> 01d6b5de0079d6f5709c29fe9a72829fbc8501de 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> df649ff4ffdc741afdbc4850c2dbf98bd3e218f1 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> cd5adf9655dc3368dbe06bfee15c65182176be2e 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>  43c38dcffd8e68c018217681cc5a3073d9fb1437 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java
>  0f8bdbbf77dd23c6c370e26a7e1dbc3f1a4ebfe0 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 5daafa9234d20dfcfd9a6cc81508836efe39f1f0 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java
>  7de8f4cebcda51953e00322caec6ad278951f6b1 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 90f8abf830478ad48f9a8a62c1c42423ab0f8d57 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java 
> 23d1c120657d5cb9d294a80c63e8a04512d361ca 
>   src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 
> 735199ac1ab343c24471890aa330d6635c26 
>   
> src/main/java/org/apache/aurora/scheduler/reconciliation/ReconciliationModule.java
>  cccee083fc2e5f204c91a9d397beb451d4d9df40 
>   src/main/java/org/apache/aurora/scheduler/scheduling/SchedulingModule.java 
> 577edcbf362493d577e2f12c876f1dbb9387ad79 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java 
> d569241a59f169eaa9982c3bba7003aa4942f50f 
>   src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java 
> 08eb6d6c722950f6bc75c97ce12380eca16e704d 
>   src/main/java/org/apache/aurora/scheduler/stats/StatsModule.java 
> 4767ef12e6a3c9d7b2d4a2b5be27786518b5b612 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java 
> cded40ba4981e0ae287b6a24e49523f40674bef9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2b3ee7bf6f7801c140f921b25f78daf6d320098a 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> ed63a7471d654dcefd2ff24e2e462974883541f2 
>   
> src/main/java/org/apache/

Re: Review Request 42328: Add metric for counting uncaught exceptions in async executor.

[Zameer Manji]

> On Jan. 15, 2016, 9:42 a.m., Maxim Khutornenko wrote:
> > NEWS, line 26
> > <https://reviews.apache.org/r/42328/diff/2/?file=1197752#file1197752line26>
> >
> > It this really news-worthy? I'd expect it to be a low noise feature 
> > related summary. IMO, adding a stat does not clear that bar.

Noted, I'll remove it from NEWS.


- Zameer


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


On Jan. 14, 2016, 3:58 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42328/
> ---
> 
> (Updated Jan. 14, 2016, 3:58 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add metric "async_executor_uncaught_exceptions" for tracking uncaught 
> exceptions in async executor.
> 
> 
> Diffs
> -
> 
>   NEWS 809077fe6f689e726204519f93fdabb89a4bb5e5 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> 5facc048bc21adb124cb761647553afa9f8ab724 
> 
> Diff: https://reviews.apache.org/r/42328/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 42328: Add metric for counting uncaught exceptions in async executor.

[Zameer Manji]

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

(Updated Jan. 15, 2016, 10:28 a.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Changes
---

Remove NEWS entry.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Add metric "async_executor_uncaught_exceptions" for tracking uncaught 
exceptions in async executor.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
5facc048bc21adb124cb761647553afa9f8ab724 

Diff: https://reviews.apache.org/r/42328/diff/


Testing
---


Thanks,

Zameer Manji

Re: Review Request 42332: Trigger shutdown on task pruning failure.

[Zameer Manji]

> On Jan. 15, 2016, 9:57 a.m., Maxim Khutornenko wrote:
> > History prunner is regsistered as a service. Shouldn't its failure triger a 
> > shutdown according to this: https://reviews.apache.org/r/39631?

`TaskHistoryPruner` is not a service, only `JobUpdateHistoryPruner` is. Even if 
this was a service this error would not shut it down because the exception 
occurs on a separate thread. In this case the exception occurs on the executor 
threads provided by `AsyncModule`. Previously, a failure there would just be 
logged (and I recently added support for a metric). I talked with John about 
this and we agreed that clients of this executor should be responsible for 
determining if an exception in the Runnable is fatal or not.


- Zameer


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


On Jan. 14, 2016, 4:53 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42332/
> ---
> 
> (Updated Jan. 14, 2016, 4:53 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is critical to the operation of a large cluster. Failure to 
> prune tasks can lead to storage growing in unexpected ways leading to 
> scheduling slowdown. This patch adds shutdown on task pruning failure to 
> prevent the scheduler from entering an undefined state.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 42332: Trigger shutdown on task pruning failure.

[Zameer Manji]

> On Jan. 15, 2016, 3:33 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, 
> > line 152
> > <https://reviews.apache.org/r/42332/diff/1/?file=1197802#file1197802line152>
> >
> > You could centralize this handling in deleteTasks - it has all the ids 
> > to give as useful a message as you have here, and that localizes the 
> > otherwise overly broad looking RTE catch.  It'll clean up the temp you had 
> > to introduce below as well.  Further, it'll give you 1 nice spot to add a 
> > comment explaining why termination on 1 failed prune is sane; ie: you can 
> > point out the inevitable history growth issue and that rolling back is 
> > better than waiting for the failure and then rolling back.

The objective is to catch any unexpected exceptions in the Runnable given to 
the executor. For example the exception that triggered this investigation did 
not come from delete tasks but from sorting the set of tasks to delete:

FluentIterable
.from(Tasks.LATEST_ACTIVITY.sortedCopy(inactiveTasks))
.filter(safeToDelete)
.limit(tasksToPrune)
.transform(Tasks::id)
.toSet();


I agree that this approach is not ideal. I'm not sure on how to change this 
without changing the executor to have some sort of ListenableFuture that does 
something `onFailure`.


- Zameer


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


On Jan. 14, 2016, 4:53 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42332/
> ---
> 
> (Updated Jan. 14, 2016, 4:53 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Task pruning is critical to the operation of a large cluster. Failure to 
> prune tasks can lead to storage growing in unexpected ways leading to 
> scheduling slowdown. This patch adds shutdown on task pruning failure to 
> prevent the scheduler from entering an undefined state.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2064089937f5178b1413d386a312f4173a0e35fb 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> 295960f13693c6ba0d7075a8ef7f9680a91ae69d 
> 
> Diff: https://reviews.apache.org/r/42332/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Review Request 42332: Trigger shutdown on task pruning failure.

[Zameer Manji]

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

Review request for Aurora, John Sirois and Maxim Khutornenko.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Task pruning is critical to the operation of a large cluster. Failure to prune 
tasks can lead to storage growing in unexpected ways leading to scheduling 
slowdown. This patch adds shutdown on task pruning failure to prevent the 
scheduler from entering an undefined state.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
2064089937f5178b1413d386a312f4173a0e35fb 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
295960f13693c6ba0d7075a8ef7f9680a91ae69d 

Diff: https://reviews.apache.org/r/42332/diff/


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

Review Request 42328: Add metric for counting uncaught exceptions in async executor.

[Zameer Manji]

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

Review request for Aurora, John Sirois and Maxim Khutornenko.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Add metric "async_executor_uncaught_exceptions" for tracking uncaught 
exceptions in async executor.


Diffs
-

  NEWS 809077fe6f689e726204519f93fdabb89a4bb5e5 
  src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
5facc048bc21adb124cb761647553afa9f8ab724 

Diff: https://reviews.apache.org/r/42328/diff/


Testing
---


Thanks,

Zameer Manji

Re: Review Request 42328: Add metric for counting uncaught exceptions in async executor.

[Zameer Manji]

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

(Updated Jan. 14, 2016, 3:58 p.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Changes
---

John's feedback.


Bugs: AURORA-1582
https://issues.apache.org/jira/browse/AURORA-1582


Repository: aurora


Description
---

Add metric "async_executor_uncaught_exceptions" for tracking uncaught 
exceptions in async executor.


Diffs (updated)
-

  NEWS 809077fe6f689e726204519f93fdabb89a4bb5e5 
  src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
5facc048bc21adb124cb761647553afa9f8ab724 

Diff: https://reviews.apache.org/r/42328/diff/


Testing
---


Thanks,

Zameer Manji

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 14, 2016, 1:10 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42046/
> ---
> 
> (Updated Jan. 14, 2016, 1:10 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1576
> https://issues.apache.org/jira/browse/AURORA-1576
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Allow for plugging in cli-configurable filters that are invoked post shiro 
> filters.
> 
> 
> Diffs
> -
> 
>   NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b 
>   docs/security.md f9b60e9afa3280dc132067c5167f624c34ade66f 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  4b6a872737e5934394e9511af7b6bd96c6034e72 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  ac92117e14c105bec74e49d8e80eb56008237abf 
> 
> Diff: https://reviews.apache.org/r/42046/diff/
> 
> 
> Testing
> ---
> 
> Unit tests:
> ```
> $ ./gradlew clean test
> ...
> BUILD SUCCESSFUL
> 
> Total time: 3 mins 24.616 secs
> ```
> 
> End to end tests:
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> 
> *** OK (All tests passed) ***
> 
> aurora-scheduler-kerberos stop/waiting
> aurora-scheduler start/running, process 15362
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> ```
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

[Zameer Manji]

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


Amol, can you please rebase so I can apply this change?

- Zameer Manji


On Jan. 14, 2016, 1:10 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42046/
> ---
> 
> (Updated Jan. 14, 2016, 1:10 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1576
> https://issues.apache.org/jira/browse/AURORA-1576
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Allow for plugging in cli-configurable filters that are invoked post shiro 
> filters.
> 
> 
> Diffs
> -
> 
>   NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b 
>   docs/security.md f9b60e9afa3280dc132067c5167f624c34ade66f 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  4b6a872737e5934394e9511af7b6bd96c6034e72 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  ac92117e14c105bec74e49d8e80eb56008237abf 
> 
> Diff: https://reviews.apache.org/r/42046/diff/
> 
> 
> Testing
> ---
> 
> Unit tests:
> ```
> $ ./gradlew clean test
> ...
> BUILD SUCCESSFUL
> 
> Total time: 3 mins 24.616 secs
> ```
> 
> End to end tests:
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> 
> *** OK (All tests passed) ***
> 
> aurora-scheduler-kerberos stop/waiting
> aurora-scheduler start/running, process 15362
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> ```
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Review Request 42225: Add `--show-error` to curl when bootstrapping thrift.

[Zameer Manji]

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

Review request for Aurora and John Sirois.


Repository: aurora


Description
---

>From the curl documentation:

-S, --show-error

When used with -s it makes curl show an error message if it fails.


It's possible for curl to fail when grabbing the tarball or patch and this will
show users why it failed.


Diffs
-

  build-support/thrift/Makefile 8b1da2a01b9ecc1832d4e56bfaacb5b40cdbbab0 

Diff: https://reviews.apache.org/r/42225/diff/


Testing
---

Ran `make` in the `build-support/thrift` directory.


Thanks,

Zameer Manji

Re: Review Request 42145: Use tags instead of branches for release candidates.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 11, 2016, 7:22 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42145/
> ---
> 
> (Updated Jan. 11, 2016, 7:22 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Follow-up from https://reviews.apache.org/r/42117 for release candidate script
> 
> 
> Diffs
> -
> 
>   build-support/release/release-candidate 
> 114bb6d284647ad9745679fa3d13684ffd45ac31 
> 
> Diff: https://reviews.apache.org/r/42145/diff/
> 
> 
> Testing
> ---
> 
> Ran `./build-support/release/release-candidate` in non-publish mode, verified 
> files and e-mail boilerplate generated.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42117: Change release script to use rel/ tag prefix.

[Zameer Manji]

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

Ship it!


Will we also retroactively push `rel/` tags for previous releases?

- Zameer Manji


On Jan. 10, 2016, 10:26 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42117/
> ---
> 
> (Updated Jan. 10, 2016, 10:26 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is in response to the e-mail below regarding git server-side policy 
> enforcement rendering `rel/` tags immutable.
> 
> ```
> Greeting PMCs:
> (bcc to p...@apache.org)
> 
> Following direction from the Board, Infrastructure has modified git to
> permit force pushes, and branch/tag deletion.
> 
> In accordance with the guidance that the Board we've implemented a few
> changes you should be aware of:
> 
> First, If a forced commit is pushed, the subsequent commit email will
> contain '[Forced Update!]' in the subject line. The hope here is that
> it draws extra attention to the situation for a project community to
> be aware, and take appropriate action if needed.
> 
> Second, we've changed the 'protected' portions of git to primarily
> focus on refs/tags/rel - thus any tags under rel, will have their
> entire commit history. This provides the provenance that the ASF needs
> for releases, while still giving projects the ability to mold their
> repository in the way they see fit.
> 
> Thus when a release vote is successful - part of the release process
> should become tagging the voted upon commit SHA under rel/ to make it
> indelible. ('# git tag rel/v15.4.2 ' or something similar.)
> 
> 
> If you have questions, please feel free to email infrastruct...@apache.org
> 
> 
> --David
> on behalf of Apache Infrastructure
> ```
> 
> 
> Diffs
> -
> 
>   build-support/release/release 9e8dd41209b8d687974d1cdc731c945afab3946e 
> 
> Diff: https://reviews.apache.org/r/42117/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 42078: Bump JMH to 1.11.2.

[Zameer Manji]

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

Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

Bump JMH to the latest available release which is 1.11.2. There isn't a 
CHANGELOG but the commit history shows several bug fixes: 
http://hg.openjdk.java.net/code-tools/jmh/


Diffs
-

  build.gradle 6d7af5e020afa58040e616caa1911ac69955f0bf 

Diff: https://reviews.apache.org/r/42078/diff/


Testing
---

./gradlew jmh -Pbenchmarks='UpdateStoreBenchmarks.*'


Thanks,

Zameer Manji

Re: Review Request 42080: Upgrade to pants 0.0.67.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 8, 2016, 11:39 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42080/
> ---
> 
> (Updated Jan. 8, 2016, 11:39 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The CHANGELOG can be read here:
>   http://pantsbuild.github.io/changelog.html
> 
> Of note for aurora is an upgrade to pex 1.1.2 which
> improves artifact resolution times.
> 
>  3rdparty/python/requirements.txt | 2 +-
>  pants.ini| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 8c728800410482234afb5031f04fd25d0be2afea 
>   pants.ini 38932348195ad2021a279d2dd64ae94204e91849 
> 
> Diff: https://reviews.apache.org/r/42080/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./build-support/jenkins/build.sh`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 42041: Enable H2 query statistics collection.

[Zameer Manji]

> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > <https://reviews.apache.org/r/42041/diff/1/?file=1186513#file1186513line126>
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.
> 
> Zameer Manji wrote:
> John, using `SET QUERY_STATISTICS` will not persist across failovers and 
> is not retroactive. This means operators need to run `SET QUERY_STATISTICS` 
> and wait for data to be collected if they notice the scheduler is slower than 
> expected. If this option is set when the database is created the data 
> collected should reflect all operations done by the scheduler and that is 
> more useful for triaging slow scheduling in large clusters.
> 
> I think providing a flag or enabling it by default (pending results of 
> benchmarks) are better solutions.
> 
> John Sirois wrote:
> Makes sense.  Have you thought about the existing slow query logging and 
> how that evolves with this?  Probably fine side-by-side, but maybe confusing 
> and less valuable once the mem store is gone and the db store is all we have.

I have updated the testing done with benchmarks and the change appears to be 
small for scheduling. I think the utility of query statistics justifies turning 
it on by default.

John, I think slow query logging can be removed once the mem store has been 
removed.


- Zameer


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


On Jan. 8, 2016, 1:52 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 8, 2016, 1:52 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> Benchmarks
> 
> Master (c595228):
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
>   N/A  thrpt   10  64138.084 ± 6732.130  ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
>   N/A  thrpt   10  23863.861 ± 2101.622  ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10   2228.883 ±  311.434  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
> 1  thrpt   10 50.914 ±2.488  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>10  thrpt   10 43.729 ±3.038  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>   100  thrpt   10 44.409 ±4.426  ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
>  1000  thrpt   10 40.429 ±7.526  ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark  
>   N/A  thrpt   10  22942.538 ± 1281.331  ops/s
> 
> 
> This change:
> Benchmark 
> (numPendingTasks)   Mode  Cnt  Score  Error  Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 
&

Re: Review Request 42041: Enable H2 query statistics collection.

[Zameer Manji]

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

(Updated Jan. 8, 2016, 1:52 p.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Repository: aurora


Description
---

With this enabled operators can visit the H2 console at /h2console and run 
queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
e3efbdb80d8b18312bf1589eef883cdeee65b225 

Diff: https://reviews.apache.org/r/42041/diff/


Testing (updated)
---

Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.

Benchmarks

Master (c595228):
Benchmark 
(numPendingTasks)   Mode  Cnt  Score  Error  Units
SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark   
N/A  thrpt   10  64138.084 ± 6732.130  ops/s
SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark  
N/A  thrpt   10  23863.861 ± 2101.622  ops/s
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10   2228.883 ±  311.434  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
  1  thrpt   10 50.914 ±2.488  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
 10  thrpt   10 43.729 ±3.038  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
100  thrpt   10 44.409 ±4.426  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
   1000  thrpt   10 40.429 ±7.526  ops/s
SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10  22942.538 ± 1281.331  ops/s


This change:
Benchmark 
(numPendingTasks)   Mode  Cnt  Score  Error  Units
SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark   
N/A  thrpt   10  65285.628 ± 2422.816  ops/s
SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark  
N/A  thrpt   10  24573.332 ± 1332.474  ops/s
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10   2430.402 ±  258.860  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
  1  thrpt   10 43.810 ±2.669  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
 10  thrpt   10 37.378 ±   14.637  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
100  thrpt   10 40.180 ±9.738  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
   1000  thrpt   10 24.130 ±   15.746  ops/s
SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
N/A  thrpt   10  18429.830 ± 3077.426  ops/s


Thanks,

Zameer Manji

Re: Review Request 42041: Enable H2 query statistics collection.

[Zameer Manji]

> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > <https://reviews.apache.org/r/42041/diff/1/?file=1186513#file1186513line126>
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?
> 
> Zameer Manji wrote:
> Good question, I'll run benchmarks and update the testing done.
> 
> John Sirois wrote:
> And if there are perf implications, there is always [`SET 
> QUERY_STATISTICS`](http://www.h2database.com/html/grammar.html?highlight=QUERY_STATISTICS=QUERY_STATISTICS#set_query_statistics)
>  via the /h2console - and maybe that could just be documented as an ad-hoc 
> option.

John, using `SET QUERY_STATISTICS` will not persist across failovers and is not 
retroactive. This means operators need to run `SET QUERY_STATISTICS` and wait 
for data to be collected if they notice the scheduler is slower than expected. 
If this option is set when the database is created the data collected should 
reflect all operations done by the scheduler and that is more useful for 
triaging slow scheduling in large clusters.

I think providing a flag or enabling it by default (pending results of 
benchmarks) are better solutions.


- Zameer


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


On Jan. 7, 2016, 3:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 3:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 42041: Enable H2 query statistics collection.

[Zameer Manji]

> On Jan. 7, 2016, 3:21 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 126
> > <https://reviews.apache.org/r/42041/diff/1/?file=1186513#file1186513line126>
> >
> > Are there any perf implications from having it ON by default? Should it 
> > be a configurable flag instead?

Good question, I'll run benchmarks and update the testing done.


- Zameer


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


On Jan. 7, 2016, 3:14 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42041/
> ---
> 
> (Updated Jan. 7, 2016, 3:14 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> With this enabled operators can visit the H2 console at /h2console and run 
> queries like `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` to diagnose slow schedulers.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> e3efbdb80d8b18312bf1589eef883cdeee65b225 
> 
> Diff: https://reviews.apache.org/r/42041/diff/
> 
> 
> Testing
> ---
> 
> Ran `SELECT * FROM INFORMATION_SCHEMA.QUERY_STATISTICS ORDER BY 
> MAX_EXECUTION_TIME DESC;` within vagrant and saw query statistics.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 42034: Adding gpg key for jsir...@apache.org.

[Zameer Manji]

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

Ship it!


Don't forget to put it in the `dev/aurora` and `release/aurora` directories on 
the dist svn.

- Zameer Manji


On Jan. 7, 2016, 12:39 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42034/
> ---
> 
> (Updated Jan. 7, 2016, 12:39 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding gpg key for jsir...@apache.org.
> 
> 
> Diffs
> -
> 
>   KEYS aed96ddf716a394a6517292a292dd38bb2460d30 
> 
> Diff: https://reviews.apache.org/r/42034/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41804: Proposal - shim interfaces to preface args system overhaul.

[Zameer Manji]

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


+1 to the proposed approach. I think the idea of encapsulating all arguments to 
a module under an interface/POJO that can be injected is good organization.
I think we can go forward with steps #1 and #2 (in two different reviews) 
immediately and start a discussion for #3 should be like on the dev@ list.

- Zameer Manji


On Dec. 30, 2015, 4:03 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> ---
> 
> (Updated Dec. 30, 2015, 4:03 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This begins to define a proposed replacement args API, from the perspective 
> of the code consuming args.  Args will be defined in interfaces, which the 
> eventual arg system will be responsible for implementing on the fly 
> (mechanism TBD).  So while what is seen here is a large net increase in code, 
> the eventual conclusion will be roughly equivalent in terms of lines of code 
> in `Module`s.
> 
> There are a few goals with the replacement:
> - sidestep current development hurdles we have encountered with the args 
> system (intellij/gradle not working nicely with apt)
> - leverage a well-maintained third-party argument parsing library
> - encourage better testability of Module classes by always injecting all args
> - enable user-friendly features like logical option groups for better 
> help/usage output
> - stretch: enable alternative configuration inputs like a configuration file 
> or environment variables
> 
> The rough plan of action is as follows (if the proposal looks good):
> 1. repeat this patch for all other `@CmdLine` declaration sites (28 files) 
> 2. introduce a 'boot' `Injector` that is loaded with bindings for these 
> params implementations (i.e. centralize the `new ExecutorModuleParams() { .. 
> }` boilerplate you see here
> 3. replace the args backend
>   (a) remove `@CmdLine Arg<>` declarations, moving help text to annotations 
> on interface methods
>   (b) implement a system to inject proxies that implement params classes 
> based on arg values, and binds them for injection
> 
> Given that this is a large multi-stage effort, i may opt to implement it on a 
> branch and land it all at once in a stream of commits to avoid 
> churn/confusion in the meantime.  Open to thoughts on that.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
> 
> Diff: https://reviews.apache.org/r/41804/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41804: Proposal - shim interfaces to preface args system overhaul.

[Zameer Manji]

> On Dec. 30, 2015, 4:14 p.m., John Sirois wrote:
> > I'd be happy with these commits landing as they come, but I'll recommend 
> > that be paired with a boilerplate comment referencing a tracking jira issue 
> > that explains the odd interfaces appearing during the transition.

+1 to landing commits as they come. I think it is preferable to landing several 
commits all at once at the end.


- Zameer


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


On Dec. 30, 2015, 4:03 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> ---
> 
> (Updated Dec. 30, 2015, 4:03 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This begins to define a proposed replacement args API, from the perspective 
> of the code consuming args.  Args will be defined in interfaces, which the 
> eventual arg system will be responsible for implementing on the fly 
> (mechanism TBD).  So while what is seen here is a large net increase in code, 
> the eventual conclusion will be roughly equivalent in terms of lines of code 
> in `Module`s.
> 
> There are a few goals with the replacement:
> - sidestep current development hurdles we have encountered with the args 
> system (intellij/gradle not working nicely with apt)
> - leverage a well-maintained third-party argument parsing library
> - encourage better testability of Module classes by always injecting all args
> - enable user-friendly features like logical option groups for better 
> help/usage output
> - stretch: enable alternative configuration inputs like a configuration file 
> or environment variables
> 
> The rough plan of action is as follows (if the proposal looks good):
> 1. repeat this patch for all other `@CmdLine` declaration sites (28 files) 
> 2. introduce a 'boot' `Injector` that is loaded with bindings for these 
> params implementations (i.e. centralize the `new ExecutorModuleParams() { .. 
> }` boilerplate you see here
> 3. replace the args backend
>   (a) remove `@CmdLine Arg<>` declarations, moving help text to annotations 
> on interface methods
>   (b) implement a system to inject proxies that implement params classes 
> based on arg values, and binds them for injection
> 
> Given that this is a large multi-stage effort, i may opt to implement it on a 
> branch and land it all at once in a stream of commits to avoid 
> churn/confusion in the meantime.  Open to thoughts on that.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
> 
> Diff: https://reviews.apache.org/r/41804/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41785: Remove scheduler log scaffolding

[Zameer Manji]

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

Ship it!


Is it possible to get thread id instead of thread name by default? I think that 
is more valuable when debugging issues and will reudce the size of each log 
line.

- Zameer Manji


On Jan. 3, 2016, 9:04 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41785/
> ---
> 
> (Updated Jan. 3, 2016, 9:04 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Using log4j as a straw man.  If we decide to choose logback instead, i'm 
> happy to update this patch.
> 
> Here's an example of the log format:
> ```
> I1229 22:16:54.568 [pool-10-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> As opposed to what's on master:
> ```
> I1230 06:26:14.987 THREAD143 
> org.apache.aurora.scheduler.app.local.FakeMaster.lambda$start$0: All offers 
> consumed, suppressing offer cycle.
> ```
> 
> I could more closely match the existing format, but i think the change is an 
> improvement.
> 
> TODO before submitting: Update `LogConfig` to align with the selected 
> backend.  I'll wait until we decide on a backend before i update this.
> 
> 
> Diffs
> -
> 
>   NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 
>   build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 
>   commons/src/main/java/org/apache/aurora/common/logging/Glog.java 
> 5bae399cd9360a0093c67c608cf68b013a709194 
>   commons/src/main/java/org/apache/aurora/common/logging/LogFormatter.java 
> 0cb621da2b2f286d9eda9eb18ecac087208d7b6b 
>   commons/src/main/java/org/apache/aurora/common/logging/RootLogConfig.java 
> 26dd0aa5682faf31ba4cc086d32bf61557ef1fde 
>   
> commons/src/main/java/org/apache/aurora/common/logging/log4j/GlogLayout.java 
> 1a90ded72c81a7e3fdc9dab515a2622c55563e6a 
>   
> commons/src/test/java/org/apache/aurora/common/logging/LogFormatterTest.java 
> 9f041915c7fd4513a6255b05b3d0096779b5f1b3 
>   
> commons/src/test/java/org/apache/aurora/common/logging/RootLogConfigTest.java 
> 9d55a1aaf555d6e25ad97622612fad61271d0e25 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 4f43892723db4744db205ea7dd107e9e9ce9d5db 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 4033184451f36cb5f0233ea96e3dceaae6741275 
>   src/main/java/org/apache/aurora/scheduler/app/Log4jConfigurator.java 
> 348ff1345a3d5ff4212a2cb211e973ad9e2ca2e8 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> de018dd3cd82b7a0a1cb285f8f3172dae529817f 
>   src/main/resources/log4j2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41785/diff/
> 
> 
> Testing
> ---
> 
> end-to-end tests are green
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41777: Use slf4j throughout the scheduler.

[Zameer Manji]

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

Ship it!


- Zameer Manji


On Dec. 29, 2015, 4:06 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41777/
> ---
> 
> (Updated Dec. 29, 2015, 4:06 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> slf4j allows us to isolate the decision and configuration of our logging 
> backend.
> 
> Apologies for the monster patch.  Most of this is pretty mechanical (and was 
> automated), however.
> 
> 
> Diffs
> -
> 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  3ff3474dc3c8536758a0a0b3e05587daa7cde3eb 
>   commons/src/main/java/org/apache/aurora/common/application/Lifecycle.java 
> a71a51d2fa9465926df5b1232f450767df150847 
>   
> commons/src/main/java/org/apache/aurora/common/application/ShutdownRegistry.java
>  b440e7e6a2b26533d0ce52bf0ffb21735ef6352f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> c98ed8750cf8306593587f28bf8f3e76a01b6254 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> db8e1694a81a5791d13f8d5c6cfa5f489209e82f 
>   commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java 
> ddffdf9c8d41166fee49866a3b7fbc2efd6bbe54 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/ListParser.java 
> 676f88be9a8f9d3f743ee9e62f133470cb84df6b 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/SetParser.java 
> 75031f49c7a509f55edc0f7d624dfcc7a0bd2930 
>   commons/src/main/java/org/apache/aurora/common/collections/Iterables2.java 
> 7ae80ce7193717fd82dc59a346d07cba0d10e4cb 
>   commons/src/main/java/org/apache/aurora/common/inject/TimedInterceptor.java 
> 684e7bb885fcd142e926b4e68c25323e465624fb 
>   
> commons/src/main/java/org/apache/aurora/common/net/InetSocketAddressHelper.java
>  b3719d8f71679f7165631fd6cd4f18a93d5f871a 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/AbortHandler.java
>  42e668d09e52bdeb50a05ee7000b7f51e3160104 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/HttpServletRequestParams.java
>  c09be674d65010dfbc73fe09d8051558d236001c 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java
>  c3fd80863e66f50023647c25e47e4406e89a7fa9 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/QuitHandler.java
>  40872e2778b9f3cb05e58be22a1ad4e31b3e1b3e 
>   commons/src/main/java/org/apache/aurora/common/stats/JvmStats.java 
> d1b072d14556be00bf2b6e06019b696c7c3235e3 
>   commons/src/main/java/org/apache/aurora/common/stats/Percentile.java 
> d8046b635b1ac5d816457430fb75ea17a6f46fde 
>   commons/src/main/java/org/apache/aurora/common/stats/Rate.java 
> dbb90c45fed4ba2c7b97c6e14646345f6fc430f7 
>   commons/src/main/java/org/apache/aurora/common/stats/RequestStats.java 
> 5467810e3ac546b007ae8436ffe8c8dce9231eef 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingStats.java 
> 334167275bdbd4f494b8e89c3b2575c21946a29e 
>   commons/src/main/java/org/apache/aurora/common/stats/Stats.java 
> d4b6a3133c5ece40a398d8cfe605627d2344f0a8 
>   commons/src/main/java/org/apache/aurora/common/stats/TimeSeries.java 
> 45f604c76ea255de3c805a6a48406bcd3e587838 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  6b237fbf7d308150c8dd12f7e25e2bd6aca900ab 
>   commons/src/main/java/org/apache/aurora/common/util/BackoffHelper.java 
> b251e9b84aa33c7c1e1366aaf8872a367864e408 
>   commons/src/main/java/org/apache/aurora/common/util/BuildInfo.java 
> 4f9c3847bc64c80e64bd3c04ea8df3ae44716947 
>   commons/src/main/java/org/apache/aurora/common/util/StateMachine.java 
> dbaaadde9aa821e2293656bd48afc47816e900a2 
>   
> commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java
>  fd74b9f37c6cc24c7ea1cb239ba6354661d931e2 
>   
> commons/src/main/java/org/apache/aurora/common/util/templating/StringTemplateHelper.java
>  2756af49c7702ac1343712476e138fd4367aa756 
>   commons/src/main/java/org/apache/aurora/common/util/testing/FakeClock.java 
> 2ed8b15686e0bfc6310c0427e3501e9f6d5d8af7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 
> f679d92e6ede869e5b374c80b261ba7e83858152 
>   commons/src/main

Re: Review Request 41786: Remove several scheduler command line arguments.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 29, 2015, 5:52 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41786/
> ---
> 
> (Updated Dec. 29, 2015, 5:52 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Included some justification in `NEWS`.  I can't recall why 
> `-enable_job_updates` and `-enable_job_creation` were introduced, but they 
> seem unnecessary (i added them 3.5 years ago without much detail: 
> https://github.com/apache/aurora/commit/91b1cb8).
> 
> 
> Diffs
> -
> 
>   NEWS 394b31cf1c863c54b89e7f27ab3694bdd51b2eeb 
>   docs/scheduler-storage.md 1cd02f87d3062d76942cb9ec755084adc64d0aec 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> da6894e242c3aca2c34097971fac71c7caea4117 
>   src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java 
> 548b5e7538c8d4914476b87cd123f0f22899ec2b 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
> 40995033472bbc404a6779fd0193f037831c212e 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 561c70b8c6b781a9ca6adc19328a691cdb2a009f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
> b252468c99a1e2f4b92c4a1d4ce02330d3f7b80c 
>   src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
> 7f29b799117b2de27f8201f62d3b199e01b36c5b 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
>  2492796be8efd60df8d47510de5a7ff0d1f42656 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
> 692ace0cdd0f44ee5b6a36d4ff9b4f4ede50ed5b 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 216f92f98786b639575888aeea9e3c067a94370b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
>  9abdbbba2eca744fc95859d5d384f91fb8dfb8ca 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> 19b74159c37151d800c889b68394af159c0c3ee5 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java
>  0edb3151760ef5423c154628ba10f2a051b72a8b 
> 
> Diff: https://reviews.apache.org/r/41786/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41711: Move all command line argument declarations to modules or SchedulerMain.

[Zameer Manji]

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

Ship it!


LGTM. Removing the access of global state is good as it improves our testing.


src/main/java/org/apache/aurora/scheduler/thrift/Thresholds.java (line 19)
<https://reviews.apache.org/r/41711/#comment172293>

Please add a `toString` method to this class.


- Zameer Manji


On Dec. 24, 2015, 2:43 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41711/
> ---
> 
> (Updated Dec. 24, 2015, 2:43 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor to make all command line argument values injectable.  This 
> is the first of several steps that would prepare us for changing command line 
> arg libraries, but is also better practice for testability.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/logging/RootLogConfig.java 
> 7f010fd7dbd491d8bc4c9664c1edf7fdd7661497 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 967e10d2b6469ed985308594296f1b2f71f034f1 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> e8bf6bc87cbab3f7496b4e3eadb21808ec0bf1b4 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0ffec91ed29f17ca826cf54eff6075da8d371d50 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
>   src/main/java/org/apache/aurora/scheduler/base/UserProvidedStrings.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  3a2056a858eff5cc692b37c924dda48230caf006 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
>  05981b9318a6c25703994436e8e91fdbf9522d77 
>   src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java 
> dd8c9cdaebc6bebe0e029b47ae3aa670d2af5cbb 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> f355bc101252fb433c7437a791e6e92f94462fa6 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> 527197c3b7a37c1c87fbd4da5493d053fc9d48d7 
>   
> src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
> f964853c168abbb2852c6ed0cc6293ec5ccab8b9 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 01448ae1d2b1c2cddd1284d0cbc8aafa5fb6f397 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 7235902b4728e055545dedcefeddd907648d5895 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  fd5e2f25802697b7da0db472bc375a9e04f07ac3 
>   src/main/java/org/apache/aurora/scheduler/thrift/Thresholds.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  c6a4ac58479402dd605c8db6de42065f376c7657 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> 5dd4aba92b2627b646087fce8118d5ebfeb75f49 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 17d12c3aa82c02991259424b1b109957e4350702 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  716e0a10ad99213bf496215fde5267f655304a22 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 13cb73d0e314c043aed9ab2aacea1908c069a297 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  011b79aa02b0d00424a2d4b7ab1c22adaaff0360 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  129851cb4a3d5e2bb82f2aba45aa8b1a7206c731 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 72b5c30e5e5ceb81eda090d5d2b8d5b213b04dc3 
> 
> Diff: https://reviews.apache.org/r/41711/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41755: Upgrade to Gradle 2.10 to pick up perf wins.

[Zameer Manji]

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

Ship it!


Not sure if you saw but setting classpath to null was the suggested work around 
on my post on the gradle forum.

- Zameer Manji


On Dec. 28, 2015, 3:15 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41755/
> ---
> 
> (Updated Dec. 28, 2015, 3:15 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1566
> https://issues.apache.org/jira/browse/AURORA-1566
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Release notes are here:
>   https://docs.gradle.org/2.10/release-notes
>   https://docs.gradle.org/2.9/release-notes
>   https://docs.gradle.org/2.8/release-notes
> 
> No-op compiles are 20% faster in local testing for example.
> 
> One hack was needed for pmd and a comment was added in `build.gradle`
> calling the shot for future removal.
> 
>  build.gradle | 15 +++
>  buildSrc/gradle.properties   |  2 +-
>  gradle/wrapper/gradle-wrapper.properties |  2 +-
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle 6d9d2d4f502c0115aedbddeb46de6b19398b1e89 
>   buildSrc/gradle.properties c7d38905b8fd5e9ec9ef81732f5b350fd56a998b 
>   gradle/wrapper/gradle-wrapper.properties 
> 7eedd255dcda47350c645701ed2fc25d6b53ab96 
> 
> Diff: https://reviews.apache.org/r/41755/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41537: Upgrade mesos dependency to 0.25.0

[Zameer Manji]

> On Dec. 26, 2015, 4:45 p.m., Stephan Erb wrote:
> > Exception message: Could not satisfy all requirements for 
> > mesos.native==0.25.0:
> > mesos.native==0.25.0
> > 
> > The end-to-end tests are failing to me. Is that local issue or do we need 
> > to upload that egg somewhere?
> 
> Bill Farner wrote:
> Is it reproducible for you?  I encountered that once but could not 
> reproduce.  The egg is here: 
> https://svn.apache.org/repos/asf/aurora/3rdparty/ubuntu/trusty64/python/

I also cannot reproduce. I think it is a pants issue, you may need to delete 
.pants.d/ and try again.


- Zameer


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


On Dec. 23, 2015, 12:10 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41537/
> ---
> 
> (Updated Dec. 23, 2015, 12:10 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1565
> https://issues.apache.org/jira/browse/AURORA-1565
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Upgrade mesos dependency to 0.25.0
> 
> 0.24->0.25 upgrade notes don't call out relevant changes on our end
> http://mesos.apache.org/documentation/latest/upgrades/
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 4301a835b538839f564769bc89cd9bd392d353d6 
>   NEWS ebfc3a661d0550eb320046ca911bddf7385100cd 
>   build.gradle 3bd4cfd3e9432fe390728cc6e605e05c69e874d5 
>   docs/deploying-aurora-scheduler.md 73b2e0460b8e79ab02bd952e653e5d08613b0360 
>   examples/vagrant/provision-dev-cluster.sh 
> 3615dc50582308900faa8d8fb03c55de1df7b985 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 2b2457879ae71d2904532ff30070540c7d699523 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> e55175590c48b9b628b7044626cdb62e6993 
> 
> Diff: https://reviews.apache.org/r/41537/diff/
> 
> 
> Testing
> ---
> 
> Verified in end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41591: Remove the deprecated ConfigGroup.instanceIds field.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 20, 2015, 9:25 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41591/
> ---
> 
> (Updated Dec. 20, 2015, 9:25 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1519
> https://issues.apache.org/jira/browse/AURORA-1519
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the deprecated ConfigGroup.instanceIds field.
> 
> 
> Diffs
> -
> 
>   NEWS 79d8668f2025de1e9e4821c103eec59b0816b7c2 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> bdfadbb52a7949fd6b73d956752f53639a4413c5 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 9f0cf8618d848824dbd00cbfc7544050bc3d5c08 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  77d5c1dbd28d6d6fe117bac5d831436d3c2a0655 
> 
> Diff: https://reviews.apache.org/r/41591/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs

[Zameer Manji]

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


I won't be able to give a review in a reasonable period of time. Please swap me 
out for Joshua.

- Zameer Manji


On Dec. 15, 2015, 11:15 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41428/
> ---
> 
> (Updated Dec. 15, 2015, 11:15 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1562
> https://issues.apache.org/jira/browse/AURORA-1562
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Refactoring HealthCheckConfig into separate structs
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f 
>   src/main/python/apache/aurora/client/config.py 
> 161c3627db0afa8fdd8afff5abf6a94556f53180 
>   src/main/python/apache/aurora/config/schema/base.py 
> e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> cba4e8c879fae2b9f56a36bf8fa4b702163667d1 
>   src/test/python/apache/aurora/client/test_config.py 
> 8fd112fd8a10120f57324d8efec22a009b93404b 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 8561abc3e7df8803785b70064bb76553d3210399 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 
> 
> Diff: https://reviews.apache.org/r/41428/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests.
> Changed end to end test.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41368: Remove the client-side updater.

[Zameer Manji]

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

Ship it!


Awesome!

- Zameer Manji


On Dec. 14, 2015, 2:36 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41368/
> ---
> 
> (Updated Dec. 14, 2015, 2:36 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-785
> https://issues.apache.org/jira/browse/AURORA-785
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> ```
>  NEWS   |   4 +
>  docs/hooks.md  |   2 -
>  .../python/apache/aurora/client/api/__init__.py|  24 -
>  .../python/apache/aurora/client/api/updater.py | 720 -
>  src/main/python/apache/aurora/client/base.py   |  19 -
>  src/main/python/apache/aurora/client/cli/jobs.py   | 115 ---
>  .../apache/aurora/client/hooks/hooked_api.py   |  14 -
>  src/test/python/apache/aurora/client/api/BUILD |  12 -
>  .../apache/aurora/client/api/test_updater.py   | 899 
> -
>  src/test/python/apache/aurora/client/cli/BUILD |  13 -
>  .../apache/aurora/client/cli/test_cancel_update.py |  62 --
>  .../python/apache/aurora/client/cli/test_update.py | 528 
>  .../apache/aurora/client/hooks/test_hooked_api.py  |   5 +-
>  .../aurora/client/hooks/test_non_hooked_api.py |  15 +-
>  src/test/python/apache/aurora/client/test_base.py  |   8 -
>  .../sh/org/apache/aurora/e2e/test_end_to_end.sh|   8 -
>  16 files changed, 11 insertions(+), 2437 deletions(-)
> ```
> 
> 
> Diffs
> -
> 
>   NEWS 99531a3ffd98acb08b9bee9c44a1553fba4b56a6 
>   docs/hooks.md 63dbdb91d014b65f129cd6b28bdb4c563f00ead3 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 1514394829cfd8d76d6db5cdfd7c6593f1193b29 
>   src/main/python/apache/aurora/client/api/updater.py 
> acbce21e991981a2e85c1a00a68e57d88c5509bb 
>   src/main/python/apache/aurora/client/base.py 
> 91c276b09500d4eb81368f0b4bd482f0177b5e53 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 46948497f59c9c4d91fdefd8038cff138b9aeb0f 
>   src/main/python/apache/aurora/client/hooks/hooked_api.py 
> 7fc0b7103e8968af8a9288b698829f31c00e6b32 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2756912af8b0716417efa992201bf72ca736a653 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 5f54d22401a5eec0027b156ebaf50114221da8bd 
>   src/test/python/apache/aurora/client/cli/BUILD 
> 1b14e8c299360f236412c8d9b5e2e143f755f4e0 
>   src/test/python/apache/aurora/client/cli/test_cancel_update.py 
> d4fc049ebef5745b7a749a74005be962c63ae2ea 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 787396e8da298fab736d4aa8d2b436a68ebee27b 
>   src/test/python/apache/aurora/client/hooks/test_hooked_api.py 
> 4d8ebabb3a1dcf26dd491b3bb73c0057d6a8ac97 
>   src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py 
> 78dffa5bbf065dcae61ae672c928f658b7400522 
>   src/test/python/apache/aurora/client/test_base.py 
> 8c880081620a8360bb2b7f0d854c7708c14cae7a 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> d7c61e26428c05d6b2f212e2bb092af38ef6aeec 
> 
> Diff: https://reviews.apache.org/r/41368/diff/
> 
> 
> Testing
> ---
> 
> Test suite + end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41413: Add PMD coverage for test sources.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 15, 2015, 3:31 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41413/
> ---
> 
> (Updated Dec. 15, 2015, 3:31 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The most interesting parts of this essentially amount to an XML refactor.  I 
> tried to pull the common style rules into `config/pmd/common.xml` and used 
> the other two for test- and main-specific rules.
> 
> I originally didn't do this because i wasn't aware we could have different 
> rules for test/main code.  Turns out you can!
> 
> 
> Diffs
> -
> 
>   build.gradle a91370f793be9b86e54cf708b5ec1e87141bfc5c 
>   config/pmd/common.xml PRE-CREATION 
>   config/pmd/custom.xml 763051b01f468cb3a60a0f068b0d460700f1a6d6 
>   config/pmd/test.xml PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 3a6d3e46f10797a9f0e4d63eff0f0724bf969f42 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 73f74ea90dfeda8643bcebe072ca3abd179c0510 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> 73ea3cbbf1458ffeecb4bf9714b1eb37bfead32f 
>   src/test/java/org/apache/aurora/scheduler/app/local/simulator/Events.java 
> f36c1ffc515241a8bc2b8c606e4bb7188fd1b73c 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java 
> 9274e7b1aa4c01b96a0a3dfa1157a2eaa318a638 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/events/Started.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 
> 259ed867730f80dd56137347460a2625c9660e73 
>   src/test/java/org/apache/aurora/scheduler/cron/CrontabEntryTest.java 
> 57e0241f60868b1cc29f4c228dd58caadb29b466 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 9cca71b329d40ad288c882366eb39b0493d5a893 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  ce6426b67c2847115a11712fdd4972fcd34115bf 
>   src/test/java/org/apache/aurora/scheduler/http/H2ConsoleModuleIT.java 
> 6ffb3704aff592ab9c3ede968f72f5e7f37f5100 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> abf823a3efba42507fb5f7e61f570e540b12fa1a 
>   src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
> 8f76230d7f80b42b95a8369e27dcf07bdf601335 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> d0c4449f78d445a2936b4bf2993c5a42ed126b4b 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 6dc65b55aaaedc1487b5450729786ee6b0f1a217 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java 
> aa3a85ac85a88a83a9801bb3c4494553141d3a9b 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  23db909c1a229687b1d58894731406a721aba7d5 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
>  42eec1244aa2a0b99d82464e6efad24ef30e007f 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilterTest.java
>  f35dcb8eb932ae7224847d53f69c411ffa680592 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosPermissiveAuthenticationFilterTest.java
>  6eb82b542a04828299ca66dc19e81eaddacd500c 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> ecef202d8545df16a9bcd27e265b3022cc1de092 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/SchedulerDriverServiceTest.java
>  45a8a5dacbcab5eb7caacdd6a4b5289217c9284e 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  49002d03b190b8642b6251f4d1fbd6663ee6becc 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  fca4c81df6d3ee1c4edaeec9ac21697e2f22e113 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> e9a37036ed7154ca078811ff08bbac7fddba56e1 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> e68fc1d767be2a95410c28607466d2b6f1fa0436 
>   
> src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
>  d1675c68ff39b00099c60c0e85d84732f031adb1 
>   src/test/java/

Re: Review Request 41315: Upgrade mesos dependency to 0.24.1.

[Zameer Manji]

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

Ship it!


Mind linking this to https://issues.apache.org/jira/browse/AURORA-1543 and 
assigning the ticket to yourself?

- Zameer Manji


On Dec. 12, 2015, 8:49 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41315/
> ---
> 
> (Updated Dec. 12, 2015, 8:49 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Vinod Kone.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Outstanding question - can we go straight to 0.25.0?  Based on the upgrade 
> docs [1], it doesn't appear that would be problematic.  If we can do it, i 
> think we should do so and i am happy to update this patch.
> 
> [1] http://mesos.apache.org/documentation/latest/upgrades/
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 1a80d66d0ab3a920cf1ddd774c922dbd71c574ad 
>   NEWS 6e158d0495afe8581540a1d9eba3082ada627fd3 
>   build-support/python/make-mesos-native-egg 
> 47438c5b71d686c4a42ae3f46f3a7093b34f3909 
>   build.gradle aeefc572c4dd854bb263000c740af8570085123d 
>   docs/deploying-aurora-scheduler.md 812beafdc3b4c47fdc813d2dd2299b63407f8011 
>   examples/vagrant/provision-dev-cluster.sh 
> 3e4bf325601fc2326743b1344d64980e8c881a11 
> 
> Diff: https://reviews.apache.org/r/41315/diff/
> 
> 
> Testing
> ---
> 
> Successfully ran end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41154: Add support for performing health checks with a shell command.

[Zameer Manji]

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

Ship it!


So long as a blocking ticket is filed for AURORA-1525 I'm good as well.

- Zameer Manji


On Dec. 10, 2015, 11:25 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41154/
> ---
> 
> (Updated Dec. 10, 2015, 11:25 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1551
> https://issues.apache.org/jira/browse/AURORA-1551
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding support for non-HTTP health checks.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 
>   docs/configuration-reference.md 364292998bebb233d300fe59c9ea42b216deee81 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/main/python/apache/aurora/common/BUILD 
> 5fce3d0d29d2a38c6563b4d9be963532e595ee19 
>   src/main/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/common/health_check/shell.py PRE-CREATION 
>   src/main/python/apache/aurora/common/http_signaler.py 
> a3193f3259276ec23d37f45839afe3c387cff6b1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 398f737bed9ef02ce4a5636896d6587bce26501e 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 03fdf0afef120c365c6ffad09e152780eed7e351 
>   src/main/python/apache/aurora/executor/http_lifecycle.py 
> 6d578cceb56375425ccac1cbfbbcd0add60f20e9 
>   src/test/python/apache/aurora/client/BUILD 
> 84c5c845d9b1c8078ae8b47242d0f2ffc00ef6dc 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/common/BUILD 
> 2556c32842b3cf7040cb3c41172a0d9c365cb649 
>   src/test/python/apache/aurora/common/health_check/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/test_shell.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/common/test_http_signaler.py 
> f68c71a6765f7f0b93c8c50662515b5742344f35 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 27c71711d52f757ed1552db4accda671a6bdafdd 
> 
> Diff: https://reviews.apache.org/r/41154/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests. 
> Ran e2e test.
> Tested expected behavior on virtual Mesos cluster.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41331: Upgrade to pants 0.0.64 and pex 1.1.1.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 13, 2015, 3:32 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41331/
> ---
> 
> (Updated Dec. 13, 2015, 3:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1554
> https://issues.apache.org/jira/browse/AURORA-1554
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> CHANGELOGs are here:
> + http://pantsbuild.github.io/changelog.html
> + https://pypi.python.org/pypi/pex/1.1.1
> 
> The pants upgrade ends the deprecation cycle for the `python_test_suite`
> aggregator target; so these are all switched to the reccomended `target`
> aggregator target.
> 
> The pex upgrade brings in support for a global `/etc/pexrc` which could
> be desirable for control over the executor components.
> 
>  3rdparty/python/requirements.txt| 2 +-
>  pants.ini   | 2 +-
>  src/test/python/BUILD   | 2 +-
>  src/test/python/apache/aurora/BUILD | 2 +-
>  src/test/python/apache/aurora/admin/BUILD   | 2 +-
>  src/test/python/apache/aurora/client/BUILD  | 2 +-
>  src/test/python/apache/aurora/client/api/BUILD  | 2 +-
>  src/test/python/apache/aurora/client/cli/BUILD  | 2 +-
>  src/test/python/apache/aurora/client/hooks/BUILD| 2 +-
>  src/test/python/apache/aurora/common/BUILD  | 2 +-
>  src/test/python/apache/aurora/config/BUILD  | 2 +-
>  src/test/python/apache/aurora/executor/BUILD| 6 +++---
>  src/test/python/apache/aurora/executor/bin/BUILD| 2 +-
>  src/test/python/apache/aurora/executor/common/BUILD | 2 +-
>  src/test/python/apache/aurora/tools/BUILD   | 2 +-
>  src/test/python/apache/thermos/BUILD| 2 +-
>  src/test/python/apache/thermos/common/BUILD | 2 +-
>  src/test/python/apache/thermos/config/BUILD | 2 +-
>  src/test/python/apache/thermos/core/BUILD   | 8 
>  src/test/python/apache/thermos/monitoring/BUILD | 2 +-
>  src/test/python/apache/thermos/observer/BUILD   | 2 +-
>  src/test/python/apache/thermos/observer/http/BUILD  | 2 +-
>  22 files changed, 27 insertions(+), 27 deletions(-)
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 
>   pants.ini 1e19d9af38bc2ec362aa2130cc37d1532454ceee 
>   src/test/python/BUILD 34d59a58758ef41ca42fd4944c953bedf4205def 
>   src/test/python/apache/aurora/BUILD 
> 7db8b0e3a798a8013ebbcddb73f49bdd5d4373d8 
>   src/test/python/apache/aurora/admin/BUILD 
> d4010458b0580e1a70ee7d5262b2427d25068e09 
>   src/test/python/apache/aurora/client/BUILD 
> 84c5c845d9b1c8078ae8b47242d0f2ffc00ef6dc 
>   src/test/python/apache/aurora/client/api/BUILD 
> c375ae98c3fe3041ab02ac2cca0b82428128c0cf 
>   src/test/python/apache/aurora/client/cli/BUILD 
> 36fd98a2f3b47630912a3e20f1d34724d508d92d 
>   src/test/python/apache/aurora/client/hooks/BUILD 
> ae97ebf2dd77661a8fa600b520c210c239a304ea 
>   src/test/python/apache/aurora/common/BUILD 
> 2556c32842b3cf7040cb3c41172a0d9c365cb649 
>   src/test/python/apache/aurora/config/BUILD 
> 92bc6800bc4ac904df9bca8d6d2a784ea243c190 
>   src/test/python/apache/aurora/executor/BUILD 
> 1c973753e65f05e2d25846de9bd693dcd78c86e4 
>   src/test/python/apache/aurora/executor/bin/BUILD 
> c3b538c026de649c442f3d3e8eef653636504830 
>   src/test/python/apache/aurora/executor/common/BUILD 
> 7dafebfd1bb2ab50691df2fbed0dcb8761edff28 
>   src/test/python/apache/aurora/tools/BUILD 
> e676aff357e48fe2c706d74c5b3d8b0a1af5f2dd 
>   src/test/python/apache/thermos/BUILD 
> 821307a2824b9f3ebdb2b1da9ecedb5ed46cc28e 
>   src/test/python/apache/thermos/common/BUILD 
> 7813dbcdb5d430d4f93cd8e3cea9c63cb625d9a4 
>   src/test/python/apache/thermos/config/BUILD 
> cee0f05246cebc3ee3d02d0227c2262082b0ef89 
>   src/test/python/apache/thermos/core/BUILD 
> 53cc575fb036690b6b361f66c75c11c4362cf4e9 
>   src/test/python/apache/thermos/monitoring/BUILD 
> b8d6d2fbc77c99e4e1739dae0582dd01ba9a5e1b 
>   src/test/python/apache/thermos/observer/BUILD 
> 34475d27228ac5f40bcd1289959ee3c13ec28bb0 
>   src/test/python/apache/thermos/observer/http/BUILD 
> bdead2cf04079c583de62854235249020585e145 
> 
> Diff: https://reviews.apache.org/r/41331/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./build-support/jenkins/build.sh`
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 41154: Add support for performing health checks with a shell command.

[Zameer Manji]

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


Overall this LGTM. The only thing holding back my shipit is a possible addition 
to the e2e test suite.

Bill, Maxim, Josh: Do you think this change requires an addition to the e2e 
suite? I'm on the fence. It seems overkill for this change, but changes to the 
executor make me extra cautious.


docs/configuration-reference.md (line 388)
<https://reviews.apache.org/r/41154/#comment169744>

Please update the documentation of `expected_response` to be "If not empty, 
fail the HTTP health check if the response differs..."



docs/configuration-reference.md (line 389)
<https://reviews.apache.org/r/41154/#comment169745>

Please update the documentation of `expected_response_code` to be "If not 
zero, fail the HTTP health check..."



src/main/python/apache/aurora/client/config.py (line 163)
<https://reviews.apache.org/r/41154/#comment169740>

You need to call `_validate_health_check_config` here.


- Zameer Manji


On Dec. 10, 2015, 11:25 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41154/
> ---
> 
> (Updated Dec. 10, 2015, 11:25 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1551
> https://issues.apache.org/jira/browse/AURORA-1551
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding support for non-HTTP health checks.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 
>   docs/configuration-reference.md 364292998bebb233d300fe59c9ea42b216deee81 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/main/python/apache/aurora/common/BUILD 
> 5fce3d0d29d2a38c6563b4d9be963532e595ee19 
>   src/main/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/common/health_check/shell.py PRE-CREATION 
>   src/main/python/apache/aurora/common/http_signaler.py 
> a3193f3259276ec23d37f45839afe3c387cff6b1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 398f737bed9ef02ce4a5636896d6587bce26501e 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 03fdf0afef120c365c6ffad09e152780eed7e351 
>   src/main/python/apache/aurora/executor/http_lifecycle.py 
> 6d578cceb56375425ccac1cbfbbcd0add60f20e9 
>   src/test/python/apache/aurora/client/BUILD 
> 84c5c845d9b1c8078ae8b47242d0f2ffc00ef6dc 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
>   src/test/python/apache/aurora/common/BUILD 
> 2556c32842b3cf7040cb3c41172a0d9c365cb649 
>   src/test/python/apache/aurora/common/health_check/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/test_shell.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/common/test_http_signaler.py 
> f68c71a6765f7f0b93c8c50662515b5742344f35 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 27c71711d52f757ed1552db4accda671a6bdafdd 
> 
> Diff: https://reviews.apache.org/r/41154/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests. 
> Ran e2e test.
> Tested expected behavior on virtual Mesos cluster.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41154: Add support for performing health checks with a shell command.

[Zameer Manji]

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


I have not looked at the tests yet but the implementation looks ok to me. I 
also agree with the idea in general. I think having a more flexible concept of 
health checking is a good feature to have.

Also I think a header needs to be in 
`src/test/python/apache/aurora/common/health_check/__init__.py`


src/main/python/apache/aurora/common/health_check/shell.py (line 55)
<https://reviews.apache.org/r/41154/#comment169518>

I don't think this is true. OSError can signal a variety of underlying 
issues.

I think it would be better to set the reason to "OSError: 
{error}".format(e.strerror).

For example the binary might not be found, but it also might not be 
executable by the current user, etc, etc.



src/main/python/apache/aurora/executor/common/health_checker.py (line 212)
<https://reviews.apache.org/r/41154/#comment169524>

It seems like a logical error to both set "shell_command" and have the 
'health' port set.

I think we should raise an error on the client side and inform the user the 
schema is invalid so the logic here can remain the same. You can do this by 
adding another check to `validate_config` in 
`src/main/python/apache/aurora/client/config.py`.

Also, it seems bit weird to have the switch between http checking and shell 
checking to be the presence of the health port vs setting the "shell_command" 
field.

What do you think about having a type field to the HealthCheck config that 
can be one of 'http' (default) or 'shell'. The documentation can then be 
updated to explain which fields are expected for each type. I think this might 
make it easier to add more types of health checking in the future.


- Zameer Manji


On Dec. 10, 2015, 1:16 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41154/
> ---
> 
> (Updated Dec. 10, 2015, 1:16 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1551
> https://issues.apache.org/jira/browse/AURORA-1551
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding support for non-HTTP health checks.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 
>   docs/configuration-reference.md 364292998bebb233d300fe59c9ea42b216deee81 
>   src/main/python/apache/aurora/common/BUILD 
> 5fce3d0d29d2a38c6563b4d9be963532e595ee19 
>   src/main/python/apache/aurora/common/health_check/BUILD PRE-CREATION 
>   src/main/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/common/health_check/shell.py PRE-CREATION 
>   src/main/python/apache/aurora/common/http_signaler.py 
> a3193f3259276ec23d37f45839afe3c387cff6b1 
>   src/main/python/apache/aurora/config/schema/base.py 
> 398f737bed9ef02ce4a5636896d6587bce26501e 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 03fdf0afef120c365c6ffad09e152780eed7e351 
>   src/main/python/apache/aurora/executor/http_lifecycle.py 
> 6d578cceb56375425ccac1cbfbbcd0add60f20e9 
>   src/test/python/apache/aurora/common/BUILD 
> 2556c32842b3cf7040cb3c41172a0d9c365cb649 
>   src/test/python/apache/aurora/common/health_check/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/test_shell.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/common/test_http_signaler.py 
> f68c71a6765f7f0b93c8c50662515b5742344f35 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 27c71711d52f757ed1552db4accda671a6bdafdd 
> 
> Diff: https://reviews.apache.org/r/41154/diff/
> 
> 
> Testing
> ---
> 
> Added unit tests. 
> Ran e2e test.
> Tested expected behavior on virtual Mesos cluster.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 41074: Use lambdas throughout the project.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 8, 2015, 10:01 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41074/
> ---
> 
> (Updated Dec. 8, 2015, 10:01 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is almost entirely IDE-assisted.  There were some spots that required 
> hand-editing (notably casting the argument to `storage.write`).
> 
> The overall stats are nice, a reduction of about 1300 lines:
> ```
>  114 files changed, 1982 insertions(+), 3292 deletions(-)
> ```
> 
> 
> Diffs
> -
> 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5abb06b1d41f3265d9d46c600ba193ce19d21188 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  d06ec69cb57d9c411ae33b301489f6460197dcc7 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9a4d441023fd9eea868fd069c197eb002d98f0ab 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> cc4710cb5f559a108b140489a981a9d2c195b75b 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> ad0b299057e787a9587822e7778bd360f77e1678 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> 051e3f93ef17de90b7bffaa74fc6b54965927424 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> c1fe1c73c46da21c471b2669dbb8207b7c468054 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> 62564a62d255e640e5790ed2e103ba8425a4c15b 
>   commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java 
> 3da1812abfdb30f12737b19f5e87bc14a02d4c3f 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/RangeNumberVerifier.java
>  27250944a60f906a4c220771e9948ed0297ee76c 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/ListParser.java 
> 02c10ed067c8395a2688cd69d4d73d5fd17774ff 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/SetParser.java 
> 1e12b06e284cd2e33b63b3690abd3dcd78f9e777 
>   commons/src/main/java/org/apache/aurora/common/base/Closures.java 
> d6cb82a30a8c6292bab0fa007dce2f54f6b66435 
>   commons/src/main/java/org/apache/aurora/common/base/Commands.java 
> 8a88ae336f57fcdafb984cc3418dcd960da0028d 
>   commons/src/main/java/org/apache/aurora/common/collections/Iterables2.java 
> 072159e79b2d8872f39dfbba9f6a1d27295f1a7a 
>   commons/src/main/java/org/apache/aurora/common/collections/Pair.java 
> b1f9b6336aa8b8d0ff9e611dc2ad1dc8068ef7f3 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java
>  348a8c923118da3b4399afb1a6f09d904c3b 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
>  8039def73377b0e56c3106876dc89e9183cd55a1 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsHandler.java
>  d1f23de1a440d47a7754af2c8418e2c4e96bbb84 
>   commons/src/main/java/org/apache/aurora/common/stats/JvmStats.java 
> 03df9b6f7d31d8567a6bd135654eb39026c375d7 
>   commons/src/main/java/org/apache/aurora/common/stats/Rate.java 
> ff29c39863f044db7fde0fff315621644393429d 
>   commons/src/main/java/org/apache/aurora/common/stats/Ratio.java 
> c332dd3af5712bfb8ed174c2285cac4d30394acc 
>   
> commons/src/main/java/org/apache/aurora/common/testing/easymock/EasyMockTest.java
>  403147527eab46e9c23ab5690d885a6c1deb1c30 
>   commons/src/main/java/org/apache/aurora/common/util/BackoffHelper.java 
> e789f804ecc0053d4605927dc981311af1df94c8 
>   commons/src/main/java/org/apache/aurora/common/util/StateMachine.java 
> 9fbfbb9012414a19cf752d69b48f2e937f4497e7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 
> b10a40327a062b354b42249097c89bf3fbecb1e8 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 
> 99a15fe8d58138ebd264d858bff842316928db06 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
> bb6fbced0adc0920592ec14f03a84895829d8812 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java 
> 1761ab70a32656af13a311bcc02f716fdc0ad067 
>   config/legacy_untested_classes.txt 07e49b11050f8b9be63d5b95cfcdb7800e15c7e5 
>   src/main/java/org/apache/aurora/GuiceUtils.java 

Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

[Zameer Manji]

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

Ship it!


This seems to be a minor change that we can reverse and it does remove 
boilerplate code.

- Zameer Manji


On Nov. 30, 2015, 7:05 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Nov. 30, 2015, 7:05 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

[Zameer Manji]

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


I tried to commit this on master and applying the patch failed. John, can you 
please rebase this?

- Zameer Manji


On Nov. 30, 2015, 7:05 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Nov. 30, 2015, 7:05 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40889: Changed mesos native lib to use mesos.executor instead

[Zameer Manji]

> On Dec. 2, 2015, 4:36 p.m., Maxim Khutornenko wrote:
> > 3rdparty/python/BUILD, line 14
> > 
> >
> > Am I reading this as revving up mesos native version in Aurora? If so, 
> > we should also update build.gradle and possibly update 
> > make-mesos-native-egg script. Previous effort as a reference: 
> > https://reviews.apache.org/r/37379
> 
> Bill Farner wrote:
> You're right, Maxim.  I think it's safe to say this patch is still in PoC 
> stage, as it can't land until some changes go into mesos.  I think Steve is 
> not expecting ship-its just yet.
> 
> Steve Niemitz wrote:
> Correct, I'll ping when I'm ready for more eyes on it.

If you do want to do this, please assign 
https://issues.apache.org/jira/browse/AURORA-1543 to yourself.


- Zameer


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


On Dec. 2, 2015, 3:57 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40889/
> ---
> 
> (Updated Dec. 2, 2015, 3:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changed mesos native lib to use mesos.executor instead
> 
> Without any context this doesnt really make any sense.
> 
> I've created a seperate mesos native lib that only contains a minimal set of 
> code needed to run the ExecutorDriver.  This library has very few 
> requirements, making it a much better option for running in docker 
> containers, as well as allowing a thermos_executor.pex built on one linux 
> distro to run on most others.
> 
> The mesos side of this is here: 
> https://github.com/tellapart/mesos/commit/46bedd5db73b535a0e4efe94730500b6da2ec7d8
> and I'll be opening a RB request on the mesos side soon.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 
>   src/main/python/apache/aurora/executor/BUILD 
> 0be65fc7c180d15f5a94bef268652ef7b868dcf7 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
> 
> Diff: https://reviews.apache.org/r/40889/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 41056: Upgrade checkstyle and pmd versions.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 7, 2015, 2:56 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41056/
> ---
> 
> (Updated Dec. 7, 2015, 2:56 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We've been holding back on PMD upgades for a while.  This was due to false 
> positives, but they seem to have been resolved.  With these updates, i 
> believe we can make more extensive use of lambdas without tripping over QA 
> tools.
> 
> Release notes:
> https://pmd.github.io/pmd-5.4.1/overview/changelog.html
> http://checkstyle.sourceforge.net/releasenotes.html
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   config/pmd/custom.xml c31cac98b14960057905d5224aa23824e48ccf42 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> 3709c775f80f84d9c4df72d2414c24bf8f6a802b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ea43912380f89b4fb06d73c63ec23c47b8f47288 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  326d4fbc3261c57c44d4dc51d74e05b1155de6ad 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> 0acceaaec851779809adc339a441f33661f9308c 
> 
> Diff: https://reviews.apache.org/r/41056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 40673: Set JDK and langauge level for entire project.

[Zameer Manji]

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

To ensure IDEA is configured correctly, set JDK and langauge level for entire 
project.


Diffs
-

  build.gradle 32295e6b790f7c7c7126a2e68d66a276dd018daa 

Diff: https://reviews.apache.org/r/40673/diff/


Testing
---

./gradlew idea
Verified that a freshly created project uses JDK 1.8 with JDK 8 langauge 
features.


Thanks,

Zameer Manji

Re: Review Request 40656: Remove SessionKey from APIs and implementations.

[Zameer Manji]

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

Ship it!


LGTM assuming E2E tests pass and 0.10.0 client can communicate against this 
scheduler.

- Zameer Manji


On Nov. 24, 2015, 12:29 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40656/
> ---
> 
> (Updated Nov. 24, 2015, 12:29 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-814
> https://issues.apache.org/jira/browse/AURORA-814
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since our auth* is now transport-level, we don't need it in the app-level 
> messages.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> b2628846dfe1d9add0d0edb0e7f2f6ce0121de2b 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ccabf55c947393bc04147c8527507e621262e7f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  6d30c25e62061855d8b22d58ad6f33fab7fcc3fb 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> c973da109103b0b2f2d47e45a353898f0635203c 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> fb5159db0a0e98d0db2fe506d0497f6b4cc09281 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  2454959eab3334e10e6bea8b7a600b9e88adb83d 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java
>  650f21460a3853dc59cec94544125c9c270404b8 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  d03cd2ae9798819f3ac9b9a563957393c219bb41 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2bfc2a79aa76a8aed51b428d5d9c2c4e77fca50c 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 63c20d19a641119e59003b6c31e6b1de3462defb 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> dafcf25cab9c6974fad07cd7ae271983c919f8c4 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
>  f0caa8bfa4b48aea7ef448243752ef32c8b0c39a 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> f31d60d7831cb7c6dfdd3978bd4189fe54f856a8 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> d244b3aa14a26ab50e921afaa9c9d7f990d8f1ef 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> b7b165351107ecd3d2d73fcbb8139758b969ad00 
>   src/test/python/apache/aurora/client/fake_scheduler_proxy.py 
> 183d2d67452d8b57c555b4b9f21f7c2927e323ce 
> 
> Diff: https://reviews.apache.org/r/40656/diff/
> 
> 
> Testing
> ---
> 
> I did a quick-and-dirty test between a new client and old scheduler to verify 
> that the thrift compatibility works as expected and didn't encounter any 
> problems.
> 
> TODO(wfarner): Verify in end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 40689: Upgrade RBTools to 0.7.5

[Zameer Manji]

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

Review request for Aurora, John Sirois and Bill Farner.


Repository: aurora


Description
---

Upgrade to RBTools 0.7.5 and use the wheel hosted on PyPI.
Changelog: https://www.reviewboard.org/docs/releasenotes/rbtools/0.7.5/


Diffs
-

  rbt 59f8629f33f77685b844b517fccb45c951d58d54 

Diff: https://reviews.apache.org/r/40689/diff/


Testing
---

Modified the script and ran ./rbt


Thanks,

Zameer Manji

Re: Review Request 40662: Remove jersey dependency from commons code.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 24, 2015, 10:41 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40662/
> ---
> 
> (Updated Nov. 24, 2015, 10:41 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We should have always been depending on the jax-rs API rather than the API 
> bundled in jersey.
> 
> 
> Diffs
> -
> 
>   build.gradle 32295e6b790f7c7c7126a2e68d66a276dd018daa 
> 
> Diff: https://reviews.apache.org/r/40662/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40658: Copy artifacts from containers rather than using volume mounts.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 24, 2015, 9:24 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40658/
> ---
> 
> (Updated Nov. 24, 2015, 9:24 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Bugs: AURORA-1539
> https://issues.apache.org/jira/browse/AURORA-1539
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Copy artifacts from containers rather than using volume mounts.
> 
> 
> Diffs
> -
> 
>   build-artifact.sh 30e4384c51d39e4a4ab3407e7b4db8022ee8da8f 
>   builder/deb/ubuntu-trusty/build.sh bb2b79962db90fe864ea89b824e92d1a91b5a6cb 
> 
> Diff: https://reviews.apache.org/r/40658/diff/
> 
> 
> Testing
> ---
> 
> Created deb and RPM of 0.10.0, artifacts left on the host machine are no 
> longer root-owned.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40656: Remove SessionKey from APIs and implementations.

[Zameer Manji]

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



api/src/main/thrift/org/apache/aurora/gen/api.thrift 
<https://reviews.apache.org/r/40656/#comment167130>

There are two things being done in this review. Removing this struct from 
our APIs and removing this struct entirely.

I beleive if you restore this struct this patch will go green and our APIs 
will not have SessionKey.

We can later remove it from the client side if you don't want to tackle 
that now.


- Zameer Manji


On Nov. 24, 2015, 9:19 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40656/
> ---
> 
> (Updated Nov. 24, 2015, 9:19 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-814
> https://issues.apache.org/jira/browse/AURORA-814
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since our auth* is now transport-level, we don't need it in the app-level 
> messages.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> b2628846dfe1d9add0d0edb0e7f2f6ce0121de2b 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ccabf55c947393bc04147c8527507e621262e7f3 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  6d30c25e62061855d8b22d58ad6f33fab7fcc3fb 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> c973da109103b0b2f2d47e45a353898f0635203c 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> fb5159db0a0e98d0db2fe506d0497f6b4cc09281 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  2454959eab3334e10e6bea8b7a600b9e88adb83d 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingInterceptorTest.java
>  650f21460a3853dc59cec94544125c9c270404b8 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  d03cd2ae9798819f3ac9b9a563957393c219bb41 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  2bfc2a79aa76a8aed51b428d5d9c2c4e77fca50c 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 63c20d19a641119e59003b6c31e6b1de3462defb 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
> dafcf25cab9c6974fad07cd7ae271983c919f8c4 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
>  f0caa8bfa4b48aea7ef448243752ef32c8b0c39a 
> 
> Diff: https://reviews.apache.org/r/40656/diff/
> 
> 
> Testing
> ---
> 
> I did a quick-and-dirty test between a new client and old scheduler to verify 
> that the thrift compatibility works as expected and didn't encounter any 
> problems.
> 
> TODO(wfarner): Verify in end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40666: Remove unnecessary volume mount argument.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 24, 2015, 11:36 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40666/
> ---
> 
> (Updated Nov. 24, 2015, 11:36 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> I neglected to omit this in https://reviews.apache.org/r/40658/
> 
> 
> Diffs
> -
> 
>   build-artifact.sh 0bb7ea6f65494cb583506aa9dc678cdb85a368b8 
> 
> Diff: https://reviews.apache.org/r/40666/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 40510: Remove unused and deprecated code from commons.

[Zameer Manji]

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

Review request for Aurora, John Sirois and Bill Farner.


Repository: aurora


Description
---

Remove unused and deprecated code from commons. This is some code I deleted 
previously but neglected to put up for review until now.


Diffs
-

  commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java 
93f6610f24b0f19bac4d8c4daad44b8aadce85b8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java 
fb578e191c0349312e065cafcc0b82fefc870ba8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
07ae3834ff07350110c5b627d29cd4ec2a623aa0 
  
commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java 
110c9ef82e445d47866ffa604fa84d37755f0c3f 

Diff: https://reviews.apache.org/r/40510/diff/


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

Re: Review Request 40510: Remove unused and deprecated code from commons.

[Zameer Manji]

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

(Updated Nov. 19, 2015, 4:13 p.m.)


Review request for Aurora, John Sirois and Bill Farner.


Changes
---

Another batch.


Repository: aurora


Description
---

Remove unused and deprecated code from commons. This is some code I deleted 
previously but neglected to put up for review until now.


Diffs (updated)
-

  
commons/src/main/java/org/apache/aurora/common/net/InetSocketAddressHelper.java 
7e88b27f2d4c210a1f6b04f256452ffbd3639be8 
  commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java 
93f6610f24b0f19bac4d8c4daad44b8aadce85b8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java 
fb578e191c0349312e065cafcc0b82fefc870ba8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
07ae3834ff07350110c5b627d29cd4ec2a623aa0 
  
commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java 
110c9ef82e445d47866ffa604fa84d37755f0c3f 
  src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
377dbfda85a1e1e47e3855b9dcdf538ffb843e54 

Diff: https://reviews.apache.org/r/40510/diff/


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

Re: Review Request 40510: Remove unused and deprecated code from commons.

[Zameer Manji]

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

(Updated Nov. 19, 2015, 4:25 p.m.)


Review request for Aurora, John Sirois and Bill Farner.


Changes
---

last batch


Repository: aurora


Description
---

Remove unused and deprecated code from commons. This is some code I deleted 
previously but neglected to put up for review until now.


Diffs (updated)
-

  
commons/src/main/java/org/apache/aurora/common/net/InetSocketAddressHelper.java 
7e88b27f2d4c210a1f6b04f256452ffbd3639be8 
  commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java 
93f6610f24b0f19bac4d8c4daad44b8aadce85b8 
  commons/src/main/java/org/apache/aurora/common/thrift/Util.java 
7802614e050f0fa6fc5ad94fd738433f7f75abab 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java 
fb578e191c0349312e065cafcc0b82fefc870ba8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
07ae3834ff07350110c5b627d29cd4ec2a623aa0 
  
commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java 
110c9ef82e445d47866ffa604fa84d37755f0c3f 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java 
a032aa35d6d7772cd5c0d8d9a45f9edc20e47778 
  src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
377dbfda85a1e1e47e3855b9dcdf538ffb843e54 

Diff: https://reviews.apache.org/r/40510/diff/


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji

Re: Review Request 40391: Introduce a utility class to read executor configurations in JSON.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 17, 2015, 9:51 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40391/
> ---
> 
> (Updated Nov. 17, 2015, 9:51 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1376
> https://issues.apache.org/jira/browse/AURORA-1376
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Another bit-sized chunk towards AURORA-1376.  The follow-up will allow an 
> executor configuration file to be provided on the scheduler command line.
> 
> 
> Diffs
> -
> 
>   build.gradle 17dbd2575138880e8ecbbee2b9daf0f1631172c3 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 556a3f80236971f6b6cf443d9d1de9394a3aae2d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoaderTest.java
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-missing-field.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-thermos-executor.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40391/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40334: Upgrade to pants `0.0.59` to avoid pytest errors.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 15, 2015, 3:36 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40334/
> ---
> 
> (Updated Nov. 15, 2015, 3:36 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The problem is described here:
>   https://github.com/pantsbuild/pants/issues/2566
> 
> Changelogs are here:
>   https://pypi.python.org/pypi/pantsbuild.pants/0.0.59
> 
>  pants.ini | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> Diffs
> -
> 
>   pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 
> 
> Diff: https://reviews.apache.org/r/40334/diff/
> 
> 
> Testing
> ---
> 
> Before the fix stumbled into this:
> ```
> $ ./build-support/jenkins/build.sh
> ...
> 11:22:51 00:00   [test]
> 11:22:51 00:00 [run_prep_command]
> 11:22:51 00:00 [test]
> 11:22:51 00:00 [pytest]
> 11:22:51 00:00   [run]
> 
> 11:23:04 00:13 [chroot]
> 11:23:11 00:20   [complete]
>FAILURE
> 
> Exception message: Could not satisfy all requirements for pytest>=2.6,<2.7:
> pytest>=2.6,<2.7, pytest>=2.8.0(from: pytest-timeout)
> ```
> 
> That triggered a bit of a pants release firefight leading
> to this RB and one for Medium's mono repo as well as patching
> of the fix in at Square. Now
> `./build-support/jenkins/build.sh` runs green as well.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40320: Organize executor-related code into a package.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 14, 2015, 9 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40320/
> ---
> 
> (Updated Nov. 14, 2015, 9 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is pure code shuffle, the interesting bits coming out of `SchedulerMain` 
> and into `ExecutorModule`.
> 
> Probably should have done this in https://reviews.apache.org/r/40149/, but 
> another follow-up change made it more obvious that this organization was 
> needed.
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt aac822b6e2c022625432e09ad8d4097790429257 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> b4ca01b0ae751bdc8fa2d6fb7c667fe3c08ca726 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 6236a531b2bb1c9ba09840911ec6d4846cdb9393 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> fb4f0a031d3976cbd225fc050487b4002e64ae0c 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 52776c9cb70127cb88a40f98e2c7cce192b3b5b1 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorConfig.java 
> b6aa2e1567c79156e1ad7270d430a6fcb282822d 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b7f30231329fe2cf5bc72d11b1f929f394c21c6b 
>   src/main/java/org/apache/aurora/scheduler/mesos/Executors.java 
> 21152f575ed6b0cafe5739df718ef735837b19ac 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> f1c2059045624ed2486982614f9df14829a00ebc 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> ad5927c9d67749a1a83640ee94d76919640a7949 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  b14ab68017b0c66141f6a8bae4b0eacde841a3a2 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> da7d1e0dfb6aa8ddf452de79db8bd23491822c89 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 50853cfffab832d75f5106742d38a1864d9c6c30 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> a258d06cb12b2962d47dbb74a2af29281e255662 
> 
> Diff: https://reviews.apache.org/r/40320/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.

[Zameer Manji]

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

Ship it!


LGTM.


pants.ini (line 49)
<https://reviews.apache.org/r/40310/#comment165447>

Out of curiosity, could we replace isort with this functionality? I like 
the idea of collapsing all of that functionality into pants.


- Zameer Manji


On Nov. 16, 2015, 9:30 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40310/
> ---
> 
> (Updated Nov. 16, 2015, 9:30 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and 
> Zameer Manji.
> 
> 
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This upgrades to pants 0.0.58 to pick up the newly split off pants
> python checks contrib plugin.  Release notes are here:
>   https://pypi.python.org/pypi/pantsbuild.pants/0.0.58
> 
> The plugin provides both python checkstyle (`compile.pythonstyle`), and
> a python eval task (`compile.python-eval`).  The `python-eval` is turned
> off since at least one of the Aurora python targets has files that have
> side-effects upon import (a repl is started).
> 
> Now style checks run before compile (and thus before tests) and they
> benefit from fingerprinting; ie: if you test your changes, those tests
> will run style checks and when you go to commit, those checks will not
> be re-run by the commit hook (although files you did not test will still
> need to be checked).
> 
> A few production files were fixed up according to style failures coming
> from:
> + no space after comment opening '#'
> + unused variables
> + mis-aligned hanging closing parens.
> 
> 
> Diffs
> -
> 
>   build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea 
>   build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 
>   build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 
>   build-support/python/checkstyle-check 
> b2bfc5dd71193a8056828e9af05a4c16965f32a1 
>   pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> 6d94c923ae37bf6b827519d3505b100af306296b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 6f07a3073a5d422373238619d459fbd09d8adf3d 
>   src/main/python/apache/aurora/client/cli/client.py 
> 297fb588808c1eebc32ac3374265ba986dab3436 
>   src/main/python/apache/aurora/client/cli/cron.py 
> 6376fd014f2a4da29442b5c2c7eb36578b503ba3 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/monitoring/process_collector_psutil.py 
> f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 
>   src/test/python/apache/aurora/admin/test_admin.py 
> 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 
>   src/test/python/apache/aurora/admin/util.py 
> 3570407b51613d0a7b4fde8a4794d88b98e150b5 
>   src/test/python/apache/aurora/client/cli/test_task.py 
> 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 
> 
> Diff: https://reviews.apache.org/r/40310/diff/
> 
> 
> Testing
> ---
> 
> See the discarded https://reviews.apache.org/r/40219/ for the
> commit-hook check.  This version of that RB engages the same code
> and this RB commit was vetted by the same commit-hook.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40201: Cleanup thermos_executor test pexes.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 13, 2015, 7:32 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40201/
> ---
> 
> (Updated Nov. 13, 2015, 7:32 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, Brian Wickman, and 
> Zameer Manji.
> 
> 
> Bugs: AURORA-547
> https://issues.apache.org/jira/browse/AURORA-547
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Previously these were built to the standard `dist/` dir, now they are
> dumped to a tmp dir that's cleaned up.
> 
>  src/test/python/apache/aurora/executor/test_thermos_executor.py| 24 
> +++-
>  src/test/python/apache/aurora/executor/test_thermos_task_runner.py | 15 
> ++-
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> d24c61162e11bb4afef2902ebb031478fe3ed247 
>   src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
> bb998c0b0ac87ba51ec13e490338b00e7f85e8cf 
> 
> Diff: https://reviews.apache.org/r/40201/diff/
> 
> 
> Testing
> ---
> 
> Green locally:
> ```
> $ rm -rf dist/ && \
>   ./pants test.pytest --no-fast src/test/python/apache/aurora/executor:
> ```
> 
> And no `dist/` created.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40304: Upgrade to checkstyle 6.12.1

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 13, 2015, 10:39 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40304/
> ---
> 
> (Updated Nov. 13, 2015, 10:39 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Release notes: http://checkstyle.sourceforge.net/releasenotes.html
> 
> Context - i'm trying to keep up-to-date with checkstyle, pmd, and findbugs to 
> get past various issues related to use of lambdas and method references 
> without giving up code quality checks.  There is a significant update to PMD 
> that is nearly to ready for us - just one false positive issue i bumped into: 
> http://sourceforge.net/p/pmd/bugs/1422/
> 
> 
> Diffs
> -
> 
>   build.gradle ca18d6c3b998321bc64e3806ad885b1ceabc99a8 
> 
> Diff: https://reviews.apache.org/r/40304/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.

[Zameer Manji]

> On Nov. 12, 2015, 11:05 a.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants 
> > plugin is the way to go here. Historically we have been very bad at 
> > upgrading pants and maintaining it, I'm afraid that if we add a custom 
> > plugin here we won't be able to upgrade pants in the future at all.
> 
> John Sirois wrote:
> As you see fit.  I will say that the APIs used here are minimal and 
> historically stable.  For example, Medium similarly enables checkstyle as 
> well as another, non-default-installed plugin, `python-eval`, and they have 
> not had to touch their plugin since initial install at 0.0.45 in August when 
> it was released.  The other bit to know is pants is locking down to 1.0.0 as 
> the year closes to isolate more radical changes.  There will be a long-term 
> 1.0.0 branch the Square will be a major user of and a maintainer of with the 
> primary focus being stability, secondary bugfixes, but ~no new public 
> activity as the pants community charges on master towards 2.0.

Don't give up on this approach just yet, I'm curious on what the other 
reviewers have to say. I think putting the check in the pants layer is far 
better than what we have right now. I'm just not sure if we can maintain this 
going forward. If pants 1.0.0 will maintain this API that's pretty good to hear.


- Zameer


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


On Nov. 12, 2015, 12:54 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
> 
> (Updated Nov. 12, 2015, 12:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and 
> Zameer Manji.
> 
> 
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal.  Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
> 
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
> 
>  build-support/hooks/pre-commit   
>|  3 +--
>  build-support/jenkins/build.sh   
>|  9 +
>  build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD} 
>| 13 ++---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/__init__.py}   | 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/__init__.py}| 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/__init__.py}  | 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/BUILD}   | 18 
> +++---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12 
> +---
>  
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
>   | 36 
>  build-support/python/checkstyle  
>| 34 --
>  build-support/python/checkstyle-check
>|  6 +++---
>  pants.ini
>| 43 
> +++
>  src/main/python/apache/aurora/admin/maintenance.py   
>|  2 +-
>  src/main/python/apache/aurora/client/api/__init__.py 
>

Re: Review Request 40208: Eliminate OOB pip install of python deps in CI.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 11, 2015, 3:58 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40208/
> ---
> 
> (Updated Nov. 11, 2015, 3:58 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-954
> https://issues.apache.org/jira/browse/AURORA-954
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Under older pants, and thus older pex, distributions were fetched using
> urllib and were not robust to flaky connections. Now that Aurora is on
> modern pants and pex, which uses the `requests` library for fetching and
> includes 5 retries by default, this step should no longer be needed to
> ensure stable CI runs.
> 
>  build-support/jenkins/build.sh | 7 ---
>  pants.ini  | 5 -
>  2 files changed, 12 deletions(-)
> 
> 
> Diffs
> -
> 
>   build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e 
>   pants.ini 0bd8ec829ae65e2296a122166dea13f315323c9f 
> 
> Diff: https://reviews.apache.org/r/40208/diff/
> 
> 
> Testing
> ---
> 
> I still can't quite run `./build-support/jenkins/build.sh` straight up
> due to https://issues.apache.org/jira/browse/AURORA-1083 but ran the
> following successfully locally which forces re-download of requirements
> by pants:
> ```
> $ ./pants clean-all test.pytest --no-fast src/test/python::
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.

[Zameer Manji]

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


Although the end result LGTM, I'm not sure if having our own custom pants 
plugin is the way to go here. Historically we have been very bad at upgrading 
pants and maintaining it, I'm afraid that if we add a custom plugin here we 
won't be able to upgrade pants in the future at all.

- Zameer Manji


On Nov. 12, 2015, 12:54 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
> 
> (Updated Nov. 12, 2015, 12:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and 
> Zameer Manji.
> 
> 
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal.  Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
> 
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
> 
>  build-support/hooks/pre-commit   
>|  3 +--
>  build-support/jenkins/build.sh   
>|  9 +
>  build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD} 
>| 13 ++---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/__init__.py}   | 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/__init__.py}| 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/__init__.py}  | 12 
> +---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/BUILD}   | 18 
> +++---
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12 
> +---
>  
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
>   | 36 
>  build-support/python/checkstyle  
>| 34 --
>  build-support/python/checkstyle-check
>|  6 +++---
>  pants.ini
>| 43 
> +++
>  src/main/python/apache/aurora/admin/maintenance.py   
>|  2 +-
>  src/main/python/apache/aurora/client/api/__init__.py 
>|  4 ++--
>  src/main/python/apache/aurora/client/cli/client.py   
>|  2 +-
>  src/main/python/apache/aurora/client/cli/cron.py 
>|  2 +-
>  src/main/python/apache/thermos/core/process.py   
>|  6 +++---
>  src/main/python/apache/thermos/monitoring/process_collector_psutil.py
>|  1 -
>  src/test/python/apache/aurora/admin/test_admin.py
>|  6 --
>  src/test/python/apache/aurora/admin/util.py  
>|  2 +-
>  src/test/python/apache/aurora/client/cli/test_task.py
>|  3 +--
>  21 files changed, 111 insertions(+), 127 deletions(-)
> 
> 
> Diffs
> -
> 
>   build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea 
>   build-support/jenkins/build.sh

Re: Review Request 40204: Update pants bootstrap script to be agnostic to sed version

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 11, 2015, 3:39 p.m., Joe Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40204/
> ---
> 
> (Updated Nov. 11, 2015, 3:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update pants bootstrap script to be agnostic to sed version
> 
> 
> Diffs
> -
> 
>   pants 47097994b1044202aa0d8ce6afb8c2dee2a4c27c 
> 
> Diff: https://reviews.apache.org/r/40204/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joe Smith
> 
>

Re: Review Request 39784: Upgrade Aurora to pants 0.0.57.

[Zameer Manji]

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

Ship it!


Thanks John!

- Zameer Manji


On Nov. 10, 2015, 2:58 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39784/
> ---
> 
> (Updated Nov. 10, 2015, 2:58 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1499
> https://issues.apache.org/jira/browse/AURORA-1499
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See the changelog here:
>   https://pypi.python.org/pypi/pantsbuild.pants/0.0.57
>   
> This also brings Aurora up to pex 1.1.0 and switches to the pants
> setup script; an equivalent to gradlew.  Of note, the script is checked
> in with chmod 555, its not intended to be edited.
> 
> Although pants now includes checkstyle built in, conversion to use it is
> left for follow-on work.
> 
> Also adapt make-pycharm-virtualenv which previously relied on the local
> bootstrapping of pants - now hidden away by the pants setup script.
> 
>  .pantsversion  |   1 -
>  3rdparty/python/requirements.txt   |   2 +-
>  BUILD  |  25 -
>  BUILD.tools|  19 --
>  build-support/jenkins/build.sh |   2 +-
>  build-support/pants_requirements.txt   |  30 ---
>  build-support/python/make-pycharm-virtualenv   |  11 +-
>  build-support/python/update-pants-requirements |  34 -
>  pants  | 102 
> +++---
>  pants.ini  |  63 
> ++-
>  src/main/python/apache/aurora/tools/BUILD  |   2 +-
>  src/main/python/apache/thermos/observer/BUILD  |   2 +-
>  12 files changed, 114 insertions(+), 179 deletions(-)
> 
> 
> Diffs
> -
> 
>   .auroraversion d2cbead89e9c31b9fb31db9a645afb99ff585b10 
>   .auroraversion PRE-CREATION 
>   .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 
>   3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee 
>   BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 
>   BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> 6b13949aceb830b962a7098ae65462151119d752 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/pants_requirements.txt 
> 6eefa7175f7bda7bea23466936e451113a2d86f7 
>   build-support/python/make-pycharm-virtualenv 
> d7bd41835bcd9a8fe62ea522a177bfa7830a897b 
>   build-support/python/update-pants-requirements 
> 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec 
>   build-support/release/release 209045767a18d263814700dfb5f9912447cc1205 
>   build-support/release/release-candidate 
> 1caa56bfaee67dec6c70b50923b18900f88ced75 
>   pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 
>   pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 
>   src/main/python/apache/aurora/client/BUILD 
> 8424237eb56abe38c4d0871557dc929250de1ba4 
>   src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION 
>   src/main/python/apache/aurora/tools/BUILD 
> e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 
>   src/main/python/apache/thermos/observer/BUILD 
> d7eedabc0930711530b45ac98a1159e69d1a0c00 
>   src/main/resources/apache/aurora/client/cli/.auroraversion 
> 67ff1ac747879c27d7db8a84994ffd254e830a24 
>   src/main/resources/apache/aurora/client/cli/BUILD 
> 7c77d10823753af56db01a3b0db49868451d6435 
> 
> Diff: https://reviews.apache.org/r/39784/diff/
> 
> 
> Testing
> ---
> 
> Locally: `./pants test.pytest --no-fast src/test/python:: -- -v`
> 
> Also generated a pycharm project via:
>   `./build-support/python/make-pycharm-virtualenv`
> Confirmed library source linking worked as did running unit tests
> via the IDE.
> 
> Also grepped for pants commands in the repo, found `binary` and `setup-py`
> and confirmed these worked.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 39784: Upgrade Aurora to pants 0.0.57.

[Zameer Manji]

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


Moving the `.auroraversion` file breaks the `build-support/release/release` and 
`build-support/release/release-candidate` scripts. John, I think updating those 
scripts to point to the new location is fine, but I am unsure on the side 
effects. Bill can you comment if there be any ill side effects from doing that?

Overall this change LGTM, but I am holding back a ship because as it stands it 
will break the release scripts we have.

- Zameer Manji


On Nov. 10, 2015, 1:50 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39784/
> ---
> 
> (Updated Nov. 10, 2015, 1:50 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1499
> https://issues.apache.org/jira/browse/AURORA-1499
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> See the changelog here:
>   https://pypi.python.org/pypi/pantsbuild.pants/0.0.57
>   
> This also brings Aurora up to pex 1.1.0 and switches to the pants
> setup script; an equivalent to gradlew.  Of note, the script is checked
> in with chmod 555, its not intended to be edited.
> 
> Although pants now includes checkstyle built in, conversion to use it is
> left for follow-on work.
> 
> Also adapt make-pycharm-virtualenv which previously relied on the local
> bootstrapping of pants - now hidden away by the pants setup script.
> 
>  .pantsversion  |   1 -
>  3rdparty/python/requirements.txt   |   2 +-
>  BUILD  |  25 -
>  BUILD.tools|  19 --
>  build-support/jenkins/build.sh |   2 +-
>  build-support/pants_requirements.txt   |  30 ---
>  build-support/python/make-pycharm-virtualenv   |  11 +-
>  build-support/python/update-pants-requirements |  34 -
>  pants  | 102 
> +++---
>  pants.ini  |  63 
> ++-
>  src/main/python/apache/aurora/tools/BUILD  |   2 +-
>  src/main/python/apache/thermos/observer/BUILD  |   2 +-
>  12 files changed, 114 insertions(+), 179 deletions(-)
> 
> 
> Diffs
> -
> 
>   .auroraversion d2cbead89e9c31b9fb31db9a645afb99ff585b10 
>   .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 
>   3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee 
>   BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 
>   BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> 6b13949aceb830b962a7098ae65462151119d752 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/pants_requirements.txt 
> 6eefa7175f7bda7bea23466936e451113a2d86f7 
>   build-support/python/make-pycharm-virtualenv 
> d7bd41835bcd9a8fe62ea522a177bfa7830a897b 
>   build-support/python/update-pants-requirements 
> 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec 
>   pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 
>   pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 
>   src/main/python/apache/aurora/client/BUILD 
> 8424237eb56abe38c4d0871557dc929250de1ba4 
>   src/main/python/apache/aurora/tools/BUILD 
> e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 
>   src/main/python/apache/thermos/observer/BUILD 
> d7eedabc0930711530b45ac98a1159e69d1a0c00 
>   src/main/resources/apache/aurora/client/cli/.auroraversion  
>   src/main/resources/apache/aurora/client/cli/BUILD 
> 7c77d10823753af56db01a3b0db49868451d6435 
> 
> Diff: https://reviews.apache.org/r/39784/diff/
> 
> 
> Testing
> ---
> 
> Locally: `./pants test.pytest --no-fast src/test/python:: -- -v`
> 
> Also generated a pycharm project via:
>   `./build-support/python/make-pycharm-virtualenv`
> Confirmed library source linking worked as did running unit tests
> via the IDE.
> 
> Also grepped for pants commands in the repo, found `binary` and `setup-py`
> and confirmed these worked.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 40149: Create ExecutorSettings creation closer to command line arguments.

[Zameer Manji]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (line 227)
<https://reviews.apache.org/r/40149/#comment164662>

Is there a reason why we are not using Java's URI type to do this sort of 
parsing?


- Zameer Manji


On Nov. 10, 2015, 12:42 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40149/
> ---
> 
> (Updated Nov. 10, 2015, 12:42 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is prep work for https://issues.apache.org/jira/browse/AURORA-1376
> 
> The high-level goal is to push ExecutorInfo creation closer to command line 
> args so they can be fully specified there in subsequent changes.  A lot of 
> the changes here are code shuffle, but there is one behavior change in 
> `MesosTaskFactory`.  There is currently a dance where `RESOURCES_EPSILON` is 
> used to effectively account all resources under the task.  With this change, 
> the executor resources are used verbatim and kept distinct from the task's 
> resources.
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt b77265ca1c89f9a2d7e6e01dea9f04b200e4ef6b 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> c210c0db07bb1f4b3f76668178dcd7e2de56a4ac 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 197184b6edc0768d677636341b5737f262abdf7d 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> c665c3237a03f0fadf8ca158b97cad14dd1eea81 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 99b874459c358c67234c6fe748e6427f313a04a8 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> aa5ce8b2f14c7dbd0eae120018ee41387c26059f 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> fb0f7ccf7f4dfb3ccb2a4cb4a5643c4b5487ff8f 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorConfig.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/Executors.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> f6ba2c40aea555d3e0ab774218bfe08d7e1c984b 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  67d7f072de045d4211b6239119802473278d6419 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 6fad3344042dc6a75cdf74ce79d388fcd4fc9861 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> f63d6f12d114c062f81137ffc0b1078837e3cf76 
>   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
> cd0295780d41bc4e914583f195b37eaed28a46dc 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  0576704f6bcdc2a7568ee1597613f71f33e092d9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> dddf7952d3f0e508cd736d5fb95e573267708d43 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> d0987251b058988fcbfab16c1b138c37e0c5b8c6 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  8ce8da88fe705bfb9846a790f76bfeb6b4af2a06 
> 
> Diff: https://reviews.apache.org/r/40149/diff/
> 
> 
> Testing
> ---
> 
> End-to-end tests pass.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 39958: deleting unused function deprecation_warning adding test coverage to base, job_monitor, quota_check, scheduler_client

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Nov. 4, 2015, 5:26 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39958/
> ---
> 
> (Updated Nov. 4, 2015, 5:26 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1536
> https://issues.apache.org/jira/browse/AURORA-1536
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> deleting unused function deprecation_warning
> adding test coverage to base, job_monitor, quota_check, scheduler_client
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/base.py 
> 487f8e73fa978b150f2e5b60e0f64c71ce783c12 
>   src/test/python/apache/aurora/client/BUILD 
> dc7912e3a146659d78e121d8865e0995179e 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> ccc8b551c15df2d726291675a90a62c30ad5ebd4 
>   src/test/python/apache/aurora/client/api/test_quota_check.py 
> 7035bb5e11ee3fd70e93065082698f529f8edbf7 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 44671fa07bca7e7296dc4922a3625336c34be787 
>   src/test/python/apache/aurora/client/test_base.py 
> fa5eb07c8e71475d3547c240435a4985faf8004b 
> 
> Diff: https://reviews.apache.org/r/39958/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 40043: Adding GPG key for zma...@apache.org.

[Zameer Manji]

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

(Updated Nov. 6, 2015, 5:14 p.m.)


Review request for Aurora, Jake Farrell and Bill Farner.


Repository: aurora


Description (updated)
---

Adding GPG key for zma...@apache.org.

Once this lands I will update the KEYS file in the apache dist repo at
   https://dist.apache.org/repos/dist/dev/aurora/KEYS
   https://dist.apache.org/repos/dist/release/aurora/KEYS


Diffs
-

  KEYS dff0fb48f1b0e8a18bc21014cc425d7d48f08c6d 

Diff: https://reviews.apache.org/r/40043/diff/


Testing
---

None. Please verify my key is correct.


Thanks,

Zameer Manji

Re: Review Request 39822: upgrade psutil to 3.2.2

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Oct. 30, 2015, 1:14 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39822/
> ---
> 
> (Updated Oct. 30, 2015, 1:14 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1531
> https://issues.apache.org/jira/browse/AURORA-1531
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> upgrade psutil to 3.2.2
> psutil changelog:
> https://github.com/giampaolo/psutil/blob/36316de315a55dece354cf98533e647d7f62c49b/HISTORY.rst
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 
>   build-support/pants_requirements.txt 
> fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 
> 
> Diff: https://reviews.apache.org/r/39822/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 39822: upgrade psutil to 3.2.2

[Zameer Manji]

> On Nov. 1, 2015, 8:59 a.m., Bill Farner wrote:
> > Ship It!
> 
> Dmitriy Shirchenko wrote:
> sweet, if there are no other objections, can someone land this for me, 
> please?

Maxim is the last reviewer, so once he ships this he will land it.


- Zameer


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


On Oct. 30, 2015, 1:14 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39822/
> ---
> 
> (Updated Oct. 30, 2015, 1:14 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1531
> https://issues.apache.org/jira/browse/AURORA-1531
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> upgrade psutil to 3.2.2
> psutil changelog:
> https://github.com/giampaolo/psutil/blob/36316de315a55dece354cf98533e647d7f62c49b/HISTORY.rst
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 
>   build-support/pants_requirements.txt 
> fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 
> 
> Diff: https://reviews.apache.org/r/39822/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 39822: upgrade psutil to 3.2.2

[Zameer Manji]

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


Can you please link to the psutil changelog in the commit message and highlight 
any API changes we might need to be aware of?

- Zameer Manji


On Oct. 30, 2015, 11:17 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39822/
> ---
> 
> (Updated Oct. 30, 2015, 11:17 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1531
> https://issues.apache.org/jira/browse/AURORA-1531
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> revert --virtualbox arg
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 
>   build-support/pants_requirements.txt 
> fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 
> 
> Diff: https://reviews.apache.org/r/39822/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 39670: Modify ClusterStateImpl to be thread safe.

[Zameer Manji]

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

(Updated Oct. 29, 2015, 1:44 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Bill's feedback.


Bugs: AURORA-1510
https://issues.apache.org/jira/browse/AURORA-1510


Repository: aurora


Description
---

ClusterStateImpl exposes a synchronized multimap which does not have a 
thread-safe implementation of `keySet`. This patch revises the `ClusterState` 
interface to offer more precise operations and modifies `ClusterStateImpl` to 
makes all of these operations thread safe.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java 
42e2ca49b3c5de997cb5363e3518512d0f5a725e 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
a1ac922d471013779710e02c0c9ca9f84b506807 

Diff: https://reviews.apache.org/r/39670/diff/


Testing
---

./gradlew build -Pq

If the approach is good, I can update this with benchmark information.


Thanks,

Zameer Manji