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 2015/02/19 01:19:13 UTC
Review Request 31170: Refactor existing write APIs for job updates to
use IJobUpdateKey.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/
-----------------------------------------------------------
Review request for Aurora, Maxim Khutornenko and Zameer Manji.
Bugs: AURORA-1093
https://issues.apache.org/jira/browse/AURORA-1093
Repository: aurora
Description
-------
In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
- SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
- JobUpdateStore.fetchUpdateKey was added to facilitate the above
If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
Diffs
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
Diff: https://reviews.apache.org/r/31170/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
> On Feb. 19, 2015, 12:52 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift, line 83
> > <https://reviews.apache.org/r/31170/diff/1/?file=868467#file868467line83>
> >
> > Do we really need a deprecation cycle here given the beta status of the updater? The interface instability is something expected from an API in beta status. Keeping things around will just hurt our iteration speed unnecessarily.
> >
> > E.g. guava @Beta annotation says:
> >
> > "Signifies that a public API (public class, method or field) is subject to incompatible changes, or even removal, in a future release. An API bearing this annotation is exempt from any compatibility guarantees made by its containing library."
> >
> > http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/annotations/Beta.html
I'm not sure guava is a great parallel here, since the API won't change unless the developer explicitly bumps the version.
In this case, the alternative is to special-case and drop log entries that don't have the field we require, so we'll still have a relic if we want to at least not crash when reading data from previous versions. I figure at that point we still effectively have a deprecation cycle for allowing IJobUpdateKey to be absent.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73043
-----------------------------------------------------------
On Feb. 19, 2015, 12:19 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:19 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73043
-----------------------------------------------------------
api/src/main/thrift/org/apache/aurora/gen/storage.thrift
<https://reviews.apache.org/r/31170/#comment119207>
Do we really need a deprecation cycle here given the beta status of the updater? The interface instability is something expected from an API in beta status. Keeping things around will just hurt our iteration speed unnecessarily.
E.g. guava @Beta annotation says:
"Signifies that a public API (public class, method or field) is subject to incompatible changes, or even removal, in a future release. An API bearing this annotation is exempt from any compatibility guarantees made by its containing library."
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/annotations/Beta.html
- Maxim Khutornenko
On Feb. 19, 2015, 12:19 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:19 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73042
-----------------------------------------------------------
Ship it!
Master (e0e3f2e) 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 Feb. 19, 2015, 12:19 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:19 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
> On Feb. 19, 2015, 12:39 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 122
> > <https://reviews.apache.org/r/31170/diff/1/?file=868471#file868471line122>
> >
> > s/t/it
Fixed.
> On Feb. 19, 2015, 12:39 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1316
> > <https://reviews.apache.org/r/31170/diff/1/?file=868477#file868477line1316>
> >
> > I think you can remove the TODO here.
Done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73038
-----------------------------------------------------------
On Feb. 19, 2015, 12:19 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:19 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73038
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
<https://reviews.apache.org/r/31170/#comment119201>
s/t/it
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/31170/#comment119203>
I think you can remove the TODO here.
- Zameer Manji
On Feb. 18, 2015, 4:19 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 4:19 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Zameer Manji <zm...@apache.org>.
> On Feb. 19, 2015, 1:11 p.m., Zameer Manji wrote:
> > Ship it, modulo my concern over uncessarily creating an IJobUpdateKey from PruneVictim.
Bill clarified the creating IJobUpdateKey from PruneVictim offline. PruneVictim needs to have the mutable types so it can be populated from storage.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73199
-----------------------------------------------------------
On Feb. 19, 2015, 12:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73199
-----------------------------------------------------------
Ship it!
Ship it, modulo my concern over uncessarily creating an IJobUpdateKey from PruneVictim.
- Zameer Manji
On Feb. 19, 2015, 12:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/
-----------------------------------------------------------
(Updated Feb. 21, 2015, 1:43 a.m.)
Review request for Aurora, Maxim Khutornenko and Zameer Manji.
Bugs: AURORA-1093
https://issues.apache.org/jira/browse/AURORA-1093
Repository: aurora
Description
-------
In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
- SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
- JobUpdateStore.fetchUpdateKey was added to facilitate the above
If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
Diffs (updated)
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
Diff: https://reviews.apache.org/r/31170/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73195
-----------------------------------------------------------
Ship it!
Master (e0e3f2e) 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 Feb. 19, 2015, 8:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 8:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
> On Feb. 20, 2015, 11:36 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 43
> > <https://reviews.apache.org/r/31170/diff/2/?file=869688#file869688line43>
> >
> > JobKeys.assertValid(summary.getJobKey()) should be better here.
> >
> > Also, you may want to inline these checks with the JobUpdateKey construction statement.
Done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73342
-----------------------------------------------------------
On Feb. 19, 2015, 8:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 8:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73342
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/updater/Updates.java
<https://reviews.apache.org/r/31170/#comment119639>
JobKeys.assertValid(summary.getJobKey()) should be better here.
Also, you may want to inline these checks with the JobUpdateKey construction statement.
- Maxim Khutornenko
On Feb. 19, 2015, 8:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 8:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/
-----------------------------------------------------------
(Updated Feb. 19, 2015, 8:30 p.m.)
Review request for Aurora, Maxim Khutornenko and Zameer Manji.
Bugs: AURORA-1093
https://issues.apache.org/jira/browse/AURORA-1093
Repository: aurora
Description
-------
In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
- SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
- JobUpdateStore.fetchUpdateKey was added to facilitate the above
If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
Diffs (updated)
-----
api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
Diff: https://reviews.apache.org/r/31170/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Zameer Manji <zm...@apache.org>.
> On Feb. 18, 2015, 5:17 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, line 166
> > <https://reviews.apache.org/r/31170/diff/1/?file=868472#file868472line166>
> >
> > Shouldn't PruneVictim have the IJobUpdateKey as the parameter?
>
> Bill Farner wrote:
> I'm not sure what you mean. Here we are extracting the `IJobUpdateKey` from the `PruneVictim`. We use the row ID to delete, and surface the externally-usable unique identifier that was deleted.
Shouldn't the PruneVictim have its own IJobUpdateKey for use? I don't think building one here should be the right thing to do.
> On Feb. 18, 2015, 5:17 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java, line 324
> > <https://reviews.apache.org/r/31170/diff/1/?file=868474#file868474line324>
> >
> > I don't see how removing `throws RuntimeException` is related to this change. If it is not necessary please remove it from this review to reduce churn.
>
> Bill Farner wrote:
> I disagree, we should be willing to have low-impact cleanup while changing neighboring code. I don't feel that this cleanup complicates reviewing.
I feel that changes that have a large ripple like this one should not be combined with low-impact cleanup as it obscures the nature of the ripple. However, I will not block the review over this change.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73050
-----------------------------------------------------------
On Feb. 19, 2015, 12:30 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:30 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Bill Farner <wf...@apache.org>.
> On Feb. 19, 2015, 1:17 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, line 166
> > <https://reviews.apache.org/r/31170/diff/1/?file=868472#file868472line166>
> >
> > Shouldn't PruneVictim have the IJobUpdateKey as the parameter?
I'm not sure what you mean. Here we are extracting the `IJobUpdateKey` from the `PruneVictim`. We use the row ID to delete, and surface the externally-usable unique identifier that was deleted.
> On Feb. 19, 2015, 1:17 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java, line 324
> > <https://reviews.apache.org/r/31170/diff/1/?file=868474#file868474line324>
> >
> > I don't see how removing `throws RuntimeException` is related to this change. If it is not necessary please remove it from this review to reduce churn.
I disagree, we should be willing to have low-impact cleanup while changing neighboring code. I don't feel that this cleanup complicates reviewing.
> On Feb. 19, 2015, 1:17 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 35
> > <https://reviews.apache.org/r/31170/diff/1/?file=868480#file868480line35>
> >
> > Use http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect/Sets.html#immutableEnumSet(java.lang.Iterable) instead.
Good catch, done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73050
-----------------------------------------------------------
On Feb. 19, 2015, 12:19 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2015, 12:19 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 31170: Refactor existing write APIs for job
updates to use IJobUpdateKey.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31170/#review73050
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java
<https://reviews.apache.org/r/31170/#comment119219>
Shouldn't PruneVictim have the IJobUpdateKey as the parameter?
src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
<https://reviews.apache.org/r/31170/#comment119222>
I don't see how removing `throws RuntimeException` is related to this change. If it is not necessary please remove it from this review to reduce churn.
src/main/java/org/apache/aurora/scheduler/updater/Updates.java
<https://reviews.apache.org/r/31170/#comment119217>
Use http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect/Sets.html#immutableEnumSet(java.lang.Iterable) instead.
- Zameer Manji
On Feb. 18, 2015, 4:19 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31170/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 4:19 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
>
>
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions:
>
> - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes
> - JobUpdateStore.fetchUpdateKey was added to facilitate the above
>
> If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3
> src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17
> src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb
> src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490
> src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb
> src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd
> src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048
> src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1
>
> Diff: https://reviews.apache.org/r/31170/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>