You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2016/01/22 08:59:44 UTC
Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/
-----------------------------------------------------------
Review request for Aurora, John Sirois and Maxim Khutornenko.
Repository: aurora
Description
-------
Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
Diffs
-----
src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
Diff: https://reviews.apache.org/r/42646/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115809
-----------------------------------------------------------
Ship it!
Master (66a4d5f) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 8:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115943
-----------------------------------------------------------
Ship it!
Master (2da1700) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 22, 2016, 9:21 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 9:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115936
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On Jan. 22, 2016, 9:21 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 9:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115932
-----------------------------------------------------------
Ship it!
Ship It!
- John Sirois
On Jan. 22, 2016, 2:21 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 2:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/
-----------------------------------------------------------
(Updated Jan. 22, 2016, 1:21 p.m.)
Review request for Aurora, John Sirois and Maxim Khutornenko.
Changes
-------
Restored initialization logic when starting storage.
Repository: aurora
Description
-------
Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
Diff: https://reviews.apache.org/r/42646/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115892
-----------------------------------------------------------
Ship it!
Ship It!
- John Sirois
On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 1:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
>
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
>
> When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Call `mutateTask()` for every task _changed_.
>
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad idea. Properly documented, it may serve as a great ramp up to whoever attempts to backfill in the future.
>
> If you despise having NOOP classes how about converting it into an interface instead? That may be a great place to put all guidance details.
But it will literally be an empty class with an empty function. I don't see that as much of a ramp.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 12:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
>
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
>
> When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class.
> When we do need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
Call `mutateTask()` for every task _changed_.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 12:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
>
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
>
> When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Call `mutateTask()` for every task _changed_.
>
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad idea. Properly documented, it may serve as a great ramp up to whoever attempts to backfill in the future.
>
> If you despise having NOOP classes how about converting it into an interface instead? That may be a great place to put all guidance details.
>
> Bill Farner wrote:
> But it will literally be an empty class with an empty function. I don't see that as much of a ramp.
Well, I tried... Let's at least keep the initialization point as you suggested.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 8:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
>
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
>
> When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Call `mutateTask()` for every task _changed_.
>
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad idea. Properly documented, it may serve as a great ramp up to whoever attempts to backfill in the future.
>
> If you despise having NOOP classes how about converting it into an interface instead? That may be a great place to put all guidance details.
>
> Bill Farner wrote:
> But it will literally be an empty class with an empty function. I don't see that as much of a ramp.
>
> Maxim Khutornenko wrote:
> Well, I tried... Let's at least keep the initialization point as you suggested.
Not trying to be argumentative here, hope it didn't come across this way. Here's what would be left of `StorageBackfill`:
```
public final class StorageBackfill {
private StorageBackfill() {
// Utility class.
}
public static void backfill(final MutableStoreProvider storeProvider) {
}
}
```
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 1:21 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 1:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 12:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
>
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
>
> When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
>
> Call `mutateTask()` for every task _changed_.
I am still not sure why having a placeholder NOOP class is such a bad idea. Properly documented, it may serve as a great ramp up to whoever attempts to backfill in the future.
If you despise having NOOP classes how about converting it into an interface instead? That may be a great place to put all guidance details.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 8:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
>
> John Sirois wrote:
> You may be, I'm not sure. Happy to accept that intentioned stance though.
We have killed and resurrected StorageBackfill a few times already. I personally remember reviving it once. Not really sure what killing again and again really gets us. Can we finally accept it's existence and keep it around as a placeholder?
When we _do_ need it next time, what's the storage support story for it? Call mutateTask() for every task processed?
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 8:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by John Sirois <js...@apache.org>.
> On Jan. 22, 2016, 11:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
>
> Bill Farner wrote:
> That is essentially the stance i intend. My hope is that when we _do_ need it again, the resulting approach has less code ripple than we see here. I could be deluded, however.
You may be, I'm not sure. Happy to accept that intentioned stance though.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 1:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/#review115888
-----------------------------------------------------------
The mechanics of the change lgtm, but is there some other way to add schema / data updates to a new aurora release such that they get applied after recovering the log but before doing anything else? I guess its valid to say add it back when we need it, but I want to make sure this is the stance you intend; ie remove the burden from the existing codebase and push it to the next contributor that needs to break the schema.
- John Sirois
On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 1:11 a.m.)
>
>
> Review request for Aurora, John Sirois and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
>
> Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
>
> This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
> src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
> src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
> src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
> src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
>
> Diff: https://reviews.apache.org/r/42646/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/
-----------------------------------------------------------
(Updated Jan. 22, 2016, 12:11 a.m.)
Review request for Aurora, John Sirois and Maxim Khutornenko.
Changes
-------
fixup pmd/findbugs errors
Repository: aurora
Description
-------
Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
Diff: https://reviews.apache.org/r/42646/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 42646: Remove storage backfill and TaskStore
mutateTasks.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42646/
-----------------------------------------------------------
(Updated Jan. 22, 2016, 12:01 a.m.)
Review request for Aurora, John Sirois and Maxim Khutornenko.
Repository: aurora
Description
-------
Related to a comment i made in https://reviews.apache.org/r/42628/, StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As you can see from the patch, there's a lot that crumbles with it.
Since it doesn't show in the patch, StorageBackfill was only filling the `TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
This backfill has been in place since `0.6.0-incubating` and should be safe to remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 0d3874e9632952c54ef5ae9aba78638e47c87056
src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 376f48545eb860aad5afa028d3b96d04acc08377
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java de4ada431634fb171fab109f1923da810b361205
src/main/java/org/apache/aurora/scheduler/storage/Storage.java 578bb37de8853c4228e76b31f601430b7170946a
src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 609a6242e8918bf4a044e6e5e2c8dfe4b900192e
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 4e4f8d2c0f237c4480abe101835176f7d69958db
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 43fda1d001f52f916b8a6d75fcdb74d4280eaa41
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 728353116c8892c015a6443d8db09ba241b32230
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 8fd024a460cd948dab703b99788b1ed2d6c43bc3
src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 9fb8aad5d1c0412efc6d1176e543321ebe503e03
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 0768ec37bbc6c3c101aa04a953a36a4af7b25963
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 1ac41d18fd9d603c4c342f9635e116bfd055f858
src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 2570464c85630a55d3e6c8653fc0307d54504e15
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7382eca281eeab17d407ed140f16d6a633d8ad72
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 3c4e2bd5eefc141a5d2c5d0e56881899816652f8
Diff: https://reviews.apache.org/r/42646/diff/
Testing
-------
Thanks,
Bill Farner