You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Stephan Erb <st...@dev.static-void.de> on 2015/06/16 11:00:41 UTC
Review Request 35498: Compute SLA stats for non-prod jobs
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
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
-----
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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88042
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java (line 57)
<https://reviews.apache.org/r/35498/#comment140448>
I could change the command line to take a list of SLA statistic enums to enable. This should help to greatly reduce the overall boilerplate. What do you think?
src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java (line 65)
<https://reviews.apache.org/r/35498/#comment140450>
Using a list of enums instead of serveral bools will also help to simplify this code.
- Stephan Erb
On June 16, 2015, 11 a.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 11 a.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> 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
> -----
>
> 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
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88261
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java (lines 73 - 87)
<https://reviews.apache.org/r/35498/#comment140683>
This suggests the current AlgorithmType model is not very flexible. How would you feel about moving the environment part from the AlgorithmType into MetricCalculator? That will allow keeping AlgorithmType env-agnostic and will make adding new env-based metrics much easier.
The AlgorithmType would only have vanilla metric names (e.g. "mtta_ms", "99.00" and etc.). The MetricCalculator would then pass "nonprod" into `runAlgorithms()` to get appended to the metric name (e.g. `sla_<job_key>_mtta_ms_nonprod`). This will change the existing nonprod metric names a bit but will let us add new metrics for various task subsets without ever touching the SlaAlgorithm.
- Maxim Khutornenko
On June 16, 2015, 9:07 a.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 9:07 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
> -----
>
> 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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88044
-----------------------------------------------------------
Ship it!
Master (b09adc6) 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 16, 2015, 9:07 a.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 9:07 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
> -----
>
> 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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88562
-----------------------------------------------------------
Ship it!
Master (589d901) 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 19, 2015, 3:23 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 3:23 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
> -----
>
> 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
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88802
-----------------------------------------------------------
Ship it!
Thanks!
src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java (line 65)
<https://reviews.apache.org/r/35498/#comment141384>
Missing "."
src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java (line 70)
<https://reviews.apache.org/r/35498/#comment141385>
Same here.
- Maxim Khutornenko
On June 21, 2015, 2:39 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 21, 2015, 2:39 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
> -----
>
> 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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88946
-----------------------------------------------------------
Ship it!
Master (2df2db9) 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 23, 2015, 9:13 a.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 9:13 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
> -----
>
> 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
Posted by Stephan Erb <st...@dev.static-void.de>.
> On June 25, 2015, 7:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 138-139
> > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138>
> >
> > 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
Posted by Stephan Erb <st...@dev.static-void.de>.
> On June 25, 2015, 7:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 138-139
> > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138>
> >
> > 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.
>
> Stephan Erb wrote:
> Your consistency argument is very valid. I guess we can leave it as it is.
>
> Maxim Khutornenko wrote:
> IMO, using getters in cases like this feels redundant and just creates ground for inconsistent field/method reference. I'd drop getters as they provide zero value here.
I've changed the code back to not using any getters where possible.
Anything still missing, or can we get this merged?
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review89394
-----------------------------------------------------------
On July 1, 2015, 7:09 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 7: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
>
>
> 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
Posted by Stephan Erb <st...@dev.static-void.de>.
> On June 25, 2015, 7:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 138-139
> > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138>
> >
> > 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
>
>
Re: Review Request 35498: Compute SLA stats for non-prod jobs
Posted by Maxim Khutornenko <ma...@apache.org>.
> On June 25, 2015, 5:59 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 138-139
> > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138>
> >
> > 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.
>
> Stephan Erb wrote:
> Your consistency argument is very valid. I guess we can leave it as it is.
IMO, using getters in cases like this feels redundant and just creates ground for inconsistent field/method reference. I'd drop getters as they provide zero value here.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review89394
-----------------------------------------------------------
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
Posted by Kevin Sweeney <ke...@apache.org>.
> On June 25, 2015, 10:59 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines 138-139
> > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138>
> >
> > 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 35498: Compute SLA stats for non-prod jobs
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/35498/#comment141980>
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 35498: Compute SLA stats for non-prod jobs
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review89244
-----------------------------------------------------------
Ship it!
Master (4b8c34c) 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 24, 2015, 7: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, 7: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 35498: Compute SLA stats for non-prod jobs
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated July 1, 2015, 7: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 (updated)
-------
Compute SLA stats for non-prod jobs
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review89956
-----------------------------------------------------------
Ship it!
Master (616ef10) 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 30, 2015, 7:51 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 30, 2015, 7:51 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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 30, 2015, 9:51 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Drop getters where possible. `getRefreshRateMs` has to remain as it is called by the SlaUpdater service.
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 35498: Compute SLA stats for non-prod jobs
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
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 35498: Compute SLA stats for non-prod jobs
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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 35498: Compute SLA stats for non-prod jobs
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
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 35498: Compute SLA stats for non-prod jobs
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
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.
Changes
-------
Typo in NEWS
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 35498: Compute SLA stats for non-prod jobs
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 24, 2015, 9:08 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Document breaking changes in NEWS
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 35498: Compute SLA stats for non-prod jobs
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 23, 2015, 11:13 a.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Add missing periods to flag help text.
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)
-----
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88699
-----------------------------------------------------------
Ship it!
Master (d240926) 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 21, 2015, 2:39 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 21, 2015, 2:39 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
> -----
>
> 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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 21, 2015, 4:39 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Update documentation sla.md
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)
-----
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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 21, 2015, 4:34 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Changes proposed by Maxim
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)
-----
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
Posted by Stephan Erb <st...@dev.static-void.de>.
> On June 19, 2015, 8:12 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java, line 65
> > <https://reviews.apache.org/r/35498/diff/2/?file=988347#file988347line65>
> >
> > Since it backs enum, you may want to add something like:
> >
> > "... Available metric categories:" + Joiner.on(",").join(MetricCategory.values())
This does not seem to work. It failes during compilation with: `error: element value must be a constant expression`.
Details on stackoverflow: https://stackoverflow.com/questions/26674013/how-to-map-all-values-of-an-enum-to-constant-expression
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88552
-----------------------------------------------------------
On June 19, 2015, 5:23 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 5:23 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
> -----
>
> 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
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/#review88552
-----------------------------------------------------------
Loogs great overall! Just a few nits/suggestions.
src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java (line 68)
<https://reviews.apache.org/r/35498/#comment141136>
s/metrices/metrics
src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java (line 73)
<https://reviews.apache.org/r/35498/#comment141138>
This should fit to the previous line. Same below.
src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java (line 137)
<https://reviews.apache.org/r/35498/#comment141139>
requireNonNull
src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java (line 207)
<https://reviews.apache.org/r/35498/#comment141142>
Move this to const and reference from tests?
src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java (line 65)
<https://reviews.apache.org/r/35498/#comment141141>
Since it backs enum, you may want to add something like:
"... Available metric categories:" + Joiner.on(",").join(MetricCategory.values())
- Maxim Khutornenko
On June 19, 2015, 3:23 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35498/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 3:23 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
> -----
>
> 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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 19, 2015, 5:23 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Changes:
* Implment Maxim's proposal
* Introduce MetricCategory enum encapsulating related metrics
Breaking changes:
* nonprod metric name change from "...nonprod_ms" to "...ms_nonprod"
* all non prod metrics disabled by default
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)
-----
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
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35498/
-----------------------------------------------------------
(Updated June 16, 2015, 11:07 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
-----
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