Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review189160
---



Master (38476ab) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 6:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 6:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 76bf86863a8fafbcd605ae87bb3436f3b4a6abb4 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 44ba3895c83bc66adf68cb68c34cde2f4ae27b75 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 47fd92f12f5db38b6d3927ab7aadd6086d49b9da 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 24, 2017, 9:53 a.m., Jordan Ly wrote:
> > Overall LGTM! Tested upgrade/downgrade paths at scale and it seems to work 
> > well.
> 
> Stephan Erb wrote:
> Pure curiosity on my end: the benchmarks show pretty impressive speed 
> improvements. Are those also noticeable in practice for you?

I don't believe Jordan's test environment stresses codepaths related to job 
updates.  Effects of the in-memory `AttributeStore` may have been observable, 
though, as the scheduling path is stressed.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review189062
---


On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:11 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 76bf86863a8fafbcd605ae87bb3436f3b4a6abb4 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 44ba3895c83bc66adf68cb68c34cde2f4ae27b75 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 47fd92f12f5db38b6d3927ab7aadd6086d49b9da 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Stephan Erb]

> On Okt. 24, 2017, 6:53 nachm., Jordan Ly wrote:
> > Overall LGTM! Tested upgrade/downgrade paths at scale and it seems to work 
> > well.

Pure curiosity on my end: the benchmarks show pretty impressive speed 
improvements. Are those also noticeable in practice for you?


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review189062
---


On Okt. 18, 2017, 8:11 nachm., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Okt. 18, 2017, 8:11 nachm.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Jordan Ly]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review189062
---


Ship it!




Overall LGTM! Tested upgrade/downgrade paths at scale and it seems to work well.

- Jordan Ly


On Oct. 18, 2017, 6:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 6:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 6fdf31d6ed430f3dc0a0117442fc8b532aa40230 
>   
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Jordan Ly]

> On Oct. 21, 2017, 4 p.m., Bill Farner wrote:
> > FYI i'm in no rush, but awaiting an explicit ship it from Jordan.  Want to 
> > make sure everyone is on board with this!

Apologies for the delay everyone! Code-wise the patch looks good, but I would 
like to test it at scale for extra confidence.

I estimate that this will be all be done by Tuesday at the latest.


- Jordan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188897
---


On Oct. 18, 2017, 6:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 6:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188898
---


Ship it!




Ship It!

- David McLaughlin


On Oct. 18, 2017, 6:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 6:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 6fdf31d6ed430f3dc0a0117442fc8b532aa40230 
>   
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188897
---



FYI i'm in no rush, but awaiting an explicit ship it from Jordan.  Want to make 
sure everyone is on board with this!

- Bill Farner


On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:11 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188808
---



Master (a935389) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 11:41 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:41 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1ec6d740013575bb016645ea0863a7cd4c272838 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188767
---



This patch does not apply cleanly against master (b9ede2f), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 11:11 a.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 254c9b7a3fa0b8c3fb47a351748baf9b77f2f2e4 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> df37fb764b3efb7ef97c1a63bdef638c06aea61c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188627
---



Master (c638877) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 2:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 2:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 079f495d2f41272456daec5caca0944aa2d7fafc 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Stephan Erb]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188544
---


Ship it!




Thanks. You have answered all my questions and the latest additional patch 
looks good to me as well!

Ship it! (once review bot agrees as well :)

- Stephan Erb


On Oct. 18, 2017, 8:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188542
---



This patch does not apply cleanly against master (b7e83a0), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 8:11 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 18, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/
---

(Updated Oct. 18, 2017, 11:11 a.m.)


Review request for Aurora, Jordan Ly and Stephan Erb.


Changes
---

Addressed first batch of review comments.


Repository: aurora


Description
---

This patch introduces map-based volatile stores, most of which were revived 
from git history with minimal changes.  The DB storage system is now only used 
in a temporary storage when replaying a snapshot containing the `dbScript` 
field.

Please take special care to double-check my work in `SnapshotStoreImpl`, as it 
must function for backwards compatibility with `Snapshot`s in the wild.  
Another area of interest is the implementation of `MemJobUpdateStore`, which 
must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most of 
this stems from behind-the-scenes interaction with `LockStore` and use of 
cascading deletes.

Note that this change removes the transactional nature of in-memory storage 
operations as well as the `READ COMMITTED` transaction isolation previously 
available to some stores (proven in necessary changes to 
`StorageTransactionTest`).  This means some stores will permit dirty reads when 
they previously did not.  `TaskStore` has always had this non-transactional 
behavior by default, as the DB task store was never deemed suitable for 
production.  Nonetheless, this non-transactional behavior should be considered 
safe as the scheduler fails over on a storage operation failure, and relies on 
the persistent log storage for transaction atomicity.

See the [mailing list 
discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
 for context.


Diffs (updated)
-

  RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
  api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
9ee9eff6c3d657decd93458dc3e6792a30614a60 
  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
04e3d311c6845689cd3bd813aa8dcf9df55f1199 
  examples/vagrant/upstart/aurora-scheduler.conf 
63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
7d3766829161457b1b3ba50bce128047d10b2c58 
  src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
ae59f3def75d0e1d3866c8d2f85063456643e9a6 
  src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
45c2ab96d8fa65ab12ba710954128da987f0544b 
  src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
  src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
7b4050637433ab656fc09958bac1a050fec02bed 
  src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
cac02a55d26b2934099a2b03457c703600296292 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
  src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
64733e5b38f238e64424127a53591e6546a3e9a8 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
93cd20adc28ed700719e472bb2331137a93d1d9d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
52c3c6618a3cf1009435ca8a9cece36365913e55 
  src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
6c676693e531541ef9aec7f7a130c119ebf35df3 
  src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 
bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
7904e3880d214aac1013ce18265fed924ef7897e 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
942d180fe1a8b7a583e812f9106ee0e8db1bea55 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
6fdf31d6ed430f3dc0a0117442fc8b532aa40230 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
fdf9c33f80be3eff03f50732b8c4735cb9ff3b91 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
d145259653dd4df90e3877fc3e744e24c7a15d13 
  src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
24296b6c61e62a09c30f01dfb84448f73a5f1e44 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java 
PRE-CREATION 
  

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
> > Lines 79-103 (original), 72-96 (patched)
> > 
> >
> > Should these be removed? The release notes indicate these have been 
> > removed? Are we keeping this for the deprecation cycle?

This was an attempt to minimize the diff and make easier to prove that i did 
not change the Db implementations.  If you look closely, you'll see that this 
`Options` class doesn't make its way to the CLI code, so it's rendered useless. 
 However, to make this less confusing i will remove the `@Parameter` labels 
from all of the fields.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
> > Lines 34 (patched)
> > 
> >
> > public?
> > 
> > s/Mutable/AttributeStore.Mutable/

> public?

Store implementations are deliberately non-public where practical, as they 
should only be used through the store interface.

> s/Mutable/AttributeStore.Mutable/

Done.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
> > Lines 43-52 (patched)
> > 
> >
> > I guess you mean `replace` on `ConcurrentMap`?
> > 
> > +1 (Sounds more robust)

Unfortunately not, we'd be looking for something much closer to `merge()`, but 
per the preivous comment, `merge()` falls short.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 156-159 (patched)
> > 
> >
> > Should we insert the refreshed entry into the map as soon as possible? 
> > Inserting all at once towards the end seems like it can cause races.

See previous comments about write concurrency and let me know if you still have 
concerns here.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 236 (patched)
> > 
> >
> > `LockStore` can change after `tokenFromLockStore` returns. Should we 
> > protect against this race?

See previous comments about write concurrency and let me know if you still have 
concerns here.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 298 (patched)
> > 
> >
> > Same here- wonder if we can move to using `Stream`s?

Done by popular demand :-)


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 307 (patched)
> > 
> >
> > Is this correct? Should it be - 
> > 
> > `System.currentTimeMillis() - s.getState().getCreatedTimestampMs() > 
> > historyPruneThresholdMs` 
> > 
> > ?

The caller code in `JobUpdateHistoryPruner` should alleviate this concern:

```
  Set prunedUpdates = 
storeProvider.getJobUpdateStore().pruneHistory(
  settings.maxUpdatesPerJob,
  clock.nowMillis() - 
settings.maxHistorySize.as(Time.MILLISECONDS));
```

i.e. `pruneHistory()` accepts the point-in-time _threshold_ rather than the 
interval.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 345-360 (patched)
> > 
> >
> > +1

Stream conversion done.

Aside - reviewboard makes a +1 difficult to stitch together when in a separate 
review, as it only threads in a _comment_ on a review, rather than a comment on 
the same line in a diff.


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java
> > Lines 33 (patched)
> > 
> >
> > public

-1, see previous comment for rationale


> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
> > Lines 27-28 (patched)
> > 
> >
> > public

-1, see previous comment for rationale


> On Oct. 17, 2017, 11:21 a.m., 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote:
> > Overall LGTM as well! I believe the logic for upgrading and maintaining 
> > forwards/backwards compatibility is sound. Ensuring my understanding, the 
> > upgrade path is:
> > 
> > 1. A new version with the memory stores is released.
> > 2. People upgrading to the new version would load their previous DB 
> > snapshot, it gets converted into their Thrift store equivalents, and then 
> > applied.
> > 3. The next snapshot that occurs with the new version will use only the 
> > Thrift stores in the Snapshot object.
> > 
> > This new Snapshot object is forwards and backwards compatibile since it 
> > only uses the Thrift stores which both the previous and upgrade versions 
> > can apply.

Correct.


> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
> > Lines 43-52 (patched)
> > 
> >
> > `sychronized` would definitely solve this issue, but I would recommend 
> > keeping the method not `synchronized` and using the `merge` function in 
> > `ConcurrentMap` as opposed to a `put` with a function within the arguments. 
> > This should maintain thread safety while increasing throughput.

This would be a welcome change, but complicates the necessary no-op detection 
currently provided (short of making the `BiFunction` stateful, which would be 
unwelcome).

It's worth noting that concurrency == 1 here, due to the global write lock in 
`LogStorage`.  As a result, i suggest we not be concerned with concurrent 
throughput gains.


> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java
> > Lines 39-50 (patched)
> > 
> >
> > This method probably has to be `sychronized`, since you need the state 
> > of the whole map before you do anything.

Dropped, see previous replies about store concurrency.


> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorage.java
> > Lines 40-46 (patched)
> > 
> >
> > Mostly for my knowledge, but what is the use of this?

Nothing :-)

It was previously used during the transition from Mem->Db stores, and i 
neglected to remove it when resurrecting from git history.  Removed.


> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorageModule.java
> > Lines 72-73 (patched)
> > 
> >
> > Should this be configurable?

I went back and forth on this.  I concluded that we are risking making the 
scheduler _too_ configurable, and with knobs that don't really help.  I 
factored this into the "unhelpful knob" territory, and opted to bake it in.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188148
---


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 12, 2017, 2:53 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 190-196 (patched)
> > 
> >
> > You can combine these, right?
> > 
> > Also, use Streams?

Yes, combined.  I had it broken up to separate the "possibly absent" piece from 
the transform, but nbd combining.

Re: streams, i went ahead and switched this file, but i'd like to minimize 
churn and hold off from a sweeping change in this patch.


> On Oct. 12, 2017, 2:53 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 274 (patched)
> > 
> >
> > May be a premature optimization, but given the frequency that this can 
> > be called - did you consider explicit locking on the UpdateKey?

This code is at the mercy of the global write lock (the same is true for 
`DbJobUpdateStore`), so finer-grained locking here would be misspent effort.


> On Oct. 12, 2017, 2:53 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 365 (patched)
> > 
> >
> > Why use FluentIterable instead of Stream? If we return a Stream here, 
> > the transformations applied (e.g. to summaries) can be done in the same 
> > single pass through the updates.

Capturing offline discussion - there's no clear performance benefit to 
refactoring this code.  Conversion to Stream is coming in the next revision of 
this patch.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187872
---


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
> > Lines 43-52 (patched)
> > 
> >
> > With multiple accesses of `hostAttributes` this class is subject to 
> > race conditions. 
> > 
> > Should we add `synchronized` to all public methods?

Historically, all stores have assumed the presence of a centralized storage 
write lock.  This is true (`LogStorage#writeLock`), but distant and apparently 
no longer documented.  Since this type of concurrency concern is sure to exist 
in all stores, i propose restoring the documentation in `Storage` for now.


> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 146-161 (patched)
> > 
> >
> > As indicated in the other comment, this will miss the case when 
> > something is in the LockStore but not reflected in the corresponding update.
> > 
> > I think we should handle that case for additonal robustness.

I agree with you; the only thing holding me back is practical longevity of 
`LockStore`.  I'd like to remove it ASAP (its only purpose is to indirectly 
answer "Is this job updating?"); but tight coupling to `DbJobUpdateStore` 
severly complicated that.  Since i'm pulling that whole thread now, i'd like to 
avoid any kind of cross-store synchronization unless correctness is otherwise 
at risk.

With that said, `refreshLocks()` is used locally in what appear to be read 
paths, but ultimately part of write paths.  How would you feel about moving 
`fetchAllJobUpdateDetails()` and `getLockToken()` to the 
`JobUpdateStore.Mutable` interface, making them subject to the now-documented 
write lock guarantee?


> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 235-240 (patched)
> > 
> >
> > What happens for the case when `lockToken` is not present but have have 
> > a token in the LockStore? 
> > 
> > It seems like `refreshLocks` does not handle that case either.

This is precisely where the coupling between `LockStore` and `JobUpdateStore` 
gets ugly - they both assume calling code is coordinating use of the stores in 
a specific way.  The implementation attempts to keep the stores in sync to 
prevent this case, but it's technically possible.  The same is true of the DB 
implementations.

However, this is tempting me to go back and take another stab at removing 
`LockStore` since carrying over the store coupling feels error-prone.


> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
> > Lines 345-360 (patched)
> > 
> >
> > This relies on the assumption that all `UpdateEvent`s and 
> > `InstanceUpdateEvent`s are sorted chronologically. 
> > 
> > I think we should enforce this invariant in `saveJobUpdateEvent` and 
> > `saveJobInstanceUpdateEvent` to prevent surprises.

Good call, done.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187861
---


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Santhosh Kumar Shanmugham]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188327
---



Mostly looks good to me. Some questions about concurrency when the stores go 
into action. We need a multi-threaded stress/load test.


src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
Lines 79-103 (original), 72-96 (patched)


Should these be removed? The release notes indicate these have been 
removed? Are we keeping this for the deprecation cycle?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 34 (patched)


public?

s/Mutable/AttributeStore.Mutable/



src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 43-52 (patched)


I guess you mean `replace` on `ConcurrentMap`?

+1 (Sounds more robust)



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 156-159 (patched)


Should we insert the refreshed entry into the map as soon as possible? 
Inserting all at once towards the end seems like it can cause races.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 236 (patched)


`LockStore` can change after `tokenFromLockStore` returns. Should we 
protect against this race?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 298 (patched)


Same here- wonder if we can move to using `Stream`s?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 307 (patched)


Is this correct? Should it be - 

`System.currentTimeMillis() - s.getState().getCreatedTimestampMs() > 
historyPruneThresholdMs` 

?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 345-360 (patched)


+1



src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java
Lines 33 (patched)


public



src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
Lines 27-28 (patched)


public



src/main/java/org/apache/aurora/scheduler/storage/mem/MemSchedulerStore.java
Lines 26 (patched)


public

(Not sure which one is the convention that you are going for. Make all 
`Store`s public or package-private.)



src/test/java/org/apache/aurora/scheduler/storage/db/AttributeStoreTest.java
Lines 19 (patched)


Is this going to be all the test or do you intend to add more later? (Here 
and for all the `MemStore`s)


- Santhosh Kumar Shanmugham


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Jordan Ly]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review188148
---



Overall LGTM as well! I believe the logic for upgrading and maintaining 
forwards/backwards compatibility is sound. Ensuring my understanding, the 
upgrade path is:

1. A new version with the memory stores is released.
2. People upgrading to the new version would load their previous DB snapshot, 
it gets converted into their Thrift store equivalents, and then applied.
3. The next snapshot that occurs with the new version will use only the Thrift 
stores in the Snapshot object.

This new Snapshot object is forwards and backwards compatibile since it only 
uses the Thrift stores which both the previous and upgrade versions can apply.


src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 43-52 (patched)


`sychronized` would definitely solve this issue, but I would recommend 
keeping the method not `synchronized` and using the `merge` function in 
`ConcurrentMap` as opposed to a `put` with a function within the arguments. 
This should maintain thread safety while increasing throughput.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java
Lines 39-50 (patched)


This method probably has to be `sychronized`, since you need the state of 
the whole map before you do anything.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorage.java
Lines 40-46 (patched)


Mostly for my knowledge, but what is the use of this?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorageModule.java
Lines 72-73 (patched)


Should this be configurable?


- Jordan Ly


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187872
---



This LGTM! Super excited about this. 

I focused my rview effort on the snapshot logic and new store (update store) in 
particular. Only minor questions below.


src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 190-196 (patched)


You can combine these, right?

Also, use Streams?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 274 (patched)


May be a premature optimization, but given the frequency that this can be 
called - did you consider explicit locking on the UpdateKey?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 365 (patched)


Why use FluentIterable instead of Stream? If we return a Stream here, the 
transformations applied (e.g. to summaries) can be done in the same single pass 
through the updates.


- David McLaughlin


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Stephan Erb]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187861
---



I am finished with the second part of my review. Looks very solid to me.


src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 43-52 (patched)


With multiple accesses of `hostAttributes` this class is subject to race 
conditions. 

Should we add `synchronized` to all public methods?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 146-161 (patched)


As indicated in the other comment, this will miss the case when something 
is in the LockStore but not reflected in the corresponding update.

I think we should handle that case for additonal robustness.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 235-240 (patched)


What happens for the case when `lockToken` is not present but have have a 
token in the LockStore? 

It seems like `refreshLocks` does not handle that case either.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 345-360 (patched)


This relies on the assumption that all `UpdateEvent`s and 
`InstanceUpdateEvent`s are sorted chronologically. 

I think we should enforce this invariant in `saveJobUpdateEvent` and 
`saveJobInstanceUpdateEvent` to prevent surprises.


- Stephan Erb


On Oct. 10, 2017, 9:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 9:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Stephan Erb]

> On Oct. 10, 2017, 11:23 p.m., Stephan Erb wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift
> > Line 149 (original)
> > 
> >
> > Can we remove that already? Could it be the case that we have some 
> > small scale users that run using the DB task store and still need that 
> > migration?
> 
> Bill Farner wrote:
> _Disclaimer - please push back unless you're fully convinced_
> 
> This is not necessary; the field is only used to avoid redundant work.
> 
> On master:
> - `Snapshot.tasks` and `Snapshot.cronJobs` will always be *written*
> - `Snapshot.tasks` and `Snapshot.cronJobs` will be *read* if any of:
>   a. CLI option `useDbSnapshotForTaskStore = false`
>   b. `Snapshot.dbScript` is empty
>   c. `Snapshot.experimentalTaskStore = false`
> 
> This patch will always populate `Snapshot.tasks` and `Snapshot.cronJobs`, 
> and will never populate `Snapshot.dbScript`.  As a result, the prior version 
> will never skip over reading `Snapshot.tasks` or `Snapshot.cronJobs`.

This sounds correct. I needed to type out the scenario below to fully convince 
me though :)

Using `-snapshot_hydrate_stores` an operator might have disabled the hydration 
of the dedicated thrift fields for performance reasons 
(https://reviews.apache.org/r/54774/):

* Operators adjust their deployment scripts to deploy this patch or the next 
Aurora release (update package, drop usage of `-snapshot_hydrate_stores` as it 
has been removed, ...)
* Operators deploys the version as in this patch. 
* Aurora comes up with the new version and reads everything from 
`Snapshot.dbScript` while ignoring the empty `Snapshot` fields.
* Aurora performs a snapshot that no longer contains `Snapshot.dbScript`.
* Operators see an issue. They decides to rollback to the last good version in 
their version control system.
* Aurora comes up with the old version. `Snapshot.dbScript` is now empty but in 
return the other `Snapshot` fields are filled and will be read

So all in all this looks good to me.


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187581
---


On Oct. 10, 2017, 9:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 9:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187675
---



Master (519e3df) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 04e3d311c6845689cd3bd813aa8dcf9df55f1199 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 7d3766829161457b1b3ba50bce128047d10b2c58 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> c293a9fd7c435e17dfc09f9ccc180154be8c7b2d 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 45c2ab96d8fa65ab12ba710954128da987f0544b 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> f5e44dfd6d2c81e90859eaf24cf904a9662c1c72 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 7b4050637433ab656fc09958bac1a050fec02bed 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> dd0e480c628f6bb44de0ce758c8af2bb54db1bc6 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> 64733e5b38f238e64424127a53591e6546a3e9a8 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 93cd20adc28ed700719e472bb2331137a93d1d9d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> fb3dadb4b4c7036112e423b2bc1b2540d793b4d6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187612
---



This patch does not apply cleanly against master (519e3df), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> bb7055ed6826d57cb6b5c5c77e4a27c3bcdd10c7 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 0a2516e4843b8f920700ece70b3cc816d2acecf0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 835f1604c0c5d913a87d570ee01d053bbbf92ecb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 9d2164f4f2d18e4595d3039d64cedacb7df00ae2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> d145259653dd4df90e3877fc3e744e24c7a15d13 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Bill Farner]

> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > RELEASE-NOTES.md
> > Line 4 (original), 4 (patched)
> > 
> >
> > This is a major change. We should proabably call it out here.

Done.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift
> > Line 149 (original)
> > 
> >
> > Can we remove that already? Could it be the case that we have some 
> > small scale users that run using the DB task store and still need that 
> > migration?

_Disclaimer - please push back unless you're fully convinced_

This is not necessary; the field is only used to avoid redundant work.

On master:
- `Snapshot.tasks` and `Snapshot.cronJobs` will always be *written*
- `Snapshot.tasks` and `Snapshot.cronJobs` will be *read* if any of:
  a. CLI option `useDbSnapshotForTaskStore = false`
  b. `Snapshot.dbScript` is empty
  c. `Snapshot.experimentalTaskStore = false`

This patch will always populate `Snapshot.tasks` and `Snapshot.cronJobs`, and 
will never populate `Snapshot.dbScript`.  As a result, the prior version will 
never skip over reading `Snapshot.tasks` or `Snapshot.cronJobs`.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
> > Lines 47 (patched)
> > 
> >
> > Nitpick: Inconsistent style.

Fixed.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
> > Lines 93-100 (original), 89-94 (patched)
> > 
> >
> > These comments are now outdated and should be removed.

Fixed, thanks!


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
> > Lines 145-162 (original), 130-131 (patched)
> > 
> >
> > How about we leave this in but allow it to be disabled with a flag? 
> > 
> > That way it would be easier to deploy this patch: If something goes 
> > wrong one can simply perform a rollback as the snapshot is completely 
> > compatible. If however we stop writing the db dump as done here, we would 
> > have to restore from a backup.
> > 
> > We could remove this once we drop the entire H2/mybatis stuff.

> If however we stop writing the db dump as done here, we would have to restore 
> from a backup

Please see my previous comment about compatibility, but i believe this is 
incorrect.  I've convinced myself that as long as `dbScript` is empty, all is 
well.  This is what motivated doing all stores at once - to avoid piecemeal 
removal of tables from `dbScript`.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
> > Lines 246-249 (original), 217 (patched)
> > 
> >
> > To check my understanding: There are three cases we have to handle:
> > 
> > a) db script but no host attributes in the snapshot -> we restore 
> > everything from the db script 
> > b) db script and host attributes in the snapshot -> the data is 
> > duplicated but consistent. Restoring it twice will do no harm
> > c) no db script any longer just host attributes in the snapshot: 
> > simple restore as seen here. Will be the code path used in the future.
> >   
> > Correct?

Correct, this is my understanding.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
> > Lines 41 (patched)
> > 
> >
> > Argument checking for stores is somewhat inconsistent. We should decide 
> > if we want them everywhere. If not, we should drop them.

Opting to remove based on popular majority within the code.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187581
---


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Stephan Erb]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187581
---



I have a first set of comments, but will have to complete my review in a second 
sitting.


RELEASE-NOTES.md
Line 4 (original), 4 (patched)


This is a major change. We should proabably call it out here.



api/src/main/thrift/org/apache/aurora/gen/storage.thrift
Line 149 (original)


Can we remove that already? Could it be the case that we have some small 
scale users that run using the DB task store and still need that migration?



src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
Lines 47 (patched)


Nitpick: Inconsistent style.



src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
Lines 93-100 (original), 89-94 (patched)


These comments are now outdated and should be removed.



src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
Lines 145-162 (original), 130-131 (patched)


How about we leave this in but allow it to be disabled with a flag? 

That way it would be easier to deploy this patch: If something goes wrong 
one can simply perform a rollback as the snapshot is completely compatible. If 
however we stop writing the db dump as done here, we would have to restore from 
a backup.

We could remove this once we drop the entire H2/mybatis stuff.



src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
Lines 246-249 (original), 217 (patched)


To check my understanding: There are three cases we have to handle:

a) db script but no host attributes in the snapshot -> we restore 
everything from the db script 
b) db script and host attributes in the snapshot -> the data is 
duplicated but consistent. Restoring it twice will do no harm
c) no db script any longer just host attributes in the snapshot: simple 
restore as seen here. Will be the code path used in the future.
  
Correct?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
Lines 41 (patched)


Argument checking for stores is somewhat inconsistent. We should decide if 
we want them everywhere. If not, we should drop them.


- Stephan Erb


On Oct. 10, 2017, 9:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 9:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187591
---



Master (4a1fba3) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> bb7055ed6826d57cb6b5c5c77e4a27c3bcdd10c7 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 0a2516e4843b8f920700ece70b3cc816d2acecf0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 835f1604c0c5d913a87d570ee01d053bbbf92ecb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 9d2164f4f2d18e4595d3039d64cedacb7df00ae2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Aurora ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187574
---



Master (4a1fba3) is red with this patch.
  ./build-support/jenkins/build.sh

 @ ./~/moment/src/lib/locale/locales.js 65:16-60
 @ ./~/moment/src/lib/locale/locale.js
 @ ./~/moment/src/moment.js
 @ ./src/main/js/utils/Task.js
 @ ./src/main/js/pages/Instance.js
 @ ./src/main/js/index.js
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileTestJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
: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 NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java:71:
 'org.apache.aurora.scheduler.storage.Util.jobUpdateActionStatName' should be 
separated from previous imports. [ImportOrder]
 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.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 2m 57s
37 actionable tasks: 31 executed, 6 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>