Re: Review Request 28742: Simplify logging in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review63976 --- Ship it! Master (905137f) 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 Dec. 5, 2014, 7:43 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/ --- (Updated Dec. 5, 2014, 7:43 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-919 https://issues.apache.org/jira/browse/AURORA-919 Repository: aurora Description --- This patch makes multiple changes to simplify the logging done in the Aurora client: 1. Remove the TRANSCRIPT log level and replaced all instances with the standard Python DEBUG level. 2. Remove the custom aurora_client logger. This logger was designed to give each invocation of the client a unique id and record the username of the user with the intention that a hook could take this information and ship it to the cluster administer. However a hook could capture logs by adding a handler to the root log handler and generate a unique id itself. 3. Remove the 'print_log' method of the context and replaced all callers with the standard python logging facilities. 4. Removed duplicate printing/logging messages by just printing the information to the user. 5. Removed the custom PlainFormatter implementation and replaced it with Python's default formatter. 6. Replaced the --verbose-logging and --logging-level flags with a single --verbose/-v flag which enables DEBUG logging. Without this flag the user sees INFO and up. Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/__init__.py 6e553d8af459e575b2d62282a3bc0d1e266203d8 src/main/python/apache/aurora/client/cli/command_hooks.py aa850bf941bede1d3bd8aae4811cb094ba77965f src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/logsetup.py 55d99c42f643910db0bf3c24022596383e160276 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/task.py 91175facdc8c9fd59ab66781f86ee8b5940a src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_task.py c69a624ec7063973d365846f7df3516047ceeb68 Diff: https://reviews.apache.org/r/28742/diff/ Testing --- ./pants ./src/test/python/apache/aurora:: vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora jobs=[devcluster/www-data/prod/hello] vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Creating job hello INFO:root:Checking status of devcluster/www-data/prod/hello job create succeeded: job url=http://192.168.33.7:8081/scheduler/www-data/prod/hello vagrant@192:~$ aurora job list devcluster/* Must supply one of the following commands: cancel_update, create, diff, get_quota, help, inspect, kill, killall, list_jobs, open, restart, run, ssh, start_cron, status, update, version vagrant@192:~$ aurora2 job list devcluster/* INFO:root:Retrieving jobs for role None devcluster/www-data/prod/hello vagrant@192:~$ aurora2 job update devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Updating job: hello INFO:root:Instances to update: [0] INFO:root:Processing in parallel with 1 worker thread(s) INFO:root:Examining instance: 0 INFO:root:Skipping unchanged instance: 0 INFO:root:Update successful Update completed successfully vagrant@192:~$ aurora2 job killall devcluster/www-data/prod/hello INFO:root:Checking status of devcluster/www-data/prod/hello INFO:root:Killing tasks for job: devcluster/www-data/prod/hello INFO:root:Instances to be killed: [0] Successfully killed shards [0] job killall succeeded Thanks, Zameer Manji
Re: Review Request 28738: Remove unused DefaultServlet subclass.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28738/#review64016 --- Ship it! Ship It! - Maxim Khutornenko On Dec. 5, 2014, 4:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28738/ --- (Updated Dec. 5, 2014, 4:06 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Remove unused DefaultServlet subclass. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 424859867d7bd1aafefe406f455b831246c1cca5 Diff: https://reviews.apache.org/r/28738/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 5, 2014, 6:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Rebased. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs (updated) - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28738: Remove unused DefaultServlet subclass.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28738/ --- (Updated Dec. 5, 2014, 6:33 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- Remove unused DefaultServlet subclass. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 424859867d7bd1aafefe406f455b831246c1cca5 Diff: https://reviews.apache.org/r/28738/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/#review64056 --- Ship it! Master (40d2718) 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 Dec. 5, 2014, 6:32 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 5, 2014, 6:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28710/#review63897
---
build.gradle
https://reviews.apache.org/r/28710/#comment106210
This should be unneeded.
build.gradle
https://reviews.apache.org/r/28710/#comment106452
im not convinced you want to apply this to the api subproject.
build.gradle
https://reviews.apache.org/r/28710/#comment106449
Do we want to apply jmh to the 'api' project? I'd think we'd want to add
the root. That will give you the benefit of not having to disable this task on
the api project at all.
build.gradle
https://reviews.apache.org/r/28710/#comment106451
should this be a test scope? also should this be in the root project
instead of the api project?
build.gradle
https://reviews.apache.org/r/28710/#comment106454
Does
```groovy
tasks('jmh') {
//...
}
```
work?
- Kevin Sweeney
On Dec. 5, 2014, 10:32 a.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28710/
---
(Updated Dec. 5, 2014, 10:32 a.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Repository: aurora
Description
---
This RB is superseding the https://reviews.apache.org/r/28474/.
I have spent some time researching the available microbenchmark frameworks
and JMH [1] came as a clear winner:
- Active development trail [2]
- Advanced featureset and built-in optimizations improving accuracy and
consistency [3]
- Well documented set of examples [4]
- Large community experience and collective wisdom.
This RB adds gradle support for running JMH benchmarks and is relying on JMH
gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command.
[1] - http://openjdk.java.net/projects/code-tools/jmh/
[2] - http://hg.openjdk.java.net/code-tools/jmh/
[3] -
https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ
[4] -
http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples
[5] - https://github.com/melix/jmh-gradle-plugin
Diffs
-
build.gradle 152ba631e2dd07f0306e58e355274e10a4128140
config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f
src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java
PRE-CREATION
Diff: https://reviews.apache.org/r/28710/diff/
Testing
---
$ ./gradlew jmh
Sample results generated:
```
# VM invoker:
/Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
# VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en
-Duser.variant
# Warmup: 1 iterations, 1 s each
# Measurement: 3 iterations, 1 s each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Throughput, ops/time
# Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example
# Run progress: 0.00% complete, ETA 00:00:04
# Fork: 1 of 1
# Warmup Iteration 1: 3156839103.911 ops/s
Iteration 1: 544897.411 ops/s
Iteration 2: 3357230627.218 ops/s
Iteration 3: 3461073727.560 ops/s
Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average]
Statistics: (min, avg, max) = (544897.411, 3383949750.729,
3461073727.560), stdev = 67833135.714
Confidence interval (99.9%): [2146420835.212, 4621478666.247]
# Run complete. Total time: 00:00:05
Benchmark Mode Samples Score
Error Units
o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ±
1237528915.517 ops/s
```
Thanks,
Maxim Khutornenko
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/#review64066 --- Not to bikeshed, but do either of you have a sense for what it would take to put the benchmarking code into a gradle subproject? My sense is that would avoid further complicating the root build.gradle, but i'm not familiar enough with gradle to know what, if any, hurdles that would present. - Bill Farner On Dec. 5, 2014, 6:32 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 5, 2014, 6:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
On Dec. 5, 2014, 8:06 p.m., Bill Farner wrote: Not to bikeshed, but do either of you have a sense for what it would take to put the benchmarking code into a gradle subproject? My sense is that would avoid further complicating the root build.gradle, but i'm not familiar enough with gradle to know what, if any, hurdles that would present. There is not much to hide in a subproject as all you see here is configuring the plugin to run with our gradle. I don't see any sane way of hiding it in a subproject short of reimplementing the plugin ourselves. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/#review64066 --- On Dec. 5, 2014, 6:32 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 5, 2014, 6:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
On Dec. 5, 2014, 8:06 p.m., Bill Farner wrote: Not to bikeshed, but do either of you have a sense for what it would take to put the benchmarking code into a gradle subproject? My sense is that would avoid further complicating the root build.gradle, but i'm not familiar enough with gradle to know what, if any, hurdles that would present. Maxim Khutornenko wrote: There is not much to hide in a subproject as all you see here is configuring the plugin to run with our gradle. I don't see any sane way of hiding it in a subproject short of reimplementing the plugin ourselves. Kevin - do you agree with this? You're more versed in subprojects than i am, so i'll defer to your judgement. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/#review64066 --- On Dec. 5, 2014, 6:32 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 5, 2014, 6:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28742: Simplify logging in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review64075 --- What would it take to massage the log format? I'd love to drop the `INFO:root:` prefix to every line. Personally, i find it noisy for a command line tool to include the log level on each line of output, though others may feel differently. However, i'm pretty firm on omitting the `root` part. - Bill Farner On Dec. 5, 2014, 7:43 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/ --- (Updated Dec. 5, 2014, 7:43 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-919 https://issues.apache.org/jira/browse/AURORA-919 Repository: aurora Description --- This patch makes multiple changes to simplify the logging done in the Aurora client: 1. Remove the TRANSCRIPT log level and replaced all instances with the standard Python DEBUG level. 2. Remove the custom aurora_client logger. This logger was designed to give each invocation of the client a unique id and record the username of the user with the intention that a hook could take this information and ship it to the cluster administer. However a hook could capture logs by adding a handler to the root log handler and generate a unique id itself. 3. Remove the 'print_log' method of the context and replaced all callers with the standard python logging facilities. 4. Removed duplicate printing/logging messages by just printing the information to the user. 5. Removed the custom PlainFormatter implementation and replaced it with Python's default formatter. 6. Replaced the --verbose-logging and --logging-level flags with a single --verbose/-v flag which enables DEBUG logging. Without this flag the user sees INFO and up. Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/__init__.py 6e553d8af459e575b2d62282a3bc0d1e266203d8 src/main/python/apache/aurora/client/cli/command_hooks.py aa850bf941bede1d3bd8aae4811cb094ba77965f src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/logsetup.py 55d99c42f643910db0bf3c24022596383e160276 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/task.py 91175facdc8c9fd59ab66781f86ee8b5940a src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_task.py c69a624ec7063973d365846f7df3516047ceeb68 Diff: https://reviews.apache.org/r/28742/diff/ Testing --- ./pants ./src/test/python/apache/aurora:: vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora jobs=[devcluster/www-data/prod/hello] vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Creating job hello INFO:root:Checking status of devcluster/www-data/prod/hello job create succeeded: job url=http://192.168.33.7:8081/scheduler/www-data/prod/hello vagrant@192:~$ aurora job list devcluster/* Must supply one of the following commands: cancel_update, create, diff, get_quota, help, inspect, kill, killall, list_jobs, open, restart, run, ssh, start_cron, status, update, version vagrant@192:~$ aurora2 job list devcluster/* INFO:root:Retrieving jobs for role None devcluster/www-data/prod/hello vagrant@192:~$ aurora2 job update devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Updating job: hello INFO:root:Instances to update: [0] INFO:root:Processing in parallel with 1 worker thread(s) INFO:root:Examining instance: 0 INFO:root:Skipping unchanged instance: 0 INFO:root:Update successful Update completed successfully vagrant@192:~$ aurora2 job killall devcluster/www-data/prod/hello INFO:root:Checking status of devcluster/www-data/prod/hello INFO:root:Killing tasks for job: devcluster/www-data/prod/hello INFO:root:Instances to be killed: [0] Successfully killed shards [0] job killall succeeded Thanks, Zameer Manji
Re: Review Request 28742: Simplify logging in the client.
On Dec. 5, 2014, 12:37 p.m., Bill Farner wrote: What would it take to massage the log format? I'd love to drop the `INFO:root:` prefix to every line. Personally, i find it noisy for a command line tool to include the log level on each line of output, though others may feel differently. However, i'm pretty firm on omitting the `root` part. Technically we don't need to do a lot of work to add a new log Formatter. However I think logging should be used for debugging purposes and maintaining the logger name (root/requests/etc) is very useful. If the logging info is annoying and we show it we should evaluate the logging level and if it should be visible during normal operation. I added a TODO to only show messages from WARN+. Currently it is INFO because the client side updater uses logging.info to send information to the user. I am willing to followup this diff with cleaning up the logging/printing so we don't use it to display useful information to the user during normal operation. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review64075 --- On Dec. 4, 2014, 11:43 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/ --- (Updated Dec. 4, 2014, 11:43 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-919 https://issues.apache.org/jira/browse/AURORA-919 Repository: aurora Description --- This patch makes multiple changes to simplify the logging done in the Aurora client: 1. Remove the TRANSCRIPT log level and replaced all instances with the standard Python DEBUG level. 2. Remove the custom aurora_client logger. This logger was designed to give each invocation of the client a unique id and record the username of the user with the intention that a hook could take this information and ship it to the cluster administer. However a hook could capture logs by adding a handler to the root log handler and generate a unique id itself. 3. Remove the 'print_log' method of the context and replaced all callers with the standard python logging facilities. 4. Removed duplicate printing/logging messages by just printing the information to the user. 5. Removed the custom PlainFormatter implementation and replaced it with Python's default formatter. 6. Replaced the --verbose-logging and --logging-level flags with a single --verbose/-v flag which enables DEBUG logging. Without this flag the user sees INFO and up. Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/__init__.py 6e553d8af459e575b2d62282a3bc0d1e266203d8 src/main/python/apache/aurora/client/cli/command_hooks.py aa850bf941bede1d3bd8aae4811cb094ba77965f src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/logsetup.py 55d99c42f643910db0bf3c24022596383e160276 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/task.py 91175facdc8c9fd59ab66781f86ee8b5940a src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_task.py c69a624ec7063973d365846f7df3516047ceeb68 Diff: https://reviews.apache.org/r/28742/diff/ Testing --- ./pants ./src/test/python/apache/aurora:: vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora jobs=[devcluster/www-data/prod/hello] vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Creating job hello INFO:root:Checking status of devcluster/www-data/prod/hello job create succeeded: job url=http://192.168.33.7:8081/scheduler/www-data/prod/hello vagrant@192:~$ aurora job list devcluster/* Must supply one of the following commands: cancel_update, create, diff, get_quota, help, inspect, kill, killall, list_jobs, open, restart, run, ssh, start_cron, status, update, version vagrant@192:~$ aurora2 job list devcluster/* INFO:root:Retrieving jobs for role None devcluster/www-data/prod/hello vagrant@192:~$ aurora2 job update
Re: Review Request 28742: Simplify logging in the client.
On Dec. 5, 2014, 8:37 p.m., Bill Farner wrote: What would it take to massage the log format? I'd love to drop the `INFO:root:` prefix to every line. Personally, i find it noisy for a command line tool to include the log level on each line of output, though others may feel differently. However, i'm pretty firm on omitting the `root` part. Zameer Manji wrote: Technically we don't need to do a lot of work to add a new log Formatter. However I think logging should be used for debugging purposes and maintaining the logger name (root/requests/etc) is very useful. If the logging info is annoying and we show it we should evaluate the logging level and if it should be visible during normal operation. I added a TODO to only show messages from WARN+. Currently it is INFO because the client side updater uses logging.info to send information to the user. I am willing to followup this diff with cleaning up the logging/printing so we don't use it to display useful information to the user during normal operation. The logger name might be useful to us, but i don't think it's useful to our users. Libraries and refactors are implementation details they should be insulated from. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review64075 --- On Dec. 5, 2014, 7:43 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/ --- (Updated Dec. 5, 2014, 7:43 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-919 https://issues.apache.org/jira/browse/AURORA-919 Repository: aurora Description --- This patch makes multiple changes to simplify the logging done in the Aurora client: 1. Remove the TRANSCRIPT log level and replaced all instances with the standard Python DEBUG level. 2. Remove the custom aurora_client logger. This logger was designed to give each invocation of the client a unique id and record the username of the user with the intention that a hook could take this information and ship it to the cluster administer. However a hook could capture logs by adding a handler to the root log handler and generate a unique id itself. 3. Remove the 'print_log' method of the context and replaced all callers with the standard python logging facilities. 4. Removed duplicate printing/logging messages by just printing the information to the user. 5. Removed the custom PlainFormatter implementation and replaced it with Python's default formatter. 6. Replaced the --verbose-logging and --logging-level flags with a single --verbose/-v flag which enables DEBUG logging. Without this flag the user sees INFO and up. Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/__init__.py 6e553d8af459e575b2d62282a3bc0d1e266203d8 src/main/python/apache/aurora/client/cli/command_hooks.py aa850bf941bede1d3bd8aae4811cb094ba77965f src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/logsetup.py 55d99c42f643910db0bf3c24022596383e160276 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/task.py 91175facdc8c9fd59ab66781f86ee8b5940a src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_task.py c69a624ec7063973d365846f7df3516047ceeb68 Diff: https://reviews.apache.org/r/28742/diff/ Testing --- ./pants ./src/test/python/apache/aurora:: vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora jobs=[devcluster/www-data/prod/hello] vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora INFO:root:Creating job hello INFO:root:Checking status of devcluster/www-data/prod/hello job create succeeded: job url=http://192.168.33.7:8081/scheduler/www-data/prod/hello vagrant@192:~$ aurora job list devcluster/* Must supply one of the following commands: cancel_update, create, diff, get_quota, help, inspect, kill, killall, list_jobs, open, restart, run, ssh,
Review Request 28771: Don't fall back to old command syntax in the new client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-782 https://issues.apache.org/jira/browse/AURORA-782 Repository: aurora Description --- The diff appears like a large delta to `cli/client.py`, but it is in fact a pure `git mv` of standalone_client.py to client.py, since the qualification no longer makes sense. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8d34bf71a225210f1fef8b5a69bc9f14b49d17bb src/main/python/apache/aurora/client/cli/client.py a1df788adf2c8066d207e6b70d7694f6975b8edd src/main/python/apache/aurora/client/cli/standalone_client.py 93120a15f5e1967b4067d3c068b9bc564f3fb432 Diff: https://reviews.apache.org/r/28771/diff/ Testing --- ``` ./pants build src/test/python/apache/aurora/client:all -vxs ./src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 28771: Don't fall back to old command syntax in the new client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/#review64086 --- Ship it! Ship It! - Zameer Manji On Dec. 5, 2014, 1:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/ --- (Updated Dec. 5, 2014, 1:12 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-782 https://issues.apache.org/jira/browse/AURORA-782 Repository: aurora Description --- The diff appears like a large delta to `cli/client.py`, but it is in fact a pure `git mv` of standalone_client.py to client.py, since the qualification no longer makes sense. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8d34bf71a225210f1fef8b5a69bc9f14b49d17bb src/main/python/apache/aurora/client/cli/client.py a1df788adf2c8066d207e6b70d7694f6975b8edd src/main/python/apache/aurora/client/cli/standalone_client.py 93120a15f5e1967b4067d3c068b9bc564f3fb432 Diff: https://reviews.apache.org/r/28771/diff/ Testing --- ``` ./pants build src/test/python/apache/aurora/client:all -vxs ./src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 28771: Don't fall back to old command syntax in the new client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/#review64087 --- Master (40d2718) 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 Dec. 5, 2014, 9:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/ --- (Updated Dec. 5, 2014, 9:12 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-782 https://issues.apache.org/jira/browse/AURORA-782 Repository: aurora Description --- The diff appears like a large delta to `cli/client.py`, but it is in fact a pure `git mv` of standalone_client.py to client.py, since the qualification no longer makes sense. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8d34bf71a225210f1fef8b5a69bc9f14b49d17bb src/main/python/apache/aurora/client/cli/client.py a1df788adf2c8066d207e6b70d7694f6975b8edd src/main/python/apache/aurora/client/cli/standalone_client.py 93120a15f5e1967b4067d3c068b9bc564f3fb432 Diff: https://reviews.apache.org/r/28771/diff/ Testing --- ``` ./pants build src/test/python/apache/aurora/client:all -vxs ./src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 28771: Don't fall back to old command syntax in the new client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/#review64103 --- Ship it! Ship It! - Maxim Khutornenko On Dec. 5, 2014, 9:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28771/ --- (Updated Dec. 5, 2014, 9:12 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-782 https://issues.apache.org/jira/browse/AURORA-782 Repository: aurora Description --- The diff appears like a large delta to `cli/client.py`, but it is in fact a pure `git mv` of standalone_client.py to client.py, since the qualification no longer makes sense. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8d34bf71a225210f1fef8b5a69bc9f14b49d17bb src/main/python/apache/aurora/client/cli/client.py a1df788adf2c8066d207e6b70d7694f6975b8edd src/main/python/apache/aurora/client/cli/standalone_client.py 93120a15f5e1967b4067d3c068b9bc564f3fb432 Diff: https://reviews.apache.org/r/28771/diff/ Testing --- ``` ./pants build src/test/python/apache/aurora/client:all -vxs ./src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 28617: Implemented offer filtering for tasks with static vetoes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/ --- (Updated Dec. 5, 2014, 10:57 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Improving banned offer lookup performance. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description (updated) --- Modified the task offer/task matching logic to skip offer matching for tasks previously vetoed statically. Preliminary testing in vagrant (see pictures below) shows close to 50% improvement in task scheduling performance. Update: Testing with JMH (https://reviews.apache.org/r/28710/ and https://reviews.apache.org/r/28731/) shows over 97% better perf: ``` Master with cluster fillup 0.9: Benchmark Mode SamplesScoreError Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 8291046.074 ± 145251.995 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 7522269.050 ± 142446.265 ns/op This RB with cluster fillup 0.9: Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 204171.046 ± 3800.124 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 215854.129 ± 8959.851 ns/op ``` Diffs (updated) - src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399 src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ead9d28100673440168a32d114ecaa15874978a6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/28617/diff/ Testing --- ./gradlew -Pq build Tested in vagrant File Attachments NoStaticVetoFiltering.png https://reviews.apache.org/media/uploaded/files/2014/12/03/7945c60b-4135-4016-a9bf-8d4815a4a573__NoStaticVetoFiltering.png StaticVetoFiltering.png https://reviews.apache.org/media/uploaded/files/2014/12/03/2f73b94a-5ba9-43b6-922e-e9e4ec18d0bb__StaticVetoFiltering.png Thanks, Maxim Khutornenko
Re: Review Request 28731: Implemented TaskScheduler benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28731/ --- (Updated Dec. 5, 2014, 11:02 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Adding cluster fillup to better simulate real cluster conditions. Repository: aurora Description --- Added baseline benchmarks for a few static veto cases. This is branched off of https://reviews.apache.org/r/28710 Diffs (updated) - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeEventSink.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ead9d28100673440168a32d114ecaa15874978a6 Diff: https://reviews.apache.org/r/28731/diff/ Testing --- Sample run on a local box: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 10 iterations, 1 s each # Measurement: 100 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Average time, time/op # Benchmark: org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example # Run progress: 0.00% complete, ETA 00:03:40 # Fork: 1 of 1 # Warmup Iteration 1: 104725900.000 ns/op # Warmup Iteration 2: 20439888.889 ns/op # Warmup Iteration 3: 13612185.185 ns/op # Warmup Iteration 4: 5856248.677 ns/op # Warmup Iteration 5: 5148014.019 ns/op # Warmup Iteration 6: 4972004.505 ns/op # Warmup Iteration 7: 4595562.500 ns/op # Warmup Iteration 8: 4639226.891 ns/op # Warmup Iteration 9: 4639521.008 ns/op # Warmup Iteration 10: 4660248.945 ns/op Iteration 1: 4718478.632 ns/op Iteration 2: 4800686.957 ns/op Iteration 3: 4438253.012 ns/op Iteration 4: 4935376.682 ns/op Iteration 5: 5236895.735 ns/op Iteration 6: 6439345.029 ns/op Iteration 7: 4792708.696 ns/op Iteration 8: 4579008.299 ns/op Iteration 9: 4681224.576 ns/op Iteration 10: 4651143.460 ns/op Iteration 11: 4394079.681 ns/op Iteration 12: 4390573.705 ns/op Iteration 13: 4363304.348 ns/op Iteration 14: 4419488.000 ns/op Iteration 15: 4869718.062 ns/op Iteration 16: 4600612.500 ns/op Iteration 17: 4475906.883 ns/op Iteration 18: 4710388.889 ns/op Iteration 19: 6137000.000 ns/op Iteration 20: 4467425.101 ns/op Iteration 21: 4348877.953 ns/op Iteration 22: 4666907.173 ns/op Iteration 23: 4431325.301 ns/op Iteration 24: 4463186.235 ns/op Iteration 25: 4580074.689 ns/op Iteration 26: 4467805.668 ns/op Iteration 27: 4886960.177 ns/op Iteration 28: 4858515.419 ns/op Iteration 29: 4734025.751 ns/op Iteration 30: 4718081.197 ns/op Iteration 31: 4711688.034 ns/op Iteration 32: 4462392.713 ns/op Iteration 33: 4683597.458 ns/op Iteration 34: 4755176.724 ns/op Iteration 35: 4530278.689 ns/op Iteration 36: 4392119.048 ns/op Iteration 37: 4659722.689 ns/op Iteration 38: 4805869.565 ns/op Iteration 39: 4398573.705 ns/op Iteration 40: 4572547.718 ns/op Iteration 41: 4643478.992 ns/op Iteration 42: 4545699.588 ns/op Iteration 43: 4670266.949 ns/op Iteration 44: 4506677.551 ns/op Iteration 45: 4574149.378 ns/op Iteration 46: 4743154.506 ns/op Iteration 47: 4777606.061 ns/op Iteration 48: 4481987.805 ns/op Iteration 49: 4840921.053 ns/op Iteration 50: 4637231.092 ns/op Iteration 51: 4515286.885 ns/op Iteration 52: 4372884.921 ns/op Iteration 53: 4385003.968 ns/op Iteration 54: 4619648.536 ns/op Iteration 55: 4557822.314 ns/op Iteration 56: 4594362.500 ns/op Iteration 57: 7870319.149 ns/op Iteration 58: 9764495.575 ns/op Iteration 59: 10302028.037 ns/op Iteration 60: 6043169.399 ns/op Iteration 61: 5523472.362 ns/op Iteration 62: 4513685.714 ns/op Iteration 63: 4361806.324 ns/op Iteration 64: 4388615.079 ns/op Iteration 65: 4548264.463 ns/op Iteration 66: 4710752.137 ns/op Iteration 67: 5688128.866 ns/op Iteration 68: 6109292.818 ns/op Iteration 69: 5896914.439 ns/op Iteration 70: 5986548.913 ns/op Iteration 71: 6093618.785 ns/op Iteration 72: 6051450.549 ns/op Iteration 73: 6237870.056 ns/op Iteration 74: 5954345.946 ns/op Iteration 75: 6460748.538 ns/op Iteration 76: 5929037.634 ns/op Iteration 77:
Re: Review Request 28617: Implemented offer filtering for tasks with static vetoes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/#review64111 --- Ship it! Master (40d2718) 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 Dec. 5, 2014, 10:57 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/ --- (Updated Dec. 5, 2014, 10:57 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Modified the task offer/task matching logic to skip offer matching for tasks previously vetoed statically. Preliminary testing in vagrant (see pictures below) shows close to 50% improvement in task scheduling performance. Update: Testing with JMH (https://reviews.apache.org/r/28710/ and https://reviews.apache.org/r/28731/) shows over 97% better perf: ``` Master with cluster fillup 0.9: Benchmark Mode SamplesScoreError Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 8291046.074 ± 145251.995 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 7522269.050 ± 142446.265 ns/op This RB with cluster fillup 0.9: Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 204171.046 ± 3800.124 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 215854.129 ± 8959.851 ns/op ``` Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399 src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ead9d28100673440168a32d114ecaa15874978a6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/28617/diff/ Testing --- ./gradlew -Pq build Tested in vagrant File Attachments NoStaticVetoFiltering.png https://reviews.apache.org/media/uploaded/files/2014/12/03/7945c60b-4135-4016-a9bf-8d4815a4a573__NoStaticVetoFiltering.png StaticVetoFiltering.png https://reviews.apache.org/media/uploaded/files/2014/12/03/2f73b94a-5ba9-43b6-922e-e9e4ec18d0bb__StaticVetoFiltering.png Thanks, Maxim Khutornenko
Re: Review Request 28731: Implemented TaskScheduler benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28731/#review64112 --- This patch does not apply cleanly on master (40d2718), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Dec. 5, 2014, 11:02 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28731/ --- (Updated Dec. 5, 2014, 11:02 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Added baseline benchmarks for a few static veto cases. This is branched off of https://reviews.apache.org/r/28710 Diffs - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeEventSink.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ead9d28100673440168a32d114ecaa15874978a6 Diff: https://reviews.apache.org/r/28731/diff/ Testing --- Sample run on a local box: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 10 iterations, 1 s each # Measurement: 100 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Average time, time/op # Benchmark: org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example # Run progress: 0.00% complete, ETA 00:03:40 # Fork: 1 of 1 # Warmup Iteration 1: 104725900.000 ns/op # Warmup Iteration 2: 20439888.889 ns/op # Warmup Iteration 3: 13612185.185 ns/op # Warmup Iteration 4: 5856248.677 ns/op # Warmup Iteration 5: 5148014.019 ns/op # Warmup Iteration 6: 4972004.505 ns/op # Warmup Iteration 7: 4595562.500 ns/op # Warmup Iteration 8: 4639226.891 ns/op # Warmup Iteration 9: 4639521.008 ns/op # Warmup Iteration 10: 4660248.945 ns/op Iteration 1: 4718478.632 ns/op Iteration 2: 4800686.957 ns/op Iteration 3: 4438253.012 ns/op Iteration 4: 4935376.682 ns/op Iteration 5: 5236895.735 ns/op Iteration 6: 6439345.029 ns/op Iteration 7: 4792708.696 ns/op Iteration 8: 4579008.299 ns/op Iteration 9: 4681224.576 ns/op Iteration 10: 4651143.460 ns/op Iteration 11: 4394079.681 ns/op Iteration 12: 4390573.705 ns/op Iteration 13: 4363304.348 ns/op Iteration 14: 4419488.000 ns/op Iteration 15: 4869718.062 ns/op Iteration 16: 4600612.500 ns/op Iteration 17: 4475906.883 ns/op Iteration 18: 4710388.889 ns/op Iteration 19: 6137000.000 ns/op Iteration 20: 4467425.101 ns/op Iteration 21: 4348877.953 ns/op Iteration 22: 4666907.173 ns/op Iteration 23: 4431325.301 ns/op Iteration 24: 4463186.235 ns/op Iteration 25: 4580074.689 ns/op Iteration 26: 4467805.668 ns/op Iteration 27: 4886960.177 ns/op Iteration 28: 4858515.419 ns/op Iteration 29: 4734025.751 ns/op Iteration 30: 4718081.197 ns/op Iteration 31: 4711688.034 ns/op Iteration 32: 4462392.713 ns/op Iteration 33: 4683597.458 ns/op Iteration 34: 4755176.724 ns/op Iteration 35: 4530278.689 ns/op Iteration 36: 4392119.048 ns/op Iteration 37: 4659722.689 ns/op Iteration 38: 4805869.565 ns/op Iteration 39: 4398573.705 ns/op Iteration 40: 4572547.718 ns/op Iteration 41: 4643478.992 ns/op Iteration 42: 4545699.588 ns/op Iteration 43: 4670266.949 ns/op Iteration 44: 4506677.551 ns/op Iteration 45: 4574149.378 ns/op Iteration 46: 4743154.506 ns/op Iteration 47: 4777606.061 ns/op Iteration 48: 4481987.805 ns/op Iteration 49: 4840921.053 ns/op Iteration 50: 4637231.092 ns/op Iteration 51: 4515286.885 ns/op Iteration 52: 4372884.921 ns/op Iteration 53: 4385003.968 ns/op Iteration 54: 4619648.536 ns/op Iteration 55: 4557822.314 ns/op Iteration 56: 4594362.500 ns/op Iteration 57: 7870319.149 ns/op Iteration 58: 9764495.575 ns/op Iteration 59: 10302028.037 ns/op Iteration 60: 6043169.399 ns/op Iteration 61: 5523472.362 ns/op
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28710/ --- (Updated Dec. 6, 2014, 12:33 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- CR comments. Repository: aurora Description --- This RB is superseding the https://reviews.apache.org/r/28474/. I have spent some time researching the available microbenchmark frameworks and JMH [1] came as a clear winner: - Active development trail [2] - Advanced featureset and built-in optimizations improving accuracy and consistency [3] - Well documented set of examples [4] - Large community experience and collective wisdom. This RB adds gradle support for running JMH benchmarks and is relying on JMH gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command. [1] - http://openjdk.java.net/projects/code-tools/jmh/ [2] - http://hg.openjdk.java.net/code-tools/jmh/ [3] - https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ [4] - http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples [5] - https://github.com/melix/jmh-gradle-plugin Diffs (updated) - build.gradle 152ba631e2dd07f0306e58e355274e10a4128140 config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java PRE-CREATION Diff: https://reviews.apache.org/r/28710/diff/ Testing --- $ ./gradlew jmh Sample results generated: ``` # VM invoker: /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant # Warmup: 1 iterations, 1 s each # Measurement: 3 iterations, 1 s each # Timeout: 10 min per iteration # Threads: 1 thread, will synchronize iterations # Benchmark mode: Throughput, ops/time # Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example # Run progress: 0.00% complete, ETA 00:00:04 # Fork: 1 of 1 # Warmup Iteration 1: 3156839103.911 ops/s Iteration 1: 544897.411 ops/s Iteration 2: 3357230627.218 ops/s Iteration 3: 3461073727.560 ops/s Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average] Statistics: (min, avg, max) = (544897.411, 3383949750.729, 3461073727.560), stdev = 67833135.714 Confidence interval (99.9%): [2146420835.212, 4621478666.247] # Run complete. Total time: 00:00:05 Benchmark Mode Samples Score Error Units o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ± 1237528915.517 ops/s ``` Thanks, Maxim Khutornenko
Re: Review Request 28710: Adding JMH framework support for scheduler performance analysis.
On Dec. 5, 2014, 7:42 p.m., Kevin Sweeney wrote:
build.gradle, line 53
https://reviews.apache.org/r/28710/diff/1/?file=782698#file782698line53
This should be unneeded.
Dropped.
On Dec. 5, 2014, 7:42 p.m., Kevin Sweeney wrote:
build.gradle, line 103
https://reviews.apache.org/r/28710/diff/3/?file=783833#file783833line103
im not convinced you want to apply this to the api subproject.
This was only needed to make :api recognize .jmh configuration. However, played
a bit more and found a way to reference parent configuration. Dropped.
On Dec. 5, 2014, 7:42 p.m., Kevin Sweeney wrote:
build.gradle, line 156
https://reviews.apache.org/r/28710/diff/3/?file=783833#file783833line156
Do we want to apply jmh to the 'api' project? I'd think we'd want to
add the root. That will give you the benefit of not having to disable this
task on the api project at all.
Gone.
On Dec. 5, 2014, 7:42 p.m., Kevin Sweeney wrote:
build.gradle, line 165
https://reviews.apache.org/r/28710/diff/3/?file=783833#file783833line165
should this be a test scope? also should this be in the root project
instead of the api project?
It's only needed to resolve /jmh deps. Test scope is irrelevant here.
On Dec. 5, 2014, 7:42 p.m., Kevin Sweeney wrote:
build.gradle, line 458
https://reviews.apache.org/r/28710/diff/3/?file=783833#file783833line458
Does
```groovy
tasks('jmh') {
//...
}
```
work?
It does not: ` Could not find method tasks() for arguments [jmh] on root
project 'aurora'.`
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28710/#review63897
---
On Dec. 5, 2014, 6:32 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28710/
---
(Updated Dec. 5, 2014, 6:32 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Repository: aurora
Description
---
This RB is superseding the https://reviews.apache.org/r/28474/.
I have spent some time researching the available microbenchmark frameworks
and JMH [1] came as a clear winner:
- Active development trail [2]
- Advanced featureset and built-in optimizations improving accuracy and
consistency [3]
- Well documented set of examples [4]
- Large community experience and collective wisdom.
This RB adds gradle support for running JMH benchmarks and is relying on JMH
gradle plugin [5]. The benchmarks are run via `./gradlew jmh` command.
[1] - http://openjdk.java.net/projects/code-tools/jmh/
[2] - http://hg.openjdk.java.net/code-tools/jmh/
[3] -
https://groups.google.com/forum/#!msg/mechanical-sympathy/m4opvy4xq3U/7lY8x8SvHgwJ
[4] -
http://hg.openjdk.java.net/code-tools/jmh/file/adb6047266d8/jmh-samples/src/main/java/org/openjdk/jmh/samples
[5] - https://github.com/melix/jmh-gradle-plugin
Diffs
-
build.gradle 152ba631e2dd07f0306e58e355274e10a4128140
config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f
src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java
PRE-CREATION
Diff: https://reviews.apache.org/r/28710/diff/
Testing
---
$ ./gradlew jmh
Sample results generated:
```
# VM invoker:
/Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
# VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en
-Duser.variant
# Warmup: 1 iterations, 1 s each
# Measurement: 3 iterations, 1 s each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Throughput, ops/time
# Benchmark: org.apache.aurora.benchmark.SchedulerBenchmark.example
# Run progress: 0.00% complete, ETA 00:00:04
# Fork: 1 of 1
# Warmup Iteration 1: 3156839103.911 ops/s
Iteration 1: 544897.411 ops/s
Iteration 2: 3357230627.218 ops/s
Iteration 3: 3461073727.560 ops/s
Result: 3383949750.729 ±(99.9%) 1237528915.517 ops/s [Average]
Statistics: (min, avg, max) = (544897.411, 3383949750.729,
3461073727.560), stdev = 67833135.714
Confidence interval (99.9%): [2146420835.212, 4621478666.247]
# Run complete. Total time: 00:00:05
Benchmark Mode Samples Score
Error Units
o.a.a.b.SchedulerBenchmark.examplethrpt3 3383949750.729 ±
1237528915.517 ops/s
```
Thanks,
Maxim Khutornenko
Re: Review Request 28742: Simplify logging in the client.
On Dec. 5, 2014, 8:37 p.m., Bill Farner wrote: What would it take to massage the log format? I'd love to drop the `INFO:root:` prefix to every line. Personally, i find it noisy for a command line tool to include the log level on each line of output, though others may feel differently. However, i'm pretty firm on omitting the `root` part. Zameer Manji wrote: Technically we don't need to do a lot of work to add a new log Formatter. However I think logging should be used for debugging purposes and maintaining the logger name (root/requests/etc) is very useful. If the logging info is annoying and we show it we should evaluate the logging level and if it should be visible during normal operation. I added a TODO to only show messages from WARN+. Currently it is INFO because the client side updater uses logging.info to send information to the user. I am willing to followup this diff with cleaning up the logging/printing so we don't use it to display useful information to the user during normal operation. Bill Farner wrote: The logger name might be useful to us, but i don't think it's useful to our users. Libraries and refactors are implementation details they should be insulated from. Zameer Manji wrote: Our users shouldn't see log messages in regular operation so I don't see the need to remove the logger name from the message and make our lives harder when debugging. Per offline discussion, please adapt this change to match the current client output so we can tackle the broader output discussion separately. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review64075 --- On Dec. 5, 2014, 7:43 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/ --- (Updated Dec. 5, 2014, 7:43 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-919 https://issues.apache.org/jira/browse/AURORA-919 Repository: aurora Description --- This patch makes multiple changes to simplify the logging done in the Aurora client: 1. Remove the TRANSCRIPT log level and replaced all instances with the standard Python DEBUG level. 2. Remove the custom aurora_client logger. This logger was designed to give each invocation of the client a unique id and record the username of the user with the intention that a hook could take this information and ship it to the cluster administer. However a hook could capture logs by adding a handler to the root log handler and generate a unique id itself. 3. Remove the 'print_log' method of the context and replaced all callers with the standard python logging facilities. 4. Removed duplicate printing/logging messages by just printing the information to the user. 5. Removed the custom PlainFormatter implementation and replaced it with Python's default formatter. 6. Replaced the --verbose-logging and --logging-level flags with a single --verbose/-v flag which enables DEBUG logging. Without this flag the user sees INFO and up. Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/__init__.py 6e553d8af459e575b2d62282a3bc0d1e266203d8 src/main/python/apache/aurora/client/cli/command_hooks.py aa850bf941bede1d3bd8aae4811cb094ba77965f src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/logsetup.py 55d99c42f643910db0bf3c24022596383e160276 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/task.py 91175facdc8c9fd59ab66781f86ee8b5940a src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_logging.py 6285fbb07442291c2dc4096e68eb285c98994097 src/test/python/apache/aurora/client/cli/test_plugins.py 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a src/test/python/apache/aurora/client/cli/test_task.py c69a624ec7063973d365846f7df3516047ceeb68 Diff: https://reviews.apache.org/r/28742/diff/ Testing --- ./pants ./src/test/python/apache/aurora:: vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora jobs=[devcluster/www-data/prod/hello] vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello ./aurora/examples/jobs/hello_world.aurora
