Re: Review Request 35901: Add a DbCronJobStore implementation.

2015-06-25 Thread Aurora ReviewBot

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

Ship it!


Master (56bb1e6) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 26, 2015, 2:13 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35901/
> ---
> 
> (Updated June 26, 2015, 2:13 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-415
> https://issues.apache.org/jira/browse/AURORA-415
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This pulled a longer thread than i had hoped, but rest assured that the diff 
> is exaggerated due to some code moves.
> 
> I'll leave comments in specific places of note.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 8bfb076a80face2703045800cf5530f37d64269f 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> bb61542f4043847b1c8c92ff1b4a0ecfcfc88973 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> a5acb3ae2ca5ecfd7d470fcd02de6091d66a1694 
>   src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 
> 44dc8f5e3dcc91e80a03d980c5d8ae0db65c8b89 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 46fa940b9cdf0544fa33b76e87ad21045e8bcdbb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
>  0a519be65f90cb730f6bda1e6d7b019f0f15252b 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java
>  b89e7b5463cdb9ff9d1f9106dda0c0c4908225ca 
>   
> src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
> 35c83b9b8838a00693c8ebc96e496886ca249ed1 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> f9e9e89a52c9bce3dd7e5a727498a7c87c26a56c 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> 80ff3472c768cd116770e887b68e70d2ea3c9a8d 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> e9660b732bdc97117ee467fc124e5a014f176c67 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 0f77db7397866c008d32ea6f55c5bb577fe6468f 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> b9e16578b27de2985d24c25aae507b3540fcd3ff 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 863e9c998c97506759a5526135a33a161a8fb75f 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 2d74b326831fbe22fa6045610ca8d714cd64779e 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 5ad0de7c6a648f5eb6408eea7bcd789c25d55f88 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbCronJobStoreTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  31feaea7ba74350fc199333a2384419ec05f1816 
>   
> src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 
> 2ed748383f269217860b80c831a3521facba83ba 
>   
> src/test/java/org/apache/aurora/scheduler/storage/mem/MemCronJobStoreTest.java
>  58256afafd12e5de234755969605861891af4daf 
> 
> Diff: https://reviews.apache.org/r/35901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 35901: Add a DbCronJobStore implementation.

2015-06-25 Thread Bill Farner

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


Adding reviewer notes.


src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 


This check is made superfluous now that `ScheduledTaskWrapper` extends 
`InsertResult`.



src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 


This moved to `TaskConfigManager`.



src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 


Also moved to `TaskConfigManager`.



src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
(line 26)


We don't actually fill an unset cronCollisionPolicy with the default, but 
handle `null` in the consuming code.  I opted to maintain this behavior.



src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
(line 72)


This test validated bad data coming from storage, which is no longer 
possible.



src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
(line 43)


The test cases in here are unchanged from the original, but the delta in 
the file causes git to not track it as a move.



src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
(line 49)


This test case relates to AURORA-749 [1].  While we are not ready to remove 
the `Identity` struct as that ticket calls for, we are now at a release where 
we can expect `JobConfiguration.taskConfig.job` to be set.

I opted to change behavior here to avoid an if/else in the mapper code.

[1] https://issues.apache.org/jira/browse/AURORA-749


- Bill Farner


On June 26, 2015, 2:13 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35901/
> ---
> 
> (Updated June 26, 2015, 2:13 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-415
> https://issues.apache.org/jira/browse/AURORA-415
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This pulled a longer thread than i had hoped, but rest assured that the diff 
> is exaggerated due to some code moves.
> 
> I'll leave comments in specific places of note.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 8bfb076a80face2703045800cf5530f37d64269f 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> bb61542f4043847b1c8c92ff1b4a0ecfcfc88973 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> a5acb3ae2ca5ecfd7d470fcd02de6091d66a1694 
>   src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 
> 44dc8f5e3dcc91e80a03d980c5d8ae0db65c8b89 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 46fa940b9cdf0544fa33b76e87ad21045e8bcdbb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
>  0a519be65f90cb730f6bda1e6d7b019f0f15252b 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java
>  b89e7b5463cdb9ff9d1f9106dda0c0c4908225ca 
>   
> src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
> 35c83b9b8838a00693c8ebc96e496886ca249ed1 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> f9e9e89a52c9bce3dd7e5a727498a7c87c26a56c 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> 80ff3472c768cd116770e887b68e70d2ea3c9a8d 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> e9660b732bdc97117ee467fc124e5a014f176c67 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 0f77db7397866c008d32ea6f55c5bb577fe6468f 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> b9e16578b27de2985d24c25aae507b3540fcd3ff 
>   src/test/java/org/apache/aurora/schedu

Review Request 35901: Add a DbCronJobStore implementation.

2015-06-25 Thread Bill Farner

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

Review request for Aurora and Maxim Khutornenko.


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


Repository: aurora


Description
---

This pulled a longer thread than i had hoped, but rest assured that the diff is 
exaggerated due to some code moves.

I'll leave comments in specific places of note.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
8bfb076a80face2703045800cf5530f37d64269f 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
bb61542f4043847b1c8c92ff1b4a0ecfcfc88973 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
a5acb3ae2ca5ecfd7d470fcd02de6091d66a1694 
  src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 
44dc8f5e3dcc91e80a03d980c5d8ae0db65c8b89 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
46fa940b9cdf0544fa33b76e87ad21045e8bcdbb 
  
src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
 0a519be65f90cb730f6bda1e6d7b019f0f15252b 
  
src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java
 b89e7b5463cdb9ff9d1f9106dda0c0c4908225ca 
  src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
35c83b9b8838a00693c8ebc96e496886ca249ed1 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
f9e9e89a52c9bce3dd7e5a727498a7c87c26a56c 
  src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
PRE-CREATION 
  src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
80ff3472c768cd116770e887b68e70d2ea3c9a8d 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
e9660b732bdc97117ee467fc124e5a014f176c67 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
0f77db7397866c008d32ea6f55c5bb577fe6468f 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
b9e16578b27de2985d24c25aae507b3540fcd3ff 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
863e9c998c97506759a5526135a33a161a8fb75f 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
2d74b326831fbe22fa6045610ca8d714cd64779e 
  
src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
5ad0de7c6a648f5eb6408eea7bcd789c25d55f88 
  src/test/java/org/apache/aurora/scheduler/storage/db/DbCronJobStoreTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
 31feaea7ba74350fc199333a2384419ec05f1816 
  src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 
2ed748383f269217860b80c831a3521facba83ba 
  
src/test/java/org/apache/aurora/scheduler/storage/mem/MemCronJobStoreTest.java 
58256afafd12e5de234755969605861891af4daf 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On June 26, 2015, 12:07 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 26, 2015, 12:07 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS 1a0fb48c3bed1761937e98fa528fac39f2bdc05b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  9ee4fe2e76d36e17f8de2ab3eb714a6aae52c09c 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   src/test/java/org/apache/aurora/scheduler/mesos/Offers.java 
> 83eec5d3c8e493fcefbcb5b9cf67dae4e741b095 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  71b09b1fddc5faa7960ade43929cb57eec3243dd 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Aurora ReviewBot

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

Ship it!


Master (56bb1e6) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 26, 2015, 12:07 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 26, 2015, 12:07 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS 1a0fb48c3bed1761937e98fa528fac39f2bdc05b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  9ee4fe2e76d36e17f8de2ab3eb714a6aae52c09c 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   src/test/java/org/apache/aurora/scheduler/mesos/Offers.java 
> 83eec5d3c8e493fcefbcb5b9cf67dae4e741b095 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  71b09b1fddc5faa7960ade43929cb57eec3243dd 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Zameer Manji


> On June 25, 2015, 9:38 a.m., Bill Farner wrote:
> > NEWS, line 5
> > 
> >
> > ```
> > The scheduler command line argument enable_legacy_constraints has been 
> > removed, and the scheduler no longer automatically injects 'host' and 
> > 'rack' constraints for production services.
> > ```

Done.


- Zameer


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


On June 25, 2015, 5:07 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 25, 2015, 5:07 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS 1a0fb48c3bed1761937e98fa528fac39f2bdc05b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  9ee4fe2e76d36e17f8de2ab3eb714a6aae52c09c 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   src/test/java/org/apache/aurora/scheduler/mesos/Offers.java 
> 83eec5d3c8e493fcefbcb5b9cf67dae4e741b095 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  71b09b1fddc5faa7960ade43929cb57eec3243dd 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Zameer Manji


> On June 25, 2015, 10:40 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java,
> >  lines 75-76
> > 
> >
> > AFAICT, these are now only referenced in test now. Remove?

Good catch. I removed them.


- Zameer


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


On June 25, 2015, 5:07 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 25, 2015, 5:07 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS 1a0fb48c3bed1761937e98fa528fac39f2bdc05b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  9ee4fe2e76d36e17f8de2ab3eb714a6aae52c09c 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   src/test/java/org/apache/aurora/scheduler/mesos/Offers.java 
> 83eec5d3c8e493fcefbcb5b9cf67dae4e741b095 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  71b09b1fddc5faa7960ade43929cb57eec3243dd 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Zameer Manji

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

(Updated June 25, 2015, 5:07 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Feedback.


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


Repository: aurora


Description
---

Remove the "enable_legacy_constraints" flag and associated behaviour.


Diffs (updated)
-

  NEWS 1a0fb48c3bed1761937e98fa528fac39f2bdc05b 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b77b0ebbf303778e528b16ff3db1aa4e76f1 
  
src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
 9ee4fe2e76d36e17f8de2ab3eb714a6aae52c09c 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 abbd23dd3ee4382565ce846eb035e2aa502badae 
  src/test/java/org/apache/aurora/scheduler/mesos/Offers.java 
83eec5d3c8e493fcefbcb5b9cf67dae4e741b095 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 71b09b1fddc5faa7960ade43929cb57eec3243dd 

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


Testing
---

./gradlew build -Pq


Thanks,

Zameer Manji



Re: Review Request 35886: Remove static modifier from test case.

2015-06-25 Thread Aurora ReviewBot

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

Ship it!


Master (ca15f25) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 25, 2015, 6:50 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35886/
> ---
> 
> (Updated June 25, 2015, 6:50 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove static modifier from test case, which was introduced in the last 
> revision to https://reviews.apache.org/r/35842/
> 
> This fixes
> 
> [ant:checkstyle] 
> /home/ksweeney/workspace/aurora/src/test/java/org/apache/aurora/GuavaUtilsTest.java:23:1:
>  Utility classes should not have a public or default constructor.
> 
> 
> Diffs
> -
> 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java 
> 065a4bac48b7264c1c6aaf228c684f813f2a39ea 
> 
> Diff: https://reviews.apache.org/r/35886/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35886: Remove static modifier from test case.

2015-06-25 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On June 25, 2015, 6:50 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35886/
> ---
> 
> (Updated June 25, 2015, 6:50 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove static modifier from test case, which was introduced in the last 
> revision to https://reviews.apache.org/r/35842/
> 
> This fixes
> 
> [ant:checkstyle] 
> /home/ksweeney/workspace/aurora/src/test/java/org/apache/aurora/GuavaUtilsTest.java:23:1:
>  Utility classes should not have a public or default constructor.
> 
> 
> Diffs
> -
> 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java 
> 065a4bac48b7264c1c6aaf228c684f813f2a39ea 
> 
> Diff: https://reviews.apache.org/r/35886/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35886: Remove static modifier from test case.

2015-06-25 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On June 25, 2015, 11:50 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35886/
> ---
> 
> (Updated June 25, 2015, 11:50 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove static modifier from test case, which was introduced in the last 
> revision to https://reviews.apache.org/r/35842/
> 
> This fixes
> 
> [ant:checkstyle] 
> /home/ksweeney/workspace/aurora/src/test/java/org/apache/aurora/GuavaUtilsTest.java:23:1:
>  Utility classes should not have a public or default constructor.
> 
> 
> Diffs
> -
> 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java 
> 065a4bac48b7264c1c6aaf228c684f813f2a39ea 
> 
> Diff: https://reviews.apache.org/r/35886/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Bill Farner


> On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java,
> >  line 60
> > 
> >
> > I'd still recommend having a run counter to help us monitor collector 
> > availability/health.
> 
> Bill Farner wrote:
> Does the comment above change your opinion?  If not, perhaps a more 
> cross-cutting approach is stats on the states of all Services?
> 
> Maxim Khutornenko wrote:
> > If not, perhaps a more cross-cutting approach is stats on the states of 
> all Services?
> 
> That would be ideal. Happy to accept a TODO.

Filed https://issues.apache.org/jira/browse/AURORA-1373


- Bill


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


On June 24, 2015, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35793/
> ---
> 
> (Updated June 24, 2015, 10:46 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1298
> https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> c31446c447c3385a4763b8a516827988e46cc480 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 11c9c4ada400d51fc83e9e0de03108456be15fdf 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java 
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 
>   src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> f5829ac063272123995193caef5151e0d52d435b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35793/diff/
> 
> 
> Testing
> ---
> 
> Note that this removes a defensive branch wherein we checked for inbound 
> config references before attempting to delete.  With this change, we 
> proactively delete and count on foreign key constraints to prevent deletion 
> of rows that are still referenced.  I propose we adopt this as our pattern 
> for handling shared references, as it seems to be the most sane approach 
> available.
> 
> A gotcha with this case is that i do not believe mybatis provides a 
> vendor-neutral approach to identify a consistency violation, and the best 
> signal is a generic `PersistenceException`.  This is unfortunate since we 
> can't distinguish between a hopeless query with invalid syntax, a network 
> disconnection, or the anticipated case of a consistency violation.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Review Request 35886: Remove static modifier from test case.

2015-06-25 Thread Kevin Sweeney

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

Review request for Aurora, Bill Farner and Zameer Manji.


Repository: aurora


Description
---

Remove static modifier from test case, which was introduced in the last 
revision to https://reviews.apache.org/r/35842/

This fixes

[ant:checkstyle] 
/home/ksweeney/workspace/aurora/src/test/java/org/apache/aurora/GuavaUtilsTest.java:23:1:
 Utility classes should not have a public or default constructor.


Diffs
-

  src/test/java/org/apache/aurora/GuavaUtilsTest.java 
065a4bac48b7264c1c6aaf228c684f813f2a39ea 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney



Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On June 25, 2015, 11:32 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35842/
> ---
> 
> (Updated June 25, 2015, 11:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use java.util.Optional and streams in Lock*.
> 
> Taking advantage of some Java 8 features along the way, and introduced 
> GuavaCollectors.toImmutableSet for compatibility with the streams API
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 22104e6d005ae6706a06a21ffe4c730b14ecfe53 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> e8303f914466ebe93518ec2970e1173dc39d9adb 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 6aa281cb7b07e3e82c578c553eee8c7066cf2761 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 2be3eb0e9405e3d34533520095423429fe00a2ca 
>   src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
> 596a3787369616d30a2763a37bcb7b7cc39ad386 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  0670b2b4c3817becefb38f0294f111768386817b 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
> ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
> 
> Diff: https://reviews.apache.org/r/35842/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Aurora ReviewBot

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

Ship it!


Master (d28bd4f) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 25, 2015, 6:32 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35842/
> ---
> 
> (Updated June 25, 2015, 6:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use java.util.Optional and streams in Lock*.
> 
> Taking advantage of some Java 8 features along the way, and introduced 
> GuavaCollectors.toImmutableSet for compatibility with the streams API
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 22104e6d005ae6706a06a21ffe4c730b14ecfe53 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> e8303f914466ebe93518ec2970e1173dc39d9adb 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 6aa281cb7b07e3e82c578c553eee8c7066cf2761 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 2be3eb0e9405e3d34533520095423429fe00a2ca 
>   src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
> 596a3787369616d30a2763a37bcb7b7cc39ad386 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  0670b2b4c3817becefb38f0294f111768386817b 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
> ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
> 
> Diff: https://reviews.apache.org/r/35842/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On June 25, 2015, 6:32 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35842/
> ---
> 
> (Updated June 25, 2015, 6:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use java.util.Optional and streams in Lock*.
> 
> Taking advantage of some Java 8 features along the way, and introduced 
> GuavaCollectors.toImmutableSet for compatibility with the streams API
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 22104e6d005ae6706a06a21ffe4c730b14ecfe53 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> e8303f914466ebe93518ec2970e1173dc39d9adb 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 6aa281cb7b07e3e82c578c553eee8c7066cf2761 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 2be3eb0e9405e3d34533520095423429fe00a2ca 
>   src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
> 596a3787369616d30a2763a37bcb7b7cc39ad386 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  0670b2b4c3817becefb38f0294f111768386817b 
>   src/test/java/org/apache/aurora/GuavaUtilsTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
> ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
> 
> Diff: https://reviews.apache.org/r/35842/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Kevin Sweeney

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

(Updated June 25, 2015, 11:32 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Move GuavaCollectors to existing GuiceUtils class.


Repository: aurora


Description
---

Use java.util.Optional and streams in Lock*.

Taking advantage of some Java 8 features along the way, and introduced 
GuavaCollectors.toImmutableSet for compatibility with the streams API


Diffs (updated)
-

  src/main/java/org/apache/aurora/GuavaUtils.java 
22104e6d005ae6706a06a21ffe4c730b14ecfe53 
  src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
e8303f914466ebe93518ec2970e1173dc39d9adb 
  src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
6aa281cb7b07e3e82c578c553eee8c7066cf2761 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
2be3eb0e9405e3d34533520095423429fe00a2ca 
  src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
596a3787369616d30a2763a37bcb7b7cc39ad386 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
335d7a95e797fe940e71b10da44cbd97edea69ac 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
0670b2b4c3817becefb38f0294f111768386817b 
  src/test/java/org/apache/aurora/GuavaUtilsTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
  src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 38ef412a6d29dfef7b305e00cf44522818303965 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney



Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On June 24, 2015, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35793/
> ---
> 
> (Updated June 24, 2015, 10:46 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1298
> https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> c31446c447c3385a4763b8a516827988e46cc480 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 11c9c4ada400d51fc83e9e0de03108456be15fdf 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java 
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 
>   src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> f5829ac063272123995193caef5151e0d52d435b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35793/diff/
> 
> 
> Testing
> ---
> 
> Note that this removes a defensive branch wherein we checked for inbound 
> config references before attempting to delete.  With this change, we 
> proactively delete and count on foreign key constraints to prevent deletion 
> of rows that are still referenced.  I propose we adopt this as our pattern 
> for handling shared references, as it seems to be the most sane approach 
> available.
> 
> A gotcha with this case is that i do not believe mybatis provides a 
> vendor-neutral approach to identify a consistency violation, and the best 
> signal is a generic `PersistenceException`.  This is unfortunate since we 
> can't distinguish between a hopeless query with invalid syntax, a network 
> disconnection, or the anticipated case of a consistency violation.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko


> On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 254
> > 
> >
> > This will use a regular single threaded executor, right? Any chance to 
> > provide a logging executor instance instead?
> 
> Bill Farner wrote:
> The executor is not determined by this, just the interval.  See 
> `executor()` [1] for the executor being used.  AbstractScheduledService shuts 
> down and logs when exceptions are thrown, and our /services endpoint provides 
> visibility when a service dies.  Our other AbstractScheduledServices use this 
> approach.
> 
> [1] 
> http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/AbstractScheduledService.html#executor()

Yeah, looks like they are doing it right:
```
private final Runnable task = new Runnable() {
  @Override public void run() {
lock.lock();
try {
  AbstractScheduledService.this.runOneIteration();
} catch (Throwable t) {
  try {
shutDown();
  } catch (Exception ignored) {
logger.log(Level.WARNING, 
"Error while attempting to shut down the service after 
failure.", ignored);
  }
  notifyFailed(t);
  throw Throwables.propagate(t);
} finally {
  lock.unlock();
}
  }
};
```


> On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java,
> >  line 60
> > 
> >
> > I'd still recommend having a run counter to help us monitor collector 
> > availability/health.
> 
> Bill Farner wrote:
> Does the comment above change your opinion?  If not, perhaps a more 
> cross-cutting approach is stats on the states of all Services?

> If not, perhaps a more cross-cutting approach is stats on the states of all 
> Services?

That would be ideal. Happy to accept a TODO.


- Maxim


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


On June 24, 2015, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35793/
> ---
> 
> (Updated June 24, 2015, 10:46 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1298
> https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> c31446c447c3385a4763b8a516827988e46cc480 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 11c9c4ada400d51fc83e9e0de03108456be15fdf 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java 
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 
>   src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> f5829ac063272123995193caef5151e0d52d435b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  PRE-CREATION 
> 
> Diff: http

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Bill Farner


> On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 254
> > 
> >
> > This will use a regular single threaded executor, right? Any chance to 
> > provide a logging executor instance instead?

The executor is not determined by this, just the interval.  See `executor()` 
[1] for the executor being used.  AbstractScheduledService shuts down and logs 
when exceptions are thrown, and our /services endpoint provides visibility when 
a service dies.  Our other AbstractScheduledServices use this approach.

[1] 
http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/AbstractScheduledService.html#executor()


> On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java,
> >  line 60
> > 
> >
> > I'd still recommend having a run counter to help us monitor collector 
> > availability/health.

Does the comment above change your opinion?  If not, perhaps a more 
cross-cutting approach is stats on the states of all Services?


- Bill


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


On June 24, 2015, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35793/
> ---
> 
> (Updated June 24, 2015, 10:46 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1298
> https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> c31446c447c3385a4763b8a516827988e46cc480 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 11c9c4ada400d51fc83e9e0de03108456be15fdf 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java 
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 
>   src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> f5829ac063272123995193caef5151e0d52d435b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35793/diff/
> 
> 
> Testing
> ---
> 
> Note that this removes a defensive branch wherein we checked for inbound 
> config references before attempting to delete.  With this change, we 
> proactively delete and count on foreign key constraints to prevent deletion 
> of rows that are still referenced.  I propose we adopt this as our pattern 
> for handling shared references, as it seems to be the most sane approach 
> available.
> 
> A gotcha with this case is that i do not believe mybatis provides a 
> vendor-neutral approach to identify a consistency violation, and the best 
> signal is a generic `PersistenceException`.  This is unfortunate since we 
> can't distinguish between a hopeless query with invalid syntax, a network 
> disconnection, or the anticipated case of a consistency violation.
> 
> 
> Th

Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Kevin Sweeney

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

(Updated June 25, 2015, 11:12 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Add UNORDERED characteristic.


Repository: aurora


Description
---

Use java.util.Optional and streams in Lock*.

Taking advantage of some Java 8 features along the way, and introduced 
GuavaCollectors.toImmutableSet for compatibility with the streams API


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
e8303f914466ebe93518ec2970e1173dc39d9adb 
  src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
6aa281cb7b07e3e82c578c553eee8c7066cf2761 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
2be3eb0e9405e3d34533520095423429fe00a2ca 
  src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
596a3787369616d30a2763a37bcb7b7cc39ad386 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
335d7a95e797fe940e71b10da44cbd97edea69ac 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
0670b2b4c3817becefb38f0294f111768386817b 
  src/main/java/org/apache/aurora/util/GuavaCollectors.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
  src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 38ef412a6d29dfef7b305e00cf44522818303965 
  src/test/java/org/apache/aurora/util/GuavaCollectorsTest.java PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney



Re: Review Request 35842: Use java.util.Optional and streams in LockManager

2015-06-25 Thread Kevin Sweeney


> On June 24, 2015, 3:15 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/util/GuavaCollectors.java, line 36
> > 
> >
> > I think this collector needs to be given the unordered characteristic 
> > because it is building a set where the order will be lost: 
> > https://docs.oracle.com/javase/8/docs/api/java/util/stream/Collector.Characteristics.html#UNORDERED

Added.


> On June 24, 2015, 3:15 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 263
> > 
> >
> > I think we should be importing java.util.Optional at the top of the 
> > file and inlining use of com.google.common.base.Optional as necessary.
> > 
> > Now that we are on JDK 8 we should be prefering use of 
> > java.util.Optional. If that change is very noisey, can you check how 
> > difficult it would be to remove guava's Optional in these files?

This is the lower blast-radius change - this file is almost 1400 lines, and 
many of the APIs it calls are still written in terms of guava Optional.


- Kevin


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


On June 24, 2015, 2:52 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35842/
> ---
> 
> (Updated June 24, 2015, 2:52 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use java.util.Optional and streams in Lock*.
> 
> Taking advantage of some Java 8 features along the way, and introduced 
> GuavaCollectors.toImmutableSet for compatibility with the streams API
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/state/LockManager.java 
> e8303f914466ebe93518ec2970e1173dc39d9adb 
>   src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java 
> 6aa281cb7b07e3e82c578c553eee8c7066cf2761 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 2be3eb0e9405e3d34533520095423429fe00a2ca 
>   src/main/java/org/apache/aurora/scheduler/storage/LockStore.java 
> 596a3787369616d30a2763a37bcb7b7cc39ad386 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  0670b2b4c3817becefb38f0294f111768386817b 
>   src/main/java/org/apache/aurora/util/GuavaCollectors.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 9c9cf1b45506a2c0e1a15e3e92f1bee83778a12a 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 
> ecbe1dd1eb1f0de07bea490b076c7a920f08bccb 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
>   src/test/java/org/apache/aurora/util/GuavaCollectorsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35842/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko

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


Looks great! Just a couple suggestions below.


src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java (line 254)


This will use a regular single threaded executor, right? Any chance to 
provide a logging executor instance instead?



src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
(line 60)


I'd still recommend having a run counter to help us monitor collector 
availability/health.


- Maxim Khutornenko


On June 24, 2015, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35793/
> ---
> 
> (Updated June 24, 2015, 10:46 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1298
> https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> c31446c447c3385a4763b8a516827988e46cc480 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 11c9c4ada400d51fc83e9e0de03108456be15fdf 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 
> 335d7a95e797fe940e71b10da44cbd97edea69ac 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java 
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 
>   src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 
> f5829ac063272123995193caef5151e0d52d435b 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35793/diff/
> 
> 
> Testing
> ---
> 
> Note that this removes a defensive branch wherein we checked for inbound 
> config references before attempting to delete.  With this change, we 
> proactively delete and count on foreign key constraints to prevent deletion 
> of rows that are still referenced.  I propose we adopt this as our pattern 
> for handling shared references, as it seems to be the most sane approach 
> available.
> 
> A gotcha with this case is that i do not believe mybatis provides a 
> vendor-neutral approach to identify a consistency violation, and the best 
> signal is a generic `PersistenceException`.  This is unfortunate since we 
> can't distinguish between a hopeless query with invalid syntax, a network 
> disconnection, or the anticipated case of a consistency violation.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 35847: Split http lifecycle into a composition layer.

2015-06-25 Thread Brian Wickman


> On June 25, 2015, 10:14 a.m., Brian Brazil wrote:
> > docs/configuration-reference.md, line 449
> > 
> >
> > If we're making this configurable, I think that we should make it apply 
> > to the healthcheck config too.
> > 
> > Does the HealthCheckConfig also belong in lifecycle? I'd consider them 
> > pretty strongly related.

Coupling them together has always bothered me.  The code responsible for health 
check and the code responsible for lifecycle are two separate modules; I can 
totally see wanting one and not the other (e.g. we have customers who really 
don't want health checking because of its tendency to kill all instances 
simultaneously, while they still want the ability to gracefully drain 
connections during a rolling update.)  Making the port configurable is 
necessary to do this.  The only reason it's "health" by default is for 
backwards compatibility.


- Brian


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


On June 24, 2015, 10:45 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35847/
> ---
> 
> (Updated June 24, 2015, 10:45 p.m.)
> 
> 
> Review request for Aurora, Brian Brazil and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1368
> https://issues.apache.org/jira/browse/AURORA-1368
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Move shutdown endpoints to the Job config since the lifecycle is controlled 
> by Aurora and not Thermos.
> Split the lifecycle management into a composition layer that can more readily 
> be tested.
> 
> Also, derp, just realized I did not update the documentation.  Revision 
> forthcoming.
> (Also comment on the 'union' style here -- not sure what is preferable.)
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 7bfd63381f54b0fe5ef6a4f17b825049b19038db 
>   src/main/python/apache/aurora/config/schema/base.py 
> 9a6f8a16f85c324ec75352710e19249443bf2c6b 
>   src/main/python/apache/aurora/config/thrift.py 
> 0a3e91011eccf8573feb296bd7f72913622e0ce0 
>   src/main/python/apache/aurora/executor/BUILD 
> cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
>   src/main/python/apache/aurora/executor/http_lifecycle.py PRE-CREATION 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7bcd6c42f50665eac2e7f17b84e63f2ea7fb4d4f 
>   src/main/python/apache/thermos/config/schema_base.py 
> a85def9eea25fa01020ca2dda4e9cefe861c4a5f 
>   src/test/python/apache/aurora/executor/BUILD 
> f415ecc77022b34f053c35272d004e133803d702 
>   src/test/python/apache/aurora/executor/common/fixtures.py 
> 37d032beb66a67cfd3cfcea272747a2915a745ff 
>   src/test/python/apache/aurora/executor/test_http_lifecycle.py PRE-CREATION 
>   src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
> 3569a6abf84d5144d2e268b0a86c82285ffe2b2b 
> 
> Diff: https://reviews.apache.org/r/35847/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>



Re: Review Request 35498: Compute SLA stats for non-prod jobs

2015-06-25 Thread Kevin Sweeney

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

Ship it!



src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java (lines 129 
- 130)


Add getters for these fields and access them below via the getters rather 
than direct field access.


- Kevin Sweeney


On June 24, 2015, 12:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> ---
> 
> (Updated June 24, 2015, 12:09 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1350
> https://issues.apache.org/jira/browse/AURORA-1350
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Compute SLA stats for non-prod jobs
> 
> This is a first iteration closely following the design proposal of Maxim as 
> posted on the mailinglist. Feedback welcome.
> 
> 
> Diffs
> -
> 
>   NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
>   docs/sla.md 14e9108fda91200bbf56384c96b9cd926689311f 
>   src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java 
> 82f36d5ca15df18bdc8ebbbd868d3394db38e603 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> ff73ca6265bd0699791da5e5b6ed4aab9156d9e4 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java 
> 64e986fb2e0f955dd4a9c7824eac9494728bf24e 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> cb98834e925793fc116814371548a30470830164 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java 
> 5ee123a03e3c8670e0c03b05c48a9f4c66f6af9d 
> 
> Diff: https://reviews.apache.org/r/35498/diff/
> 
> 
> Testing
> ---
> 
> `./gradlew -Pq build` and a manual verification in Vagrant.
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Maxim Khutornenko

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



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (lines 69 - 70)


AFAICT, these are now only referenced in test now. Remove?


- Maxim Khutornenko


On June 23, 2015, 11:58 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 23, 2015, 11:58 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 34337: Add Docker Parameters

2015-06-25 Thread Bill Farner

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


LGTM in general!  Biggest blocker for me is ability to toggle this behavior off.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 207 - 213)


Move this down below `MesosContainer` so it's closer to `DockerContainer`.

I haven't followed the wiring into mesos, but i assume these are docker 
command line args?  If so, it would be handy to call that out explicitly and 
include a link [1] in our docs.

[1] https://docs.docker.com/reference/commandline/cli/



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
186 - 195)


Similar to the `allowed_container_types` argument used in 
`ConfigurationManager`, we need a command line argument to disable this, as it 
poses a security risk on a shared cluster.



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
(line 175)


s/final //



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
(line 177)


Consider asserting on the whole list rather than a single value:
```
assertEquals(
ImmutableList.of(...),
docker.getParametersList());
```


- Bill Farner


On June 20, 2015, 11:42 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34337/
> ---
> 
> (Updated June 20, 2015, 11:42 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Support Arbitrary Docker Parameters in DockerContainer
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d740a90e7732f42b43a79f8cf0afe705c061539c 
>   docs/configuration-reference.md 7bfd63381f54b0fe5ef6a4f17b825049b19038db 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> e934f570e4a728470408970485abe0809487d312 
>   src/main/python/apache/aurora/config/schema/base.py 
> ec9f983564516afe542ab277d987d4d391f87e45 
>   src/main/python/apache/aurora/config/thrift.py 
> 810febb637d168b07c4aea77984e1d1451a39af2 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 1b2a7948ebb946a2e12b0eded6acf4ce3c8e20f9 
> 
> Diff: https://reviews.apache.org/r/34337/diff/
> 
> 
> Testing
> ---
> 
> Used Docker as the container of a Job. Included volumes and label parameters 
> which are correctly picked up by mesos when starting the task. The docker 
> container gets the specified label and bind mounts the volumes correctly. 
> I've been running multiple PostgreSQL databases docker containers for several 
> weeks deploying them as aurora jobs.
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 35813: Removing GC executor code.

2015-06-25 Thread Aurora ReviewBot

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

Ship it!


Master (d28bd4f) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 25, 2015, 4:59 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35813/
> ---
> 
> (Updated June 25, 2015, 4:59 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1333
> https://issues.apache.org/jira/browse/AURORA-1333
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removing GC executor code.
> 
> 
> Diffs
> -
> 
>   NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
>   api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
> a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
>   debian/aurora-executor.install 8efb1308caf64a23bed4b580de4b86e7982539e8 
>   debian/rules 6ba18cef7fbf0989507d630a1041cdf958742617 
>   docs/test-resource-generation.md 335586d64757f1e6293a89f14c1c3d578321eac6 
>   examples/vagrant/aurorabuild.sh 5eb171cf45ffee1287f3ac039ab8cf3db6991a97 
>   src/main/python/apache/aurora/executor/BUILD 
> cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 0fbb0f1ee63499d9ce36150ae5e68fcc8a9e 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> 8093717266f8620ebc6ef4c028ac8c87ab8d22be 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> d4392faf50f8c72f08f951962913248045d7fcb5 
>   src/main/python/apache/thermos/cli/commands/BUILD 
> 1dae8c981bd750807ddd1b6071e232ff2697537d 
>   src/main/python/apache/thermos/cli/commands/gc.py 
> 23d9ff4d2048b4f2d80ea62c54e58e8d768e11c0 
>   src/main/python/apache/thermos/cli/main.py 
> f20f612790550b77ee3dc969c37317b014a64972 
>   src/main/python/apache/thermos/core/BUILD 
> efb68e84cf547cb9505a8caf5b47be394dee5145 
>   src/main/python/apache/thermos/core/helper.py 
> 8cd32948663a5d5a1e975e1661b78de701710436 
>   src/main/python/apache/thermos/core/inspector.py 
> 4fe8aa31215a12b9a53e885697b4dd4e78c1f35f 
>   src/main/python/apache/thermos/monitoring/BUILD 
> 633dd95f9d193b1f377ab5d6cdfcdca7bdaa610f 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> aa5a2729ae6c94b6a270d97425767ccee121e588 
>   src/test/python/apache/aurora/executor/BUILD 
> f415ecc77022b34f053c35272d004e133803d702 
>   src/test/python/apache/aurora/executor/bin/BUILD 
> 2caab2aec136ede9b51ce3bdd0d139270024ba48 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> d4c1d572663039eb742f70de1e06d708eb0b769a 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 17d359054d1fc33f79a7612162064abd335ccf81 
>   src/test/python/apache/thermos/cli/commands/test_import.py 
> 74d9a32cf85a9e49cfbc596a7d6d44393df14e7a 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 89030d0e25e8eb3f4d4eec6a0d0a0fc3dfd43481 
>   src/test/python/apache/thermos/monitoring/test_garbage.py 
> 4309c46a3af5f12c8eb3192e3156348fa7c0db23 
> 
> Diff: https://reviews.apache.org/r/35813/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 35813: Removing GC executor code.

2015-06-25 Thread Maxim Khutornenko

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

(Updated June 25, 2015, 4:59 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


Changes
---

Updated NEWS.


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


Repository: aurora


Description
---

Removing GC executor code.


Diffs (updated)
-

  NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
  api/src/main/thrift/org/apache/aurora/gen/BUILD 
fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
  api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
  debian/aurora-executor.install 8efb1308caf64a23bed4b580de4b86e7982539e8 
  debian/rules 6ba18cef7fbf0989507d630a1041cdf958742617 
  docs/test-resource-generation.md 335586d64757f1e6293a89f14c1c3d578321eac6 
  examples/vagrant/aurorabuild.sh 5eb171cf45ffee1287f3ac039ab8cf3db6991a97 
  src/main/python/apache/aurora/executor/BUILD 
cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
  src/main/python/apache/aurora/executor/bin/BUILD 
0fbb0f1ee63499d9ce36150ae5e68fcc8a9e 
  src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
8093717266f8620ebc6ef4c028ac8c87ab8d22be 
  src/main/python/apache/aurora/executor/gc_executor.py 
d4392faf50f8c72f08f951962913248045d7fcb5 
  src/main/python/apache/thermos/cli/commands/BUILD 
1dae8c981bd750807ddd1b6071e232ff2697537d 
  src/main/python/apache/thermos/cli/commands/gc.py 
23d9ff4d2048b4f2d80ea62c54e58e8d768e11c0 
  src/main/python/apache/thermos/cli/main.py 
f20f612790550b77ee3dc969c37317b014a64972 
  src/main/python/apache/thermos/core/BUILD 
efb68e84cf547cb9505a8caf5b47be394dee5145 
  src/main/python/apache/thermos/core/helper.py 
8cd32948663a5d5a1e975e1661b78de701710436 
  src/main/python/apache/thermos/core/inspector.py 
4fe8aa31215a12b9a53e885697b4dd4e78c1f35f 
  src/main/python/apache/thermos/monitoring/BUILD 
633dd95f9d193b1f377ab5d6cdfcdca7bdaa610f 
  src/main/python/apache/thermos/monitoring/garbage.py 
aa5a2729ae6c94b6a270d97425767ccee121e588 
  src/test/python/apache/aurora/executor/BUILD 
f415ecc77022b34f053c35272d004e133803d702 
  src/test/python/apache/aurora/executor/bin/BUILD 
2caab2aec136ede9b51ce3bdd0d139270024ba48 
  src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
d4c1d572663039eb742f70de1e06d708eb0b769a 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
17d359054d1fc33f79a7612162064abd335ccf81 
  src/test/python/apache/thermos/cli/commands/test_import.py 
74d9a32cf85a9e49cfbc596a7d6d44393df14e7a 
  src/test/python/apache/thermos/monitoring/BUILD 
89030d0e25e8eb3f4d4eec6a0d0a0fc3dfd43481 
  src/test/python/apache/thermos/monitoring/test_garbage.py 
4309c46a3af5f12c8eb3192e3156348fa7c0db23 

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


Testing
---


Thanks,

Maxim Khutornenko



Re: Review Request 35813: Removing GC executor code.

2015-06-25 Thread Maxim Khutornenko


> On June 25, 2015, 4:34 p.m., Bill Farner wrote:
> > 25 files changed, 2 insertions(+), 2054 deletions(-)
> > 
> > Nice!  2 requests:
> > 
> > - Can you confirm that end-to-end tests still pass?
> > - Can you add to this patch a note in NEWS under 0.9.0 that the GC executor 
> > has been removed?

I ran e2e tests before posting this diff.

News section updated.


- Maxim


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


On June 24, 2015, 12:22 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35813/
> ---
> 
> (Updated June 24, 2015, 12:22 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1333
> https://issues.apache.org/jira/browse/AURORA-1333
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removing GC executor code.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
>   api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
> a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
>   debian/aurora-executor.install 8efb1308caf64a23bed4b580de4b86e7982539e8 
>   debian/rules 6ba18cef7fbf0989507d630a1041cdf958742617 
>   docs/test-resource-generation.md 335586d64757f1e6293a89f14c1c3d578321eac6 
>   examples/vagrant/aurorabuild.sh 5eb171cf45ffee1287f3ac039ab8cf3db6991a97 
>   src/main/python/apache/aurora/executor/BUILD 
> cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 0fbb0f1ee63499d9ce36150ae5e68fcc8a9e 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> 8093717266f8620ebc6ef4c028ac8c87ab8d22be 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> d4392faf50f8c72f08f951962913248045d7fcb5 
>   src/main/python/apache/thermos/cli/commands/BUILD 
> 1dae8c981bd750807ddd1b6071e232ff2697537d 
>   src/main/python/apache/thermos/cli/commands/gc.py 
> 23d9ff4d2048b4f2d80ea62c54e58e8d768e11c0 
>   src/main/python/apache/thermos/cli/main.py 
> f20f612790550b77ee3dc969c37317b014a64972 
>   src/main/python/apache/thermos/core/BUILD 
> efb68e84cf547cb9505a8caf5b47be394dee5145 
>   src/main/python/apache/thermos/core/helper.py 
> 8cd32948663a5d5a1e975e1661b78de701710436 
>   src/main/python/apache/thermos/core/inspector.py 
> 4fe8aa31215a12b9a53e885697b4dd4e78c1f35f 
>   src/main/python/apache/thermos/monitoring/BUILD 
> 633dd95f9d193b1f377ab5d6cdfcdca7bdaa610f 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> aa5a2729ae6c94b6a270d97425767ccee121e588 
>   src/test/python/apache/aurora/executor/BUILD 
> f415ecc77022b34f053c35272d004e133803d702 
>   src/test/python/apache/aurora/executor/bin/BUILD 
> 2caab2aec136ede9b51ce3bdd0d139270024ba48 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> d4c1d572663039eb742f70de1e06d708eb0b769a 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 17d359054d1fc33f79a7612162064abd335ccf81 
>   src/test/python/apache/thermos/cli/commands/test_import.py 
> 74d9a32cf85a9e49cfbc596a7d6d44393df14e7a 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 89030d0e25e8eb3f4d4eec6a0d0a0fc3dfd43481 
>   src/test/python/apache/thermos/monitoring/test_garbage.py 
> 4309c46a3af5f12c8eb3192e3156348fa7c0db23 
> 
> Diff: https://reviews.apache.org/r/35813/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 35812: Remove "enable_legacy_constraints" flag.

2015-06-25 Thread Bill Farner

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

Ship it!



NEWS (line 5)


```
The scheduler command line argument enable_legacy_constraints has been 
removed, and the scheduler no longer automatically injects 'host' and 'rack' 
constraints for production services.
```


- Bill Farner


On June 23, 2015, 11:58 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35812/
> ---
> 
> (Updated June 23, 2015, 11:58 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1074
> https://issues.apache.org/jira/browse/AURORA-1074
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove the "enable_legacy_constraints" flag and associated behaviour.
> 
> 
> Diffs
> -
> 
>   NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  abbd23dd3ee4382565ce846eb035e2aa502badae 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  38ef412a6d29dfef7b305e00cf44522818303965 
> 
> Diff: https://reviews.apache.org/r/35812/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 35813: Removing GC executor code.

2015-06-25 Thread Bill Farner

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

Ship it!


25 files changed, 2 insertions(+), 2054 deletions(-)

Nice!  2 requests:

- Can you confirm that end-to-end tests still pass?
- Can you add to this patch a note in NEWS under 0.9.0 that the GC executor has 
been removed?

- Bill Farner


On June 24, 2015, 12:22 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35813/
> ---
> 
> (Updated June 24, 2015, 12:22 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1333
> https://issues.apache.org/jira/browse/AURORA-1333
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Removing GC executor code.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
>   api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
> a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
>   debian/aurora-executor.install 8efb1308caf64a23bed4b580de4b86e7982539e8 
>   debian/rules 6ba18cef7fbf0989507d630a1041cdf958742617 
>   docs/test-resource-generation.md 335586d64757f1e6293a89f14c1c3d578321eac6 
>   examples/vagrant/aurorabuild.sh 5eb171cf45ffee1287f3ac039ab8cf3db6991a97 
>   src/main/python/apache/aurora/executor/BUILD 
> cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 0fbb0f1ee63499d9ce36150ae5e68fcc8a9e 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> 8093717266f8620ebc6ef4c028ac8c87ab8d22be 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> d4392faf50f8c72f08f951962913248045d7fcb5 
>   src/main/python/apache/thermos/cli/commands/BUILD 
> 1dae8c981bd750807ddd1b6071e232ff2697537d 
>   src/main/python/apache/thermos/cli/commands/gc.py 
> 23d9ff4d2048b4f2d80ea62c54e58e8d768e11c0 
>   src/main/python/apache/thermos/cli/main.py 
> f20f612790550b77ee3dc969c37317b014a64972 
>   src/main/python/apache/thermos/core/BUILD 
> efb68e84cf547cb9505a8caf5b47be394dee5145 
>   src/main/python/apache/thermos/core/helper.py 
> 8cd32948663a5d5a1e975e1661b78de701710436 
>   src/main/python/apache/thermos/core/inspector.py 
> 4fe8aa31215a12b9a53e885697b4dd4e78c1f35f 
>   src/main/python/apache/thermos/monitoring/BUILD 
> 633dd95f9d193b1f377ab5d6cdfcdca7bdaa610f 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> aa5a2729ae6c94b6a270d97425767ccee121e588 
>   src/test/python/apache/aurora/executor/BUILD 
> f415ecc77022b34f053c35272d004e133803d702 
>   src/test/python/apache/aurora/executor/bin/BUILD 
> 2caab2aec136ede9b51ce3bdd0d139270024ba48 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> d4c1d572663039eb742f70de1e06d708eb0b769a 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 17d359054d1fc33f79a7612162064abd335ccf81 
>   src/test/python/apache/thermos/cli/commands/test_import.py 
> 74d9a32cf85a9e49cfbc596a7d6d44393df14e7a 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 89030d0e25e8eb3f4d4eec6a0d0a0fc3dfd43481 
>   src/test/python/apache/thermos/monitoring/test_garbage.py 
> 4309c46a3af5f12c8eb3192e3156348fa7c0db23 
> 
> Diff: https://reviews.apache.org/r/35813/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 35847: Split http lifecycle into a composition layer.

2015-06-25 Thread Brian Brazil

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



docs/configuration-reference.md (line 436)


If we're making this configurable, I think that we should make it apply to 
the healthcheck config too.

Does the HealthCheckConfig also belong in lifecycle? I'd consider them 
pretty strongly related.


- Brian Brazil


On June 24, 2015, 10:45 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35847/
> ---
> 
> (Updated June 24, 2015, 10:45 p.m.)
> 
> 
> Review request for Aurora, Brian Brazil and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1368
> https://issues.apache.org/jira/browse/AURORA-1368
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Move shutdown endpoints to the Job config since the lifecycle is controlled 
> by Aurora and not Thermos.
> Split the lifecycle management into a composition layer that can more readily 
> be tested.
> 
> Also, derp, just realized I did not update the documentation.  Revision 
> forthcoming.
> (Also comment on the 'union' style here -- not sure what is preferable.)
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md 7bfd63381f54b0fe5ef6a4f17b825049b19038db 
>   src/main/python/apache/aurora/config/schema/base.py 
> 9a6f8a16f85c324ec75352710e19249443bf2c6b 
>   src/main/python/apache/aurora/config/thrift.py 
> 0a3e91011eccf8573feb296bd7f72913622e0ce0 
>   src/main/python/apache/aurora/executor/BUILD 
> cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b 
>   src/main/python/apache/aurora/executor/http_lifecycle.py PRE-CREATION 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7bcd6c42f50665eac2e7f17b84e63f2ea7fb4d4f 
>   src/main/python/apache/thermos/config/schema_base.py 
> a85def9eea25fa01020ca2dda4e9cefe861c4a5f 
>   src/test/python/apache/aurora/executor/BUILD 
> f415ecc77022b34f053c35272d004e133803d702 
>   src/test/python/apache/aurora/executor/common/fixtures.py 
> 37d032beb66a67cfd3cfcea272747a2915a745ff 
>   src/test/python/apache/aurora/executor/test_http_lifecycle.py PRE-CREATION 
>   src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
> 3569a6abf84d5144d2e268b0a86c82285ffe2b2b 
> 
> Diff: https://reviews.apache.org/r/35847/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>