Re: Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-26 Thread Aurora ReviewBot 

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

Ship it!


Master (2ef6a05) 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, 11:54 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35954/
> ---
> 
> (Updated June 26, 2015, 11:54 p.m.)
> 
> 
> Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1375
> https://issues.apache.org/jira/browse/AURORA-1375
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having quota below current production resesrvation is illogical and the 
> scheduler should reject requests that would result in this.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 58ee226a3819d6796a169156320c10677e35611a 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2a2b499905a788eb59f5824bc66b487f91e838f1 
> 
> Diff: https://reviews.apache.org/r/35954/diff/
> 
> 
> Testing
> ---
> 
> ./rbt post -o
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Review Request 35954: Prevent quota from being set below current production reservation.

2015-06-26 Thread Zameer Manji 

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

Review request for Aurora, Joe Smith, Maxim Khutornenko, and Bill Farner.


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


Repository: aurora


Description
---

Having quota below current production resesrvation is illogical and the 
scheduler should reject requests that would result in this.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
7453680af5a518012f9779f82d0349e897c04994 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
9af379c36d2e3e44f462ed5d431f8a497b2d09f6 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
58ee226a3819d6796a169156320c10677e35611a 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ee8f5423810c60b2075e6b98ba8cda36393ae5cc 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2a2b499905a788eb59f5824bc66b487f91e838f1 

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


Testing
---

./rbt post -o


Thanks,

Zameer Manji

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

2015-06-26 Thread Stephan Erb 


> On June 25, 2015, 7:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 
> > 138-139
> > 
> >
> > Add getters for these fields and access them below via the getters 
> > rather than direct field access.
> 
> Stephan Erb wrote:
> I can submit an updated patch tonight. I've somewhat expected that some 
> of you would point it out :-)
> 
> I thought about adding these when writing the patch but then decided 
> against them: Getters for final attributes on an inner class did not seem to 
> offer any meaningful encapsulation or help during refactoring.
> 
> Kevin Sweeney wrote:
> In this case the class in question uses getters and direct field access 
> inconsistently, I'd also be okay with removing all getters on that class and 
> replacing them with direct field access. I think getters are more readable 
> though (it's a code review red flag when I see a new direct field access). 
> Other committers feel free to chime in here.

Your consistency argument is very valid. I guess we can leave it as it is.


- Stephan


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


On June 26, 2015, 9 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> ---
> 
> (Updated June 26, 2015, 9 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 fd5b70823cf7a695831d34453ab101c7d6a626e8 
>   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 35498: Compute SLA stats for non-prod jobs

2015-06-26 Thread Aurora ReviewBot 

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

Ship it!


Master (2ef6a05) 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, 7 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> ---
> 
> (Updated June 26, 2015, 7 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 fd5b70823cf7a695831d34453ab101c7d6a626e8 
>   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 35498: Compute SLA stats for non-prod jobs

2015-06-26 Thread Stephan Erb 

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

(Updated June 26, 2015, 9 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

Rebase.


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 (updated)
-

  NEWS fd5b70823cf7a695831d34453ab101c7d6a626e8 
  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 35932: Remove unnecessary uses of type witness.

2015-06-26 Thread Aurora ReviewBot 

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


This patch does not apply cleanly on master (2ef6a05), do you need to rebase?

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

- Aurora ReviewBot


On June 26, 2015, 5:50 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35932/
> ---
> 
> (Updated June 26, 2015, 5:50 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Note that this does not remove _all_ type witnesses, as the compiler still 
> relies on them in several cases.  For example, chained method calls cannot 
> compile without them:
> 
> ```
> ImmutableSet.builder()
>   .add('a')
>   .add('b')
>   .build()
> ```
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 308bbd92943e331179bc5fad1f3fa0febba5ed1c 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> d3e1295db9b933b5ab60d6446dfea453c6051795 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
> 3413b0a480073f39bc01a60bc93b211660d9e278 
>   src/main/java/org/apache/aurora/GuiceUtils.java 
> 5d0af1ebdccd0d0782f0fbc53d046a338c450167 
>   src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
> 9e1f35a12205065f2f0cc783a49e14384bc4d50a 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 26093ef35f18eda5b05fe2a9351ac80fbeca3dc8 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java 
> 538cb755a5f80b9e70e3069205f219558053da9b 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 968aca6053a722dbaec6c5832c09e0816ae069ae 
>   src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java 
> f0dea48b3df69dedf620a55573731ed9451369c3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/BiCache.java 
> f5a18338748da7c443b2fd2ec3a72adf75e7387f 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImpl.java
>  cd016af5621be76190ead81921096aae837b59c0 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
>  c1114a411c32c25785adc93e594ee8e291025969 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
> 41591b80ee6d078e1c2beb5d98c7a7584ce173e2 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> 7f166e2ec24f66a9ab0c35e3c65af45e461943f9 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> 673a6909ec1bccbc43b0a3cd913aa48b9b1d90c1 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> df180a4f9dff75a04ae104462c219444ef605d9a 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 770672c85c06aaf4356b3b3580428b0323edb9d3 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0063aea16ff39e95c8d32848fdb3eeec6ab1bee3 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java 
> bd74f892dde65e957f125a17979ee6c582b3043f 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintMatcher.java 
> ecba276f1882162d9578e0829b2139040876c7d0 
>   src/main/java/org/apache/aurora/scheduler/http/Cron.java 
> fd658e1ac5e73ef0c61c82685f6373c5f0eb6640 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> fcf5e4419b9908281858e591c916c586c707c6cd 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d735cbc4169dbf3f7fb8dd1c13bf683ddcce548c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  1e9b1c3b6e256fd994e94ad30d9340d12ce15f99 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java
>  b224983159c4979800fa384bbb082adc7b330b4c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
>  7425b93f476ca1228a233a56363136f9e586a5e5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> e934f570e4a728470408970485abe0809487d312 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java 
> 5bf4d9a659942c65290442fdcd4749bda046bfd8 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> a6d7ab748aeb050f93f817e3b084b03d34a58d9d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> 1b8733bff69aef5a7db6bc7d137932a69717275a 
>   src/main

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

2015-06-26 Thread Aurora ReviewBot 

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


This patch does not apply cleanly on master (2ef6a05), do you need to rebase?

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

- Aurora ReviewBot


On June 26, 2015, 5:45 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> ---
> 
> (Updated June 26, 2015, 5:45 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 35932: Remove unnecessary uses of type witness.

2015-06-26 Thread Kevin Sweeney 

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

Ship it!


Ship It!

- Kevin Sweeney


On June 26, 2015, 10:50 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35932/
> ---
> 
> (Updated June 26, 2015, 10:50 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Note that this does not remove _all_ type witnesses, as the compiler still 
> relies on them in several cases.  For example, chained method calls cannot 
> compile without them:
> 
> ```
> ImmutableSet.builder()
>   .add('a')
>   .add('b')
>   .build()
> ```
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 308bbd92943e331179bc5fad1f3fa0febba5ed1c 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> d3e1295db9b933b5ab60d6446dfea453c6051795 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
> 3413b0a480073f39bc01a60bc93b211660d9e278 
>   src/main/java/org/apache/aurora/GuiceUtils.java 
> 5d0af1ebdccd0d0782f0fbc53d046a338c450167 
>   src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
> 9e1f35a12205065f2f0cc783a49e14384bc4d50a 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 26093ef35f18eda5b05fe2a9351ac80fbeca3dc8 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java 
> 538cb755a5f80b9e70e3069205f219558053da9b 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 968aca6053a722dbaec6c5832c09e0816ae069ae 
>   src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java 
> f0dea48b3df69dedf620a55573731ed9451369c3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/BiCache.java 
> f5a18338748da7c443b2fd2ec3a72adf75e7387f 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImpl.java
>  cd016af5621be76190ead81921096aae837b59c0 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
>  c1114a411c32c25785adc93e594ee8e291025969 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
> 41591b80ee6d078e1c2beb5d98c7a7584ce173e2 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> 7f166e2ec24f66a9ab0c35e3c65af45e461943f9 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> 673a6909ec1bccbc43b0a3cd913aa48b9b1d90c1 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> df180a4f9dff75a04ae104462c219444ef605d9a 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 770672c85c06aaf4356b3b3580428b0323edb9d3 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0063aea16ff39e95c8d32848fdb3eeec6ab1bee3 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java 
> bd74f892dde65e957f125a17979ee6c582b3043f 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintMatcher.java 
> ecba276f1882162d9578e0829b2139040876c7d0 
>   src/main/java/org/apache/aurora/scheduler/http/Cron.java 
> fd658e1ac5e73ef0c61c82685f6373c5f0eb6640 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> fcf5e4419b9908281858e591c916c586c707c6cd 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d735cbc4169dbf3f7fb8dd1c13bf683ddcce548c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  1e9b1c3b6e256fd994e94ad30d9340d12ce15f99 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java
>  b224983159c4979800fa384bbb082adc7b330b4c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
>  7425b93f476ca1228a233a56363136f9e586a5e5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> e934f570e4a728470408970485abe0809487d312 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java 
> 5bf4d9a659942c65290442fdcd4749bda046bfd8 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> a6d7ab748aeb050f93f817e3b084b03d34a58d9d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> 1b8733bff69aef5a7db6bc7d137932a69717275a 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> 48d0ff667cf97002795b97c235b9e9e34f8e5018 
>   src/main/java/org/apache/aurora/sch

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

2015-06-26 Thread Kevin Sweeney 


> On June 25, 2015, 10:59 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 
> > 138-139
> > 
> >
> > Add getters for these fields and access them below via the getters 
> > rather than direct field access.
> 
> Stephan Erb wrote:
> I can submit an updated patch tonight. I've somewhat expected that some 
> of you would point it out :-)
> 
> I thought about adding these when writing the patch but then decided 
> against them: Getters for final attributes on an inner class did not seem to 
> offer any meaningful encapsulation or help during refactoring.

In this case the class in question uses getters and direct field access 
inconsistently, I'd also be okay with removing all getters on that class and 
replacing them with direct field access. I think getters are more readable 
though (it's a code review red flag when I see a new direct field access). 
Other committers feel free to chime in here.


- Kevin


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


On June 26, 2015, 10:45 a.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> ---
> 
> (Updated June 26, 2015, 10:45 a.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 35932: Remove unnecessary uses of type witness.

2015-06-26 Thread Zameer Manji 

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

Ship it!


- Zameer Manji


On June 26, 2015, 10:50 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35932/
> ---
> 
> (Updated June 26, 2015, 10:50 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Note that this does not remove _all_ type witnesses, as the compiler still 
> relies on them in several cases.  For example, chained method calls cannot 
> compile without them:
> 
> ```
> ImmutableSet.builder()
>   .add('a')
>   .add('b')
>   .build()
> ```
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 308bbd92943e331179bc5fad1f3fa0febba5ed1c 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> d3e1295db9b933b5ab60d6446dfea453c6051795 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
> 3413b0a480073f39bc01a60bc93b211660d9e278 
>   src/main/java/org/apache/aurora/GuiceUtils.java 
> 5d0af1ebdccd0d0782f0fbc53d046a338c450167 
>   src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
> 9e1f35a12205065f2f0cc783a49e14384bc4d50a 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 26093ef35f18eda5b05fe2a9351ac80fbeca3dc8 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java 
> 538cb755a5f80b9e70e3069205f219558053da9b 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 968aca6053a722dbaec6c5832c09e0816ae069ae 
>   src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java 
> f0dea48b3df69dedf620a55573731ed9451369c3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/BiCache.java 
> f5a18338748da7c443b2fd2ec3a72adf75e7387f 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImpl.java
>  cd016af5621be76190ead81921096aae837b59c0 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
>  c1114a411c32c25785adc93e594ee8e291025969 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
> 41591b80ee6d078e1c2beb5d98c7a7584ce173e2 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> 7f166e2ec24f66a9ab0c35e3c65af45e461943f9 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b77b0ebbf303778e528b16ff3db1aa4e76f1 
>   src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
> 673a6909ec1bccbc43b0a3cd913aa48b9b1d90c1 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> df180a4f9dff75a04ae104462c219444ef605d9a 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 770672c85c06aaf4356b3b3580428b0323edb9d3 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0063aea16ff39e95c8d32848fdb3eeec6ab1bee3 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java 
> bd74f892dde65e957f125a17979ee6c582b3043f 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintMatcher.java 
> ecba276f1882162d9578e0829b2139040876c7d0 
>   src/main/java/org/apache/aurora/scheduler/http/Cron.java 
> fd658e1ac5e73ef0c61c82685f6373c5f0eb6640 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> fcf5e4419b9908281858e591c916c586c707c6cd 
>   src/main/java/org/apache/aurora/scheduler/http/Mname.java 
> d735cbc4169dbf3f7fb8dd1c13bf683ddcce548c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  1e9b1c3b6e256fd994e94ad30d9340d12ce15f99 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java
>  b224983159c4979800fa384bbb082adc7b330b4c 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
>  7425b93f476ca1228a233a56363136f9e586a5e5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> e934f570e4a728470408970485abe0809487d312 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java 
> 5bf4d9a659942c65290442fdcd4749bda046bfd8 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 7453680af5a518012f9779f82d0349e897c04994 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> a6d7ab748aeb050f93f817e3b084b03d34a58d9d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> 1b8733bff69aef5a7db6bc7d137932a69717275a 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> 48d0ff667cf97002795b97c235b9e9e34f8e5018 
>   src/main/java/org/apache/aurora/scheduler/stat

Re: Review Request 35840: Don't warn about unoptimized Operations

2015-06-26 Thread Bill Farner 

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

Ship it!


Ship It!

- Bill Farner


On June 26, 2015, 5:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35840/
> ---
> 
> (Updated June 26, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1369
> https://issues.apache.org/jira/browse/AURORA-1369
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Don't warn about unoptimized Operations
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
> 12da352a749fe5d974b0cd69eda0b694b0a90bf4 
> 
> Diff: https://reviews.apache.org/r/35840/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Review Request 35932: Remove unnecessary uses of type witness.

2015-06-26 Thread Bill Farner 

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Repository: aurora


Description
---

Note that this does not remove _all_ type witnesses, as the compiler still 
relies on them in several cases.  For example, chained method calls cannot 
compile without them:

```
ImmutableSet.builder()
  .add('a')
  .add('b')
  .build()
```


Diffs
-

  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
308bbd92943e331179bc5fad1f3fa0febba5ed1c 
  src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
d3e1295db9b933b5ab60d6446dfea453c6051795 
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
3413b0a480073f39bc01a60bc93b211660d9e278 
  src/main/java/org/apache/aurora/GuiceUtils.java 
5d0af1ebdccd0d0782f0fbc53d046a338c450167 
  src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
9e1f35a12205065f2f0cc783a49e14384bc4d50a 
  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
26093ef35f18eda5b05fe2a9351ac80fbeca3dc8 
  src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java 
538cb755a5f80b9e70e3069205f219558053da9b 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
968aca6053a722dbaec6c5832c09e0816ae069ae 
  src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java 
f0dea48b3df69dedf620a55573731ed9451369c3 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/BiCache.java 
f5a18338748da7c443b2fd2ec3a72adf75e7387f 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImpl.java 
cd016af5621be76190ead81921096aae837b59c0 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
 c1114a411c32c25785adc93e594ee8e291025969 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
41591b80ee6d078e1c2beb5d98c7a7584ce173e2 
  src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
7f166e2ec24f66a9ab0c35e3c65af45e461943f9 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b77b0ebbf303778e528b16ff3db1aa4e76f1 
  src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 
673a6909ec1bccbc43b0a3cd913aa48b9b1d90c1 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
df180a4f9dff75a04ae104462c219444ef605d9a 
  src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
770672c85c06aaf4356b3b3580428b0323edb9d3 
  src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
0063aea16ff39e95c8d32848fdb3eeec6ab1bee3 
  src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java 
bd74f892dde65e957f125a17979ee6c582b3043f 
  src/main/java/org/apache/aurora/scheduler/filter/ConstraintMatcher.java 
ecba276f1882162d9578e0829b2139040876c7d0 
  src/main/java/org/apache/aurora/scheduler/http/Cron.java 
fd658e1ac5e73ef0c61c82685f6373c5f0eb6640 
  src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
fcf5e4419b9908281858e591c916c586c707c6cd 
  src/main/java/org/apache/aurora/scheduler/http/Mname.java 
d735cbc4169dbf3f7fb8dd1c13bf683ddcce548c 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
 1e9b1c3b6e256fd994e94ad30d9340d12ce15f99 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java 
b224983159c4979800fa384bbb082adc7b330b4c 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
 7425b93f476ca1228a233a56363136f9e586a5e5 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
e934f570e4a728470408970485abe0809487d312 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java 
5bf4d9a659942c65290442fdcd4749bda046bfd8 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
7453680af5a518012f9779f82d0349e897c04994 
  src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
a6d7ab748aeb050f93f817e3b084b03d34a58d9d 
  src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
1b8733bff69aef5a7db6bc7d137932a69717275a 
  src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
48d0ff667cf97002795b97c235b9e9e34f8e5018 
  src/main/java/org/apache/aurora/scheduler/stats/ResourceCounter.java 
e5c0322007c8d9424b96016c9075790c69cf2cf5 
  src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java 
97f9ee81ed18a961e6df4916d9ede1d89f90ded1 
  
src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
 0a519be65f90cb730f6bda1e6d7b019f0f15252b 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
b87157562045247f8f0aa43f04ae590a6c09a7e1 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
41e144b9c08500867bab8854770f778e6a211b19 
  src/main/

Re: Review Request 35840: Don't warn about unoptimized Operations

2015-06-26 Thread Aurora ReviewBot 

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


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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On June 26, 2015, 5:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35840/
> ---
> 
> (Updated June 26, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1369
> https://issues.apache.org/jira/browse/AURORA-1369
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Don't warn about unoptimized Operations
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
> 12da352a749fe5d974b0cd69eda0b694b0a90bf4 
> 
> Diff: https://reviews.apache.org/r/35840/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

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

2015-06-26 Thread Stephan Erb 

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

(Updated June 26, 2015, 7:45 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

Introduce getters for MetricCalculatorSettings


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 (updated)
-

  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 35928: Avoid unintentional use of TaskStatus.Reason default value.

2015-06-26 Thread Aurora ReviewBot 

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

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, 5 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35928/
> ---
> 
> (Updated June 26, 2015, 5 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1327
> https://issues.apache.org/jira/browse/AURORA-1327
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Avoid unintentional use of TaskStatus.Reason default value.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 33749de538e9a6e08f6ec640ee44f8a5d74256e0 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f08c79932f2ff9ed64b273a432c7d33c05ad95bd 
> 
> Diff: https://reviews.apache.org/r/35928/diff/
> 
> 
> Testing
> ---
> 
> The test methodology here is not great (though it's no worse than the 
> original).  For all intents and purposes, the test code is a carbon copy of 
> the application code.  Open to ideas, i couldn't quickly come up with 
> anything obviously better.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 35840: Don't warn about unoptimized Operations

2015-06-26 Thread Stephan Erb 

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

(Updated June 26, 2015, 7:32 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Drop warning and special cases for no-ops.


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


Repository: aurora


Description
---

Don't warn about unoptimized Operations


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
12da352a749fe5d974b0cd69eda0b694b0a90bf4 

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


Testing
---

./gradlew -Pq build


Thanks,

Stephan Erb

Re: Review Request 35840: Don't warn about unoptimized Operations

2015-06-26 Thread Stephan Erb 

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

(Updated June 26, 2015, 7:29 p.m.)


Review request for Aurora and Bill Farner.


Summary (updated)
-

Don't warn about unoptimized Operations


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


Repository: aurora


Description (updated)
---

Don't warn about unoptimized Operations


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
12da352a749fe5d974b0cd69eda0b694b0a90bf4 

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


Testing
---

./gradlew -Pq build


Thanks,

Stephan Erb

Review Request 35928: Avoid unintentional use of TaskStatus.Reason default value.

2015-06-26 Thread Bill Farner 

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

Review request for Aurora and Joshua Cohen.


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


Repository: aurora


Description
---

Avoid unintentional use of TaskStatus.Reason default value.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
33749de538e9a6e08f6ec640ee44f8a5d74256e0 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
f08c79932f2ff9ed64b273a432c7d33c05ad95bd 

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


Testing
---

The test methodology here is not great (though it's no worse than the 
original).  For all intents and purposes, the test code is a carbon copy of the 
application code.  Open to ideas, i couldn't quickly come up with anything 
obviously better.


Thanks,

Bill Farner

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

2015-06-26 Thread Brian Brazil 

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

Ship it!


Ship It!

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

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

2015-06-26 Thread Brian Brazil 


> 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.
> 
> Brian Wickman wrote:
> 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.

I see both as 'do things with http on the executor', but seperating them makes 
sense to an extent too.


- 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-26 Thread Stephan Erb 


> On June 25, 2015, 7:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 
> > 138-139
> > 
> >
> > Add getters for these fields and access them below via the getters 
> > rather than direct field access.

I can submit an updated patch tonight. I've somewhat expected that some of you 
would point it out :-)

I thought about adding these when writing the patch but then decided against 
them: Getters for final attributes on an inner class did not seem to offer any 
meaningful encapsulation or help during refactoring.


- Stephan


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


On June 24, 2015, 9: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, 9: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
> 
>