Re: Review Request 35587: Suppress task reconciliation status update logging.
On June 18, 2015, 4:42 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java, line 314 https://reviews.apache.org/r/35587/diff/1/?file=986660#file986660line314 What's being tested here? If i'm reading correctly, this test case would pass before the patch. Maxim Khutornenko wrote: This is mostly for test coverage similarly to the testOfferFirstAcceptsFineLogging above. Asserting a message would require replacing current anonymous logger with a mock and deeper refactoring. Do you think it's worth it here? You could pass a mock/stub `Logger` and expect the behavior (if you do, please be vague with expectations to avoid coupling to log message strings, etc). If that's not to your liking, you could punt. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88388 --- On June 18, 2015, 5:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 5:04 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35587: Suppress task reconciliation status update logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 5:04 p.m.) Review request for Aurora and Bill Farner. Changes --- Bill's comments. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35587: Suppress task reconciliation status update logging.
On June 18, 2015, 4:42 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java, line 314 https://reviews.apache.org/r/35587/diff/1/?file=986660#file986660line314 What's being tested here? If i'm reading correctly, this test case would pass before the patch. This is mostly for test coverage similarly to the testOfferFirstAcceptsFineLogging above. Asserting a message would require replacing current anonymous logger with a mock and deeper refactoring. Do you think it's worth it here? On June 18, 2015, 4:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 198 https://reviews.apache.org/r/35587/diff/1/?file=986659#file986659line198 A bit more context will help future readers. You could mention that reconciliation is expected to result in many no-op status updates. Sure, done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88388 --- On June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 1:02 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35587: Suppress task reconciliation status update logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88388 --- src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java (line 198) https://reviews.apache.org/r/35587/#comment140903 A bit more context will help future readers. You could mention that reconciliation is expected to result in many no-op status updates. src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java (line 314) https://reviews.apache.org/r/35587/#comment140904 What's being tested here? If i'm reading correctly, this test case would pass before the patch. - Bill Farner On June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 1:02 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35639: Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35639/#review88477 --- Ship it! Ship It! - Bill Farner On June 19, 2015, 1:21 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35639/ --- (Updated June 19, 2015, 1:21 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1361 https://issues.apache.org/jira/browse/AURORA-1361 Repository: aurora Description --- Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES. Diffs - src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java ccdbc029f3ce2ef51b777bfc06ee31ba78254d8a src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java 9ed1dba04667df52ba1c1538709a043308828763 Diff: https://reviews.apache.org/r/35639/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 35639: Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35639/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-1361 https://issues.apache.org/jira/browse/AURORA-1361 Repository: aurora Description --- Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES. Diffs - src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java ccdbc029f3ce2ef51b777bfc06ee31ba78254d8a src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java 9ed1dba04667df52ba1c1538709a043308828763 Diff: https://reviews.apache.org/r/35639/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35587: Suppress task reconciliation status update logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88415 --- Ship it! Master (af7e1a7) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On June 18, 2015, 5:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 5:04 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35613: Fixing broken gradle dependency scanner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/#review88447 --- Master (e271634) is red with this patch. ./build-support/jenkins/build.sh :compileJava Download https://repo1.maven.org/maven2/org/mybatis/mybatis/3.3.0/mybatis-3.3.0.pom Download https://repo1.maven.org/maven2/org/mybatis/mybatis-parent/24/mybatis-parent-24.pom Download https://repo1.maven.org/maven2/org/slf4j/slf4j-jdk14/1.7.12/slf4j-jdk14-1.7.12.pom Download https://repo1.maven.org/maven2/org/mybatis/mybatis/3.3.0/mybatis-3.3.0.jar Download https://repo1.maven.org/maven2/org/slf4j/slf4j-jdk14/1.7.12/slf4j-jdk14-1.7.12.jar Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2 :processResources :classes :jar :startScripts :distTar :distZip :assemble :compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API. Note: Recompile with -Xlint:deprecation for details. :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java:40: Line is longer than 100 characters (found 118). [ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java:41: Line is longer than 100 characters (found 119). [ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java:42: Line is longer than 100 characters (found 119). [ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java:43: Line is longer than 100 characters (found 110). [ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java:25: 'org.apache.shiro.subject.Subject' should be separated from previous imports. FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':checkstyleMain'. Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 2 mins 37.77 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On June 18, 2015, 10:12 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/ --- (Updated June 18, 2015, 10:12 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Our current scanner plugin version does not work with gradle 2.4. Upgrading plugin version and a few other deps (those that did not conflict with a mighty twitter.commons package): - junit: https://github.com/junit-team/junit/blob/master/doc/ReleaseNotes4.12.md - gson: https://sites.google.com/site/gson/gson-roadmap - slf4j: http://www.slf4j.org/news.html - mybatis: https://github.com/mybatis/mybatis-3/releases Diffs - build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc Diff: https://reviews.apache.org/r/35613/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Review Request 35627: Explicitly bind SessionContext.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-1352 https://issues.apache.org/jira/browse/AURORA-1352 Repository: aurora Description --- Explicitly bind SessionContext. Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java c89ff0fed60af04f04177e9cdfba6a24b62c2e97 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 57132ac480702c93934ada198475203b0648ad6a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 0a842cb9cdd266690b2d3103126e831fe07b1735 src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 4d6043a402a761fe44239e33b83c1c8872fe7068 Diff: https://reviews.apache.org/r/35627/diff/ Testing --- Ran kerberos e2e test and verified audit messages in UI. Thanks, Kevin Sweeney
Re: Review Request 35627: Explicitly bind SessionContext.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/ --- (Updated June 18, 2015, 2:40 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1352 https://issues.apache.org/jira/browse/AURORA-1352 Repository: aurora Description (updated) --- Explicitly bind SessionContext. Discussion on bugs like this and a potential solution (enabling `requireExplicitBindings`) are here: https://github.com/google/guice/issues/740 Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java c89ff0fed60af04f04177e9cdfba6a24b62c2e97 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 57132ac480702c93934ada198475203b0648ad6a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 0a842cb9cdd266690b2d3103126e831fe07b1735 src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 4d6043a402a761fe44239e33b83c1c8872fe7068 Diff: https://reviews.apache.org/r/35627/diff/ Testing --- Ran kerberos e2e test. Thanks, Kevin Sweeney
Re: Review Request 35627: Explicitly bind SessionContext.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/#review88440 --- Ship it! src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java (line 40) https://reviews.apache.org/r/35627/#comment140957 Given how non-obvious this was, it deserves a comment to indicate why this is necessary. - Bill Farner On June 18, 2015, 9:40 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/ --- (Updated June 18, 2015, 9:40 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1352 https://issues.apache.org/jira/browse/AURORA-1352 Repository: aurora Description --- Explicitly bind SessionContext. Discussion on bugs like this and a potential solution (enabling `requireExplicitBindings`) are here: https://github.com/google/guice/issues/740 Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java c89ff0fed60af04f04177e9cdfba6a24b62c2e97 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 57132ac480702c93934ada198475203b0648ad6a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 0a842cb9cdd266690b2d3103126e831fe07b1735 src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 4d6043a402a761fe44239e33b83c1c8872fe7068 Diff: https://reviews.apache.org/r/35627/diff/ Testing --- Ran kerberos e2e test. Thanks, Kevin Sweeney
Review Request 35613: Fixing broken gradle dependency scanner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Our current scanner plugin version does not work with gradle 2.4. Upgrading plugin version and a few other deps (those that did not conflict with a mighty twitter.commons package): - junit: https://github.com/junit-team/junit/blob/master/doc/ReleaseNotes4.12.md - gson: https://sites.google.com/site/gson/gson-roadmap - slf4j: http://www.slf4j.org/news.html - mybatis: https://github.com/mybatis/mybatis-3/releases Diffs - build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc Diff: https://reviews.apache.org/r/35613/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 35627: Explicitly bind SessionContext.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/ --- (Updated June 18, 2015, 2:55 p.m.) Review request for Aurora and Bill Farner. Changes --- Add explanatory comment, use interface in SessionValidator as well. Bugs: AURORA-1352 https://issues.apache.org/jira/browse/AURORA-1352 Repository: aurora Description --- Explicitly bind SessionContext. Discussion on bugs like this and a potential solution (enabling `requireExplicitBindings`) are here: https://github.com/google/guice/issues/740 Diffs (updated) - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java c89ff0fed60af04f04177e9cdfba6a24b62c2e97 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 57132ac480702c93934ada198475203b0648ad6a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 0a842cb9cdd266690b2d3103126e831fe07b1735 src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 4d6043a402a761fe44239e33b83c1c8872fe7068 Diff: https://reviews.apache.org/r/35627/diff/ Testing --- Ran kerberos e2e test. Thanks, Kevin Sweeney
Re: Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/#review88452
---
Ship it!
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml (line
167)
https://reviews.apache.org/r/35630/#comment140987
Is DISTINCT necessary given the Set result type?
- Maxim Khutornenko
On June 18, 2015, 10:39 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
(Updated June 18, 2015, 10:39 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Re: Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
On June 18, 2015, 11:08 p.m., Maxim Khutornenko wrote:
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml,
line 167
https://reviews.apache.org/r/35630/diff/1/?file=987636#file987636line167
Is DISTINCT necessary given the Set result type?
Functionally it's not necessary, but why not dedupe as low in the stack as we
can? If the database is on a separate host, for example, we save on the
network.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/#review88452
---
On June 18, 2015, 10:39 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
(Updated June 18, 2015, 10:39 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Re: Review Request 35633: Fixing stylecheck errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35633/#review88456 --- Ship it! Ship It! - Bill Farner On June 18, 2015, 11:15 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35633/ --- (Updated June 18, 2015, 11:15 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Looks like regression from: https://reviews.apache.org/r/35627/ Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 010e7146799381518436d410ce8272edbe3e0df9 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 0226144ffecec611200077e3ec66ede6cee58a3a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java a8badd40f63433216d809cbbebe79c15c9427571 Diff: https://reviews.apache.org/r/35633/diff/ Testing --- Thanks, Maxim Khutornenko
Review Request 35633: Fixing stylecheck errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35633/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Looks like regression from: https://reviews.apache.org/r/35627/ Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 010e7146799381518436d410ce8272edbe3e0df9 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 0226144ffecec611200077e3ec66ede6cee58a3a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java a8badd40f63433216d809cbbebe79c15c9427571 Diff: https://reviews.apache.org/r/35633/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
On June 18, 2015, 11:08 p.m., Maxim Khutornenko wrote:
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml,
line 167
https://reviews.apache.org/r/35630/diff/1/?file=987636#file987636line167
Is DISTINCT necessary given the Set result type?
Bill Farner wrote:
Functionally it's not necessary, but why not dedupe as low in the stack
as we can? If the database is on a separate host, for example, we save on
the network.
There is already a UNIQUE constraint on the job_keys table, so DISTINCT is not
really necessary anyway.
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/#review88452
---
On June 18, 2015, 10:39 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
(Updated June 18, 2015, 10:39 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Re: Review Request 35613: Fixing broken gradle dependency scanner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/#review88458 --- @ReviewBot retry - Maxim Khutornenko On June 18, 2015, 10:12 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/ --- (Updated June 18, 2015, 10:12 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Our current scanner plugin version does not work with gradle 2.4. Upgrading plugin version and a few other deps (those that did not conflict with a mighty twitter.commons package): - junit: https://github.com/junit-team/junit/blob/master/doc/ReleaseNotes4.12.md - gson: https://sites.google.com/site/gson/gson-roadmap - slf4j: http://www.slf4j.org/news.html - mybatis: https://github.com/mybatis/mybatis-3/releases Diffs - build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc Diff: https://reviews.apache.org/r/35613/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
On June 18, 2015, 11:08 p.m., Maxim Khutornenko wrote:
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml,
line 167
https://reviews.apache.org/r/35630/diff/1/?file=987636#file987636line167
Is DISTINCT necessary given the Set result type?
Bill Farner wrote:
Functionally it's not necessary, but why not dedupe as low in the stack
as we can? If the database is on a separate host, for example, we save on
the network.
Maxim Khutornenko wrote:
There is already a UNIQUE constraint on the job_keys table, so DISTINCT
is not really necessary anyway.
Closing the loop from offline discussion - the UNIQUE constraint is not
relevant since we're doing an INNER JOIN against job_keys.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/#review88452
---
On June 18, 2015, 10:39 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
(Updated June 18, 2015, 10:39 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Re: Review Request 35613: Fixing broken gradle dependency scanner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/#review88464 --- Ship it! Ship It! - Kevin Sweeney On June 18, 2015, 3:12 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35613/ --- (Updated June 18, 2015, 3:12 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Our current scanner plugin version does not work with gradle 2.4. Upgrading plugin version and a few other deps (those that did not conflict with a mighty twitter.commons package): - junit: https://github.com/junit-team/junit/blob/master/doc/ReleaseNotes4.12.md - gson: https://sites.google.com/site/gson/gson-roadmap - slf4j: http://www.slf4j.org/news.html - mybatis: https://github.com/mybatis/mybatis-3/releases Diffs - build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc Diff: https://reviews.apache.org/r/35613/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 35633: Fixing stylecheck errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35633/#review88465 --- Ship it! Ship It! - Kevin Sweeney On June 18, 2015, 4:15 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35633/ --- (Updated June 18, 2015, 4:15 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Looks like regression from: https://reviews.apache.org/r/35627/ Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 010e7146799381518436d410ce8272edbe3e0df9 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 0226144ffecec611200077e3ec66ede6cee58a3a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java a8badd40f63433216d809cbbebe79c15c9427571 Diff: https://reviews.apache.org/r/35633/diff/ Testing --- Thanks, Maxim Khutornenko
