You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/02/11 22:39:12 UTC
Review Request 30895: Offer filtering for static vetoes. Part 4 of 4:
Benchmarks.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Bill Farner.
Repository: aurora
Description
-------
Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
Original RB: https://reviews.apache.org/r/28617/
Diffs
-----
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446
src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
Diff: https://reviews.apache.org/r/30895/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review72048
-----------------------------------------------------------
Ship it!
Master (7b531e9) 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 Feb. 11, 2015, 9:39 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2015, 9:39 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 19, 2015, 10:40 p.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 272
> > <https://reviews.apache.org/r/30895/diff/1/?file=861101#file861101line272>
> >
> > You might want to use a limit rather than value constraint for this test, or add another benchmark for both. Limit constraints are more work for the scheduler to compute, and would yield more real-world results.
Added a limit constrant benchmark. From a quick run it's only 0.5% slower than value benchmark. Good to have anyway.
> On Feb. 19, 2015, 10:40 p.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 210
> > <https://reviews.apache.org/r/30895/diff/1/?file=861101#file861101line210>
> >
> > Forgive me if i've asked this before, but what's the reasoning behind using extension for what seems like static configuration? Seems like a settings builder class would be more appropriate.
I was going to address it later but since the original diff was split I may as well do it now. Thanks for reminding.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73217
-----------------------------------------------------------
On Feb. 11, 2015, 9:39 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2015, 9:39 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73217
-----------------------------------------------------------
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
<https://reviews.apache.org/r/30895/#comment119464>
Forgive me if i've asked this before, but what's the reasoning behind using extension for what seems like static configuration? Seems like a settings builder class would be more appropriate.
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
<https://reviews.apache.org/r/30895/#comment119465>
You might want to use a limit rather than value constraint for this test, or add another benchmark for both. Limit constraints are more work for the scheduler to compute, and would yield more real-world results.
- Bill Farner
On Feb. 11, 2015, 9:39 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2015, 9:39 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73677
-----------------------------------------------------------
Master (277ac43) is red with this patch.
./build-support/jenkins/build.sh
src.test.python.apache.aurora.client.cli.update ..... SUCCESS
src.test.python.apache.aurora.client.cli.version ..... SUCCESS
src.test.python.apache.aurora.client.config ..... SUCCESS
src.test.python.apache.aurora.client.factory ..... SUCCESS
src.test.python.apache.aurora.client.hooks.hooked_api ..... SUCCESS
src.test.python.apache.aurora.client.hooks.non_hooked_api ..... SUCCESS
src.test.python.apache.aurora.common.test_aurora_job_key ..... SUCCESS
src.test.python.apache.aurora.common.test_cluster ..... SUCCESS
src.test.python.apache.aurora.common.test_cluster_option ..... SUCCESS
src.test.python.apache.aurora.common.test_clusters ..... SUCCESS
src.test.python.apache.aurora.common.test_http_signaler ..... SUCCESS
src.test.python.apache.aurora.common.test_pex_version ..... SUCCESS
src.test.python.apache.aurora.common.test_shellify ..... SUCCESS
src.test.python.apache.aurora.common.test_transport ..... SUCCESS
src.test.python.apache.aurora.config.test_base ..... SUCCESS
src.test.python.apache.aurora.config.test_constraint_parsing ..... SUCCESS
src.test.python.apache.aurora.config.test_loader ..... SUCCESS
src.test.python.apache.aurora.config.test_thrift ..... SUCCESS
src.test.python.apache.aurora.executor.common.announcer ..... SUCCESS
src.test.python.apache.aurora.executor.common.directory_sandbox ..... SUCCESS
src.test.python.apache.aurora.executor.common.executor_detector ..... SUCCESS
src.test.python.apache.aurora.executor.common.executor_timeout ..... SUCCESS
src.test.python.apache.aurora.executor.common.health_checker ..... SUCCESS
src.test.python.apache.aurora.executor.common.kill_manager ..... SUCCESS
src.test.python.apache.aurora.executor.common.path_detector ..... SUCCESS
src.test.python.apache.aurora.executor.common.status_checker ..... SUCCESS
src.test.python.apache.aurora.executor.common.task_info ..... SUCCESS
src.test.python.apache.aurora.executor.executor_base ..... SUCCESS
src.test.python.apache.aurora.executor.executor_vars ..... SUCCESS
src.test.python.apache.aurora.executor.gc_executor ..... FAILURE
src.test.python.apache.aurora.executor.status_manager ..... SUCCESS
src.test.python.apache.aurora.executor.thermos_task_runner ..... SUCCESS
src.test.python.apache.thermos.common.test_pathspec ..... SUCCESS
src.test.python.apache.thermos.core.test_runner_integration ..... SUCCESS
src.test.python.apache.thermos.monitoring.test_disk ..... SUCCESS
FAILURE
[31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Feb. 23, 2015, 9:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 9:03 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 23, 2015, 9:59 p.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 66
> > <https://reviews.apache.org/r/30895/diff/3/?file=873099#file873099line66>
> >
> > Javadoc here would be nice. I failed to elaborate on the last review, but it's not immediatly obvious what this value means.
My bad, forgot to address in the previous diff. Fixed now.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73680
-----------------------------------------------------------
On Feb. 23, 2015, 9:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 9:03 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73680
-----------------------------------------------------------
Ship it!
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java
<https://reviews.apache.org/r/30895/#comment120069>
Javadoc here would be nice. I failed to elaborate on the last review, but it's not immediatly obvious what this value means.
- Bill Farner
On Feb. 23, 2015, 9:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 9:03 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73729
-----------------------------------------------------------
Ship it!
Master (1449a20) 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 Feb. 23, 2015, 10:47 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 10:47 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73887
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Sweeney
On Feb. 23, 2015, 2:47 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 2:47 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/
-----------------------------------------------------------
(Updated Feb. 23, 2015, 10:47 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Bill's comments.
Repository: aurora
Description
-------
Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
Original RB: https://reviews.apache.org/r/28617/
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
Diff: https://reviews.apache.org/r/30895/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/
-----------------------------------------------------------
(Updated Feb. 23, 2015, 9:03 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Bill's comments.
Repository: aurora
Description
-------
Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
Original RB: https://reviews.apache.org/r/28617/
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
Diff: https://reviews.apache.org/r/30895/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 23, 2015, 7:47 p.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 33
> > <https://reviews.apache.org/r/30895/diff/2/?file=873059#file873059line33>
> >
> > This is bound to become a Texas constructor [1].
> >
> > Please consider using the builder pattern instead to improve readability at the call sites.
> >
> > Brief docs on these would be helpful too. Specifically, `clusterUtilization` would benefit from a blurb.
> >
> > [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#stay-out-of-texas
I can't force myself into justifying Builder here quite yet but I am fine with improving readabilty at call sites. Done.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73642
-----------------------------------------------------------
On Feb. 23, 2015, 7:34 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 7:34 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Kevin Sweeney <ke...@apache.org>.
> On Feb. 23, 2015, 11:47 a.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 33
> > <https://reviews.apache.org/r/30895/diff/2/?file=873059#file873059line33>
> >
> > This is bound to become a Texas constructor [1].
> >
> > Please consider using the builder pattern instead to improve readability at the call sites.
> >
> > Brief docs on these would be helpful too. Specifically, `clusterUtilization` would benefit from a blurb.
> >
> > [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#stay-out-of-texas
>
> Maxim Khutornenko wrote:
> I can't force myself into justifying Builder here quite yet but I am fine with improving readabilty at call sites. Done.
FWIW there's an option in IntelliJ to do this: https://www.jetbrains.com/idea/help/replace-constructor-with-builder.html
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73642
-----------------------------------------------------------
On Feb. 23, 2015, 1:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 1:03 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/#review73642
-----------------------------------------------------------
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java
<https://reviews.apache.org/r/30895/#comment120039>
This is bound to become a Texas constructor [1].
Please consider using the builder pattern instead to improve readability at the call sites.
Brief docs on these would be helpful too. Specifically, `clusterUtilization` would benefit from a blurb.
[1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#stay-out-of-texas
- Bill Farner
On Feb. 23, 2015, 7:34 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30895/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 7:34 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
>
> Original RB: https://reviews.apache.org/r/28617/
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
>
> Diff: https://reviews.apache.org/r/30895/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 30895: Offer filtering for static vetoes. Part 4
of 4: Benchmarks.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30895/
-----------------------------------------------------------
(Updated Feb. 23, 2015, 7:34 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Bill's comments.
Repository: aurora
Description
-------
Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling.
Original RB: https://reviews.apache.org/r/28617/
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61
src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25
Diff: https://reviews.apache.org/r/30895/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko