You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/03/07 23:22:05 UTC
Review Request 44471: Implementing dbsnapshotting
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/
-----------------------------------------------------------
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-1627
https://issues.apache.org/jira/browse/AURORA-1627
Repository: aurora
Description
-------
H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
`SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
Also, moved checkstyle suppressions into annotations for more precise control.
Diffs
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
Diff: https://reviews.apache.org/r/44471/diff/
Testing
-------
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Thanks,
Maxim Khutornenko
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 7, 2016, 10:36 p.m., Bill Farner wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift, line 152
> > <https://reviews.apache.org/r/44471/diff/1/?file=1283482#file1283482line152>
> >
> > What's the motivation behind using `list<string>` here as opposed to `string`?
The reason is performance. We can't run the entire script as a single DML statement as it OOMs H2 with any reasonable DB size (e.g. couple thousand of updates). I found there are absolutely no perf issues when the entire script is split into chunks of a few rows per batch. Storing list as opposed to string makes batching easier and safer.
> On March 7, 2016, 10:36 p.m., Bill Farner wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift, line 154
> > <https://reviews.apache.org/r/44471/diff/1/?file=1283482#file1283482line154>
> >
> > I'm confused by this flag, is it needed or is it the same as `dbScript` being set/non-empty?
Unfortunately, having a non-empty dbScript is not enough to determine _how_ to handle task store restore. We need to know what's inside of that `dbScript` to properly handle db- vs. mem- task store cases. Consider this example: DBTaskStore: OFF, scheduler running with this patch. The `dbScript` will be populated but it will not have any tasks from TaskStore in it. So, deferring to dbsnapshotting alone would be dangerously wrong here.
I have covered possible cases in SnapshotStoreImplIT. Let me know if it's not clear.
> On March 7, 2016, 10:36 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java, line 110
> > <https://reviews.apache.org/r/44471/diff/1/?file=1283491#file1283491line110>
> >
> > I'd like to avoid a trend of suppressing checkstyle warnings. Consider either refactoring to address the warning, or tweak/remove the checkstyle rule.
I was split on whether to turn that rule off or suppress and decided to suppress instead. This rule does not seem to support Java 8 try-with-resources case well. Extracting a method here would only hurt readability in my opinion. I am fine turning it off as readability issues with nested TRYs are generally caught during code reviews.
> On March 7, 2016, 10:36 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java, line 113
> > <https://reviews.apache.org/r/44471/diff/1/?file=1283491#file1283491line113>
> >
> > I think i know the answer here, but please add a comment indicating why you are not letting mybatis run this query for you.
Added comment.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122391
-----------------------------------------------------------
On March 7, 2016, 10:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 10:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122391
-----------------------------------------------------------
api/src/main/thrift/org/apache/aurora/gen/storage.thrift (line 152)
<https://reviews.apache.org/r/44471/#comment184362>
What's the motivation behind using `list<string>` here as opposed to `string`?
api/src/main/thrift/org/apache/aurora/gen/storage.thrift (line 154)
<https://reviews.apache.org/r/44471/#comment184367>
I'm confused by this flag, is it needed or is it the same as `dbScript` being set/non-empty?
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java (line 109)
<https://reviews.apache.org/r/44471/#comment184365>
I'd like to avoid a trend of suppressing checkstyle warnings. Consider either refactoring to address the warning, or tweak/remove the checkstyle rule.
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java (line 112)
<https://reviews.apache.org/r/44471/#comment184366>
I think i know the answer here, but please add a comment indicating why you are not letting mybatis run this query for you.
- Bill Farner
On March 7, 2016, 2:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 2:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122420
-----------------------------------------------------------
Ship it!
Master (a91a759) 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 March 7, 2016, 11:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 11:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122554
-----------------------------------------------------------
Master (a91a759) is red with this patch.
./build-support/jenkins/build.sh
+ date
Tue Mar 8 17:54:34 UTC 2016
+ ./gradlew -Pq clean build
:buildSrc:clean UP-TO-DATE
:buildSrc:compileJava UP-TO-DATE
:buildSrc:compileGroovy
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build
FAILURE: Build failed with an exception.
* What went wrong:
A problem occurred configuring root project 'aurora'.
> Could not open proj class cache for build file '/home/jenkins/jenkins-slave/workspace/AuroraBot/build.gradle' (/home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj).
> Timeout waiting to lock proj class cache for build file '/home/jenkins/jenkins-slave/workspace/AuroraBot/build.gradle' (/home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj). It is currently in use by another Gradle instance.
Owner PID: unknown
Our PID: 10548
Owner Operation: unknown
Our operation: Initialize cache
Lock file: /home/jenkins/.gradle/caches/2.10/scripts/build_e62v1tfg3vtzz7pxxy2kzgwpr/proj/cache.properties.lock
* 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: 1 mins 28.568 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 8, 2016, 5:43 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 5:43 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/
-----------------------------------------------------------
(Updated March 8, 2016, 5:43 p.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Changes
-------
Added NEWS entry.
Bugs: AURORA-1627
https://issues.apache.org/jira/browse/AURORA-1627
Repository: aurora
Description
-------
H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
`SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
Also, moved checkstyle suppressions into annotations for more precise control.
Master:
```
Benchmark (updateCount) Mode Cnt Score Error Units
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
```
This patch:
```
Benchmark (updateCount) Mode Cnt Score Error Units
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
```
Diffs (updated)
-----
NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
Diff: https://reviews.apache.org/r/44471/diff/
Testing
-------
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Thanks,
Maxim Khutornenko
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 8, 2016, 4:49 p.m., Zameer Manji wrote:
> > I have one objection to the future of this patch which is dropping the other fields in the snapshot after we move all in memory storage to H2.
> >
> > I think keeping those fields for a while will be useful because they allow cluster operators to look at a snapshot of the cluster state and perform analytics, etc.
Thrift structs will remain in the snapshot until we fully migrate to DBTaskStore. Afterwards, I don't see any reason to continue dumping them. Analytic tools will have to be updated to work against SQL schema instead by either launching their own in-memory DB instance or working against the persistent DB store.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122541
-----------------------------------------------------------
On March 7, 2016, 11:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 11:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122541
-----------------------------------------------------------
Ship it!
I have one objection to the future of this patch which is dropping the other fields in the snapshot after we move all in memory storage to H2.
I think keeping those fields for a while will be useful because they allow cluster operators to look at a snapshot of the cluster state and perform analytics, etc.
- Zameer Manji
On March 7, 2016, 3:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 3:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122444
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Farner
On March 7, 2016, 3:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 3:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 8, 2016, 1:39 a.m., Joshua Cohen wrote:
> > Is this worthy of a NEWS entry? It seems like a pretty major change that operators should be aware of?
>
> Bill Farner wrote:
> +1
Sure, added.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122456
-----------------------------------------------------------
On March 7, 2016, 11:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 11:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Bill Farner <wf...@apache.org>.
> On March 7, 2016, 5:39 p.m., Joshua Cohen wrote:
> > Is this worthy of a NEWS entry? It seems like a pretty major change that operators should be aware of?
+1
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122456
-----------------------------------------------------------
On March 7, 2016, 3:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 3:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122456
-----------------------------------------------------------
Is this worthy of a NEWS entry? It seems like a pretty major change that operators should be aware of?
- Joshua Cohen
On March 7, 2016, 11:55 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 11:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
> Master:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
> ```
>
> This patch:
> ```
> Benchmark (updateCount) Mode Cnt Score Error Units
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
> SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
> ```
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
> src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/
-----------------------------------------------------------
(Updated March 7, 2016, 11:55 p.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Changes
-------
Bill's comments.
Bugs: AURORA-1627
https://issues.apache.org/jira/browse/AURORA-1627
Repository: aurora
Description (updated)
-------
H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
`SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
Also, moved checkstyle suppressions into annotations for more precise control.
Master:
```
Benchmark (updateCount) Mode Cnt Score Error Units
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 4.876 ± 0.403 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 0.942 ± 0.032 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 0.436 ± 0.040 ops/s
```
This patch:
```
Benchmark (updateCount) Mode Cnt Score Error Units
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 1 thrpt 5 9.846 ± 0.886 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 5 thrpt 5 1.977 ± 0.319 ops/s
SnapshotBenchmarks.RestoreSnapshotWithUpdatesBenchmark.run 10 thrpt 5 1.310 ± 0.139 ops/s
```
Diffs (updated)
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java 50044e1c38c712a52855dd96fa2a9de769d6c973
src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java ca484fab2a9b136c6d5b9be31e1ad1a5360f1b7a
src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 92849d9a82dcd6d5fe6cc452061bcdca31415f40
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
Diff: https://reviews.apache.org/r/44471/diff/
Testing
-------
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Thanks,
Maxim Khutornenko
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 7, 2016, 10:24 p.m., Bill Farner wrote:
> > Since this includes performance-sensitive changes, can you include some indication of performance gains if only to keep an eye out for regression?
>
> Maxim Khutornenko wrote:
> Good point, I need to fix SnapshotBenchmark to get it too ;)
Fixed `SnapshotBenchmark` and added results.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122389
-----------------------------------------------------------
On March 7, 2016, 10:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 10:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 7, 2016, 10:24 p.m., Bill Farner wrote:
> > Since this includes performance-sensitive changes, can you include some indication of performance gains if only to keep an eye out for regression?
Good point, I need to fix SnapshotBenchmark to get it too ;)
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122389
-----------------------------------------------------------
On March 7, 2016, 10:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 10:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122389
-----------------------------------------------------------
Since this includes performance-sensitive changes, can you include some indication of performance gains if only to keep an eye out for regression?
- Bill Farner
On March 7, 2016, 2:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 2:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44471: Implementing dbsnapshotting
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44471/#review122397
-----------------------------------------------------------
Master (a91a759) is red with this patch.
./build-support/jenkins/build.sh
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1)[0m
[1m [0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m [0m
[1m # start the health check (during health check it is still 0)[0m
[1m epsilon = 0.001[0m
[1m self._clock.tick(1.0 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)[0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m assert hct.metrics.sample()['checks'] == 0[0m
[1m [0m
[1m # finish the health check[0m
[1m self._clock.tick(0.5 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1) # interval_secs[0m
[1m> assert hct._total_latency == 0.5[0m
[1m[31mE AssertionError: assert 0.5019999999999998 == 0.5[0m
[1m[31mE + where 0.5019999999999998 = <apache.aurora.executor.common.health_checker.HealthChecker object at 0x7f7eb5f83810>._total_latency[0m
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: AssertionError
-------------- Captured stderr call --------------
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 1.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 1.5
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7eb5f83a90>] Time now: 1.502
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 1 failed, 660 passed, 5 skipped, 1 warnings in 423.17 seconds [0m
FAILURE
22:53:08 08:14 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 7, 2016, 10:22 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44471/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 10:22 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1627
> https://issues.apache.org/jira/browse/AURORA-1627
>
>
> Repository: aurora
>
>
> Description
> -------
>
> H2 DB is now `script`ed into a dedicated `dbScript` field in `Snapshot` thrift struct to speed up recovery under H2 MVStore.
>
> There was extra care needed to ensure task stores are functioning properly when they are toggled between in-memory and DB mode. Tried to document as much as possible. Let me know if something is still unclear. The existing thrift snapshot fields will be supported until all stores are fully migrated to H2 and removed subsequently along with in-memory store code.
>
> `SnapshotStoreImplTest` got rewritten into `SnapshotStoreImplIT` to fully test `TaskStore` migration matrix.
>
> Also, moved checkstyle suppressions into annotations for more precise control.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 6dc46147bb0703e83a210a81ee24081183389a89
> config/checkstyle/checkstyle.xml 2074beb66ca19eb2da3d444bc64e583409e30ead
> config/checkstyle/suppressions.xml 50a53e0d96cb365b2182b78769c1749b7e3558a9
> src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 60b66e85f83edd3c18c97e3938816cb875249d4e
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 5124d17c9bc755e33a3e97b914caed34b77cdb94
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 46b3d104931f175314902ccd39b81c4b6d67d4f3
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6d8fa11d0aed841388e1e87fc0acdaf6b8f42d06
> src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java cca92dd6e9e52f9e38394c4c688ab4897eb8e23f
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java ed63a7471d654dcefd2ff24e2e462974883541f2
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java db90150b48c5b134dde6c69f70ebca82bfdd0c12
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2f07afb73f6e585a6b43be68134e2beecac83d31
> src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 172dd206d09f131807cb33fe841ca6ebc8198a14
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 44078675286dec908da7a404ae8fce7c495a7019
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 75130a315573dbb9eff1a9b1c5408df27623cf54
>
> Diff: https://reviews.apache.org/r/44471/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>