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 2017/11/21 23:44:11 UTC
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/
-----------------------------------------------------------
(Updated Nov. 21, 2017, 3:44 p.m.)
Review request for Aurora, Jordan Ly and Stephan Erb.
Changes
-------
Addressed review feedback.
Summary (updated)
-----------------
Add RemoveJobUpdates log Op, slim JobUpdateStore API
Repository: aurora
Description (updated)
-------
JobUpdateStore historically had granular APIs in the storage layer to
minimize unnecessary use of 'expensive' database objects. The
in-memory store makes these 'free', so moving business logic out of
the storage layer is now feasible for performance and pragmatic.
This patch also introduces the `RemoveJobUpdates` log `Op`, and
`PruneJobUpdateHistory` is now ignored. In a future release (and possibly
before, with a feature flag), the scheduler will write `RemoveJobUpdates`
to the log.
LogStorage has always had the fundamental expectation that `Op`s are
idempotent. The job update event `Op`s arguably violate this requirement, but
at minimum, explicit removal of updates is necessary for idempotency.
From LogStorage.java:
This design implies that all mutations must be idempotent and free from
constraint and thus replayable over newer operations when recovering
from an old checkpoint.
Diffs (updated)
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
Diff: https://reviews.apache.org/r/63884/diff/3/
Changes: https://reviews.apache.org/r/63884/diff/2-3/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review192073
-----------------------------------------------------------
Ship it!
Master (80139da) 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 Nov. 21, 2017, 11:44 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2017, 11:44 p.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by Bill Farner <wf...@apache.org>.
> On Nov. 21, 2017, 4:27 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java
> > Line 103 (original), 107 (patched)
> > <https://reviews.apache.org/r/63884/diff/3/?file=1899269#file1899269line121>
> >
> > Can we track timings for this method? I believe we had that for the old pruner.
Done, as you are the second reviewer to request this :-)
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review191683
-----------------------------------------------------------
On Nov. 21, 2017, 3:44 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2017, 3:44 p.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review191683
-----------------------------------------------------------
This LGTM.
src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java
Line 103 (original), 107 (patched)
<https://reviews.apache.org/r/63884/#comment269537>
Can we track timings for this method? I believe we had that for the old pruner.
- David McLaughlin
On Nov. 21, 2017, 11:44 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2017, 11:44 p.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review192045
-----------------------------------------------------------
Ship it!
Ship It!
- David McLaughlin
On Nov. 21, 2017, 11:44 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2017, 11:44 p.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review191679
-----------------------------------------------------------
Ship it!
Master (c746452) 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 Nov. 21, 2017, 11:44 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2017, 11:44 p.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 63884: Add RemoveJobUpdates log Op,
slim JobUpdateStore API
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63884/#review192075
-----------------------------------------------------------
Ship it!
Ship It!
- Stephan Erb
On Nov. 22, 2017, 12:44 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63884/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2017, 12:44 a.m.)
>
>
> Review request for Aurora, Jordan Ly and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> JobUpdateStore historically had granular APIs in the storage layer to
> minimize unnecessary use of 'expensive' database objects. The
> in-memory store makes these 'free', so moving business logic out of
> the storage layer is now feasible for performance and pragmatic.
>
> This patch also introduces the `RemoveJobUpdates` log `Op`, and
> `PruneJobUpdateHistory` is now ignored. In a future release (and possibly
> before, with a feature flag), the scheduler will write `RemoveJobUpdates`
> to the log.
>
> LogStorage has always had the fundamental expectation that `Op`s are
> idempotent. The job update event `Op`s arguably violate this requirement, but
> at minimum, explicit removal of updates is necessary for idempotency.
>
> From LogStorage.java:
>
> This design implies that all mutations must be idempotent and free from
> constraint and thus replayable over newer operations when recovering
> from an old checkpoint.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift c692a5f3d715599c949c8fbef8b05c24174829e3
> src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java a5d1894a80db13632bf03f593ead46987f1c667b
> src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java c98c514bfc28afaad44bda355720347dd9ee2d3e
> src/main/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPruner.java b2768d98f7c7b0632cd31656640fcb053bcd3be1
> src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 4433b962fe73bafc0b2e06edcb9f7b117c403715
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 3cafbc2df4361458cbbcc3c1f345706069ba46d1
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 7f8c66cae388571cd3eef2668d1b42e3925dc7e8
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b3d906b96f9bfe6e4d3fcc9f3a5d02819a1ef9fe
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 3ce2c7fc06082ae0ca7a77a33bb7f333212a048c
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 57c483b049c40b56d8c83e3dcce67de00113853b
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 4d051fc4ab280418c7df70924c5e5b496c2ce052
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 826cee9c5c27b80dec55af6fb64fbe9bd72fe03e
> src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java bba1161a48738e19f10fcf72395f8d70b481ed13
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java dc8d11c5003b2637b921b6afa6e2cb5f13102858
> src/test/java/org/apache/aurora/scheduler/pruning/JobUpdateHistoryPrunerTest.java 74db5ecd8f63a2df4e24ca83a03c22e90936bbae
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 5e5c51814c12a51abada9642b550e06533f73094
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 6be4a9c4145ab38905f153a07a022707186a862c
> src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java 9fab7aeb99b155cc55c778c9b27e92daa97fe47e
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java c208210497454fabd7b3c6d41b4b5f40b1a6bb69
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java d5e5c11ad09415e861edfaa521c53fd7904d1bfd
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 39456cfe8b8c8e198a6f8c82e09769bd75db2adc
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537
>
>
> Diff: https://reviews.apache.org/r/63884/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>