Re: Review Request 30225: Modifying update controller to support heartbeats.
On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 282-292 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line282 There's a race on access to `pulseStates`. Terminating an update between :282 and :287 would cause a stale entry in the map (`storage.read` doesn't offer a lock, but we shouldn't count on that anyway). Consider `synchronized (pulseStates)`. IMHO, though, this points out that this class is difficult to reason about now that multiple disparate locks are in play. Did you consider extracting an inner class to use? This would be different from the fully-isolated class we discussed before, where you can share implementation details and accept high coupling, but at least isolate management of `pulseStates` and manage it with higher-level operations from the rest of the class. Seems like you've already done this to a degree by keeping methods managing this map near each other. Your call on this. The read lock is no more but old habits die hard. You're right, it's no longer required after removing all DB access from this method. Dropped the storage code and consolidated all map access in a private class. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 277 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line277 `storeProvider` is not used, do you need the `storage.read` at all? Dropped. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, line 166 https://reviews.apache.org/r/30225/diff/6/?file=853464#file853464line166 s/query/fetch/ That's already taken. Renamed to job_update_store_fetch_details_list. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 123 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line123 Please document this map more thoroughly. How are entries added here, how are they removed, etc. For example, one useful detail is that we retain an entry for an update that is paused. Done. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 124 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line124 Please include in test coverage some validation of the contents of this map so we can gain confidence that we do not have a memory leak. It was already tested by the following assertion: `assertEquals(JobUpdatePulseStatus.FINISHED, updater.pulse(UPDATE_ID));`. This would never return FINISHED if there was an entry in the map. Added to all pulse tests to make sure all scenarios are validated. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 305 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line305 Coverage report indicates this line is not covered. Can you try to cover it? Done. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 488 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line488 Shouldn't this be in an `else`? Looks like we've just wiped a pulse record for terminal updates, but we add another here. It's updated conditionally if PulseState exists. Added else to make it more clear. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 545 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line545 This only has one caller, consider inlining. Yeah, there is only one caller now after refactoring. Merged. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 547 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line547 Coverage report indicates an uncovered branch here. Can you try to cover it? This one is tricky as it's yellow due to `pulse == null`. Given the trivial nature of this branch and complexity of covering it, I'd like to punt it. On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 721 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line721 At risk of being overly-verbose, how about `initializePulseMonitor`? A quick skim of `initializePulse` makes it sound like it might be simulating an inital pulse. How about `initializePulseState`? This seems to fit
Re: Review Request 30818: Support separate routes for job controller tabs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/#review71999 --- src/main/resources/scheduler/assets/js/controllers.js https://reviews.apache.org/r/30818/#comment117923 You should never have to do this with routes in Angular. I think this is a code smell from mixing dynamic and static parts in the same path part. I'd route to these 'views' of the job page using anchor tags or query parameters instead. - David McLaughlin On Feb. 10, 2015, 6:01 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- (Updated Feb. 10, 2015, 6:01 a.m.) Review request for Aurora and David McLaughlin. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing --- ./gradlew jsHint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 30818: Support separate routes for job controller tabs.
On Feb. 11, 2015, 6:58 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 338 https://reviews.apache.org/r/30818/diff/2/?file=859882#file859882line338 You should never have to do this with routes in Angular. I think this is a code smell from mixing dynamic and static parts in the same path part. I'd route to these 'views' of the job page using anchor tags or query parameters instead. Alternatively - you could also change the update paths to .../update/:updateid. Having to update that elsewhere in the code might reveal some other code smells though :) - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/#review71999 --- On Feb. 10, 2015, 6:01 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- (Updated Feb. 10, 2015, 6:01 a.m.) Review request for Aurora and David McLaughlin. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing --- ./gradlew jsHint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Bill's comments. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72014 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/30225/#comment117942 can drop the else? - Joshua Cohen On Feb. 11, 2015, 7:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72022 --- Ship it! Master (7b531e9) 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. 11, 2015, 7:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72016 --- Ship it! Ship It! - Bill Farner On Feb. 11, 2015, 7:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 28617: Implemented offer filtering for tasks with static vetoes.
On Feb. 5, 2015, 12:01 a.m., Bill Farner wrote: Can you see any opportunity to break this diff apart? As it stands i'm having a hard time giving a thoughtful review. Perhaps you can start by introducing the `Assignment` class? Maxim Khutornenko wrote: I'd really prefer keeping this diff as a whole. The Assignment class would not make sense without the entire picture in mind. Bill Farner wrote: It's okay if you commit them back-to-back, but we really need to exercise a pattern for incrementally building features even if they appear as code islands in the commit history. Doing so makes it easier to review with confidence, and more likely that you get faster review turnaround. Splitting this RB into 4 parts for easier reviewing. Keeping it around for history though. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/#review71079 --- On Feb. 4, 2015, 11:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/ --- (Updated Feb. 4, 2015, 11:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Modified the task offer/task matching logic to skip offer matching for tasks previously vetoed statically. Real life testing in vagrant (see pictures) shows close to 50% improvement in task scheduling performance. Testing with JMH shows over 97% better perf when testing with disabled preemptor (1 scheduling loop): ``` Master Benchmark Mode SamplesScore ErrorUnits o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 1008291046.074 ± 145251.995 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 1007522269.050 ± 142446.265 ns/op This RB Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 100 204171.046 ± 3800.124 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 215854.129 ± 8959.851 ns/op ``` Testing with preemptor enabled and no tasks eligible for preemption gives around 40% improvement (2 scheduling loops): ``` Master Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 1001767479.299 ± 26907.571 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 1001538682.287 ± 119119.911 ns/op This RB Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 1001105731.141 ± 10040.721 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 939230.662 ± 11091.505 ns/op ``` Testing with preemptor enabled and running the worst case possible scenario (every slave is eligible and all tasks are victims) yields the least improvement 2-3% (3 scheduling loops). ``` Master Benchmark Mode Samples ScoreError Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 100 11043701.243 ± 40550.259 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 10478631.055 ± 178833.158 ns/op o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 116258653.000 ± 403080.017 ns/op This RB Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 100 10886116.889 ± 193934.324 ns/op o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 10182572.955 ± 35740.891 ns/op
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- Ship it! FWIW I do agree with Bill's comment that minimum pulse time should be enforced in scheduler rather than here. - David McLaughlin On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Josh's comments. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
On Feb. 11, 2015, 8:46 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 276 https://reviews.apache.org/r/30225/diff/7/?file=860914#file860914line276 can drop the else? Sure. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72014 --- On Feb. 11, 2015, 7:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 10:52 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Bill's comments. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to return a new result object. Original RB: https://reviews.apache.org/r/28617/ Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30888/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
On Feb. 11, 2015, 2:13 a.m., Bill Farner wrote: api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 630 https://reviews.apache.org/r/30325/diff/4/?file=859286#file859286line630 s/Key// Done and done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review71904 --- On Feb. 10, 2015, 12:53 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 10, 2015, 12:53 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
On Feb. 10, 2015, 11:57 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1609-1618 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1609 s/final// Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review71878 --- On Feb. 10, 2015, 12:53 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 10, 2015, 12:53 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:51 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Rebased. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift cc2273025aa86ed17da691a143bb5b28226b124e src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 2 of 4: Exposing Veto groups and refactoring TaskVars. Will not apply cleanly: diffed with https://reviews.apache.org/r/30888/ as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 6a43bcd1719e8aa32fd3fcb7387d0318c3c0b804 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f06fdaeb92e154d0982bdabed5df93e7bcba9048 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 52ee7c1e3742d9315c7e7aaa77677121e1e9288d Diff: https://reviews.apache.org/r/30890/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72028 --- Ship it! Ship It! - Joshua Cohen On Feb. 11, 2015, 10:01 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to return a new result object. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30888/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
On Feb. 10, 2015, 7:28 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1609-1618 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1609 I don't know how you feel about the need for a Supplier, but using Optional#or[1] here might read better? [1] http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Optional.html#or(com.google.common.base.Supplier) I feel like it will overcomplicate things for no good reason here. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review71840 --- On Feb. 10, 2015, 12:53 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 10, 2015, 12:53 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:35 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Bill's comments. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift cc2273025aa86ed17da691a143bb5b28226b124e src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72065 --- Ship it! pending clean test run from review bot. - Joshua Cohen On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.
On Feb. 11, 2015, 9:59 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 64 https://reviews.apache.org/r/30888/diff/1/?file=861003#file861003line64 fits on one line ditto elsewhere in this file Fixed. On Feb. 11, 2015, 9:59 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 102 https://reviews.apache.org/r/30888/diff/1/?file=861003#file861003line102 use a constant for this Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72026 --- On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to return a new result object. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30888/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72058 --- Ship it! Master (7b531e9) 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. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 2 of 4: Exposing Veto groups and refactoring TaskVars. Will not apply cleanly: diffed with https://reviews.apache.org/r/30888/ as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 6a43bcd1719e8aa32fd3fcb7387d0318c3c0b804 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f06fdaeb92e154d0982bdabed5df93e7bcba9048 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 52ee7c1e3742d9315c7e7aaa77677121e1e9288d Diff: https://reviews.apache.org/r/30890/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72059 --- This patch does not apply cleanly on master (7b531e9), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 11, 2015, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:35 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift cc2273025aa86ed17da691a143bb5b28226b124e src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72061 --- src/main/java/org/apache/aurora/scheduler/TaskVars.java https://reviews.apache.org/r/30890/#comment117998 s/SchedulingFilter.// src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java https://reviews.apache.org/r/30890/#comment117997 I feel like `VetoType` still applies and is actually a better name. It's odd for an enum value to be considered a 'group'. (When i hear group i think collection.) src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java https://reviews.apache.org/r/30890/#comment117999 Is this comment useful? Ignoring the performance requirements, it seems like a perfectly rational implementation. src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java https://reviews.apache.org/r/30890/#comment118000 Revert to former location? Seems like noise in the diff src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java https://reviews.apache.org/r/30890/#comment118002 What's the thought process behind converting this to a `Set`? The code doesn't create duplicates, and you pick up the performance hit of a hash table. I'm actually wondering if signatures should use `Iterable` all the way out of this class. - Bill Farner On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 2 of 4: Exposing Veto groups and refactoring TaskVars. Will not apply cleanly: diffed with https://reviews.apache.org/r/30888/ as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 6a43bcd1719e8aa32fd3fcb7387d0318c3c0b804 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f06fdaeb92e154d0982bdabed5df93e7bcba9048 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 52ee7c1e3742d9315c7e7aaa77677121e1e9288d Diff: https://reviews.apache.org/r/30890/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72026 --- Ship it! src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java https://reviews.apache.org/r/30888/#comment117959 fits on one line ditto elsewhere in this file src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java https://reviews.apache.org/r/30888/#comment117961 use a constant for this - Bill Farner On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to return a new result object. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30888/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72030 --- src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java https://reviews.apache.org/r/30891/#comment117964 The cohesion of the HostOffer class seems to be rather low. The new field `staticallyBannedOffers` has not much in common with the other members. We might improve this situation by performing the `isStaticallyBanned` check within an overload of the `getWeaklyConsistentOffers` method. - Stephan Erb On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 11, 2015, 11:59 p.m., David McLaughlin wrote: FWIW I do agree with Bill's comment that minimum pulse time should be enforced in scheduler rather than here. I am +1 on the scheduler check as complementary to the client when/if there is a need for it. I am -1 on using scheduler as the only source of truth here given the reasons I mentioned earlier. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72036 --- This patch does not apply cleanly on master (7b531e9), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30225: Modifying update controller to support heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72041 --- Ship it! Master (7b531e9) 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. 11, 2015, 10:01 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1010 https://issues.apache.org/jira/browse/AURORA-1010 Repository: aurora Description --- Added pulsing support into the JobUpdateController. The qualified coordinated updates get blocked until a pulse arrives. An update then becomes active and proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a terminal state (whichever comes first). UPDATE: - Added explicit states to capture blocked updates - Refactored pulseUpdate() to not rely on DB state - Dropped JobUpdatePulseState.PAUSED as it does not seem necessary. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 4f603f9e7ed004e53937a45ea8edf7241e15f5cf src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 7f2ec71d744d5fafac84291cc79feba3398d0e1e src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b7d8d524e15f101416889c00efc3ecf2c8d9c1a1 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d479d20259f284528b2291e2e486b36d8e47fe5e src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java 60f535988a20638fb16515d5027bfa65f1afb73c src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d3b30d48b76d8d7c64cda006a34f7ed3296526f2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ec9d1e07abca1bf30262e1c0f741a34741e96f5d src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 76460f95e058181b711fb6869f5a34c1d5bdfe31 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f9c9ceddc559b43b4a5c45c745d54ff47484edde src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java ca7c0c2675477cc727ca006697665f997972dfde src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 92cfa2b30c1c4daa3ae225fc8609fbeaecdaff7a src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 89765ac3d34a827d3748fb96a275d78e9d1b8b72 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 41d422939209d0808235128e4242c11e8ef25969 Diff: https://reviews.apache.org/r/30225/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
On Feb. 11, 2015, 10:22 p.m., Stephan Erb wrote: src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 316 https://reviews.apache.org/r/30891/diff/1/?file=861043#file861043line316 The cohesion of the HostOffer class seems to be rather low. The new field `staticallyBannedOffers` has not much in common with the other members. We might improve this situation by performing the `isStaticallyBanned` check within an overload of the `getWeaklyConsistentOffers` method. Yeah, this was my original approach but it turned out to be hurting performance too much in benchmark testing (due to excessive filtering and dual map lookup). This is much cleaner and faster, so I'd rather stick with it as I am more concerned about performance than private class cohesiveness in this case. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72030 --- On Feb. 11, 2015, 9:38 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 11, 2015, 9:38 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72043 --- Master (7b531e9) is red with this patch. ./build-support/jenkins/build.sh :processResources :classes :jar :assemble :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' 1 warning :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava :processTestResources :testClasses :checkstyleTest :findbugsJmh :findbugsMain :findbugsTest :licenseJmh UP-TO-DATE :licenseMain UP-TO-DATE :licenseTest UP-TO-DATE :license UP-TO-DATE :pmdMain /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java:314: A switch with less than three branches is inefficient, use a if statement instead. :pmdMain FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':pmdMain'. 1 PMD rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/pmd/main.html * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 3 mins 2.802 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 11, 2015, 10:52 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 10:52 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 1 of 4: TaskAssigner changes to return a new result object. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30888/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/#review72048 --- Ship it! Master (7b531e9) 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. 11, 2015, 9:39 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- (Updated Feb. 11, 2015, 9:39 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 8c11ef8bd6609f3e4d97ca154d922898f8362446 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
On Feb. 11, 2015, 11:44 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1596-1603 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1596 It seems strange that the UPDATE_COORDINATOR can pulse the update but cannot take any other action on it. Maybe allow this role to perform all write actions on updates? Thanks for reminding. I was going to address it separately. Created https://issues.apache.org/jira/browse/AURORA-1119 - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72060 --- On Feb. 11, 2015, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:35 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift cc2273025aa86ed17da691a143bb5b28226b124e src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30325: Implementing pulseJobUpdate RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72071 --- Ship it! Master (b8f71fb) 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. 11, 2015, 11:51 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:51 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1009 https://issues.apache.org/jira/browse/AURORA-1009 Repository: aurora Description --- Implemented the `pulseJobUpdate` RPC. The RB is diffed against https://reviews.apache.org/r/30225/ Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift cc2273025aa86ed17da691a143bb5b28226b124e src/main/java/org/apache/aurora/auth/CapabilityValidator.java 45ef643ebe57c1517cdae373574331ea302a8b74 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fd4d6908fe7680808cfdee9e78c37506af280068 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java a9966a823e9616d0bf9bd19fd62e632d931ddf0a src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 4bf63921f8aa4be943b2b9a7b0be9fb33f7762db Diff: https://reviews.apache.org/r/30325/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72091 --- @ReviewBot retry - Maxim Khutornenko On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72098 --- Master (61e6c35) is red with this patch. ./build-support/jenkins/build.sh Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.collections-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.util Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.util-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.log Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.log-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.process Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.process-0.3.0-py2.7-nspkg.pth Running setup.py install for gitdb building 'gitdb._perf' extension x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c gitdb/_fun.c -o build/temp.linux-x86_64-2.7/gitdb/_fun.o x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c gitdb/_delta_apply.c -o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/gitdb/_fun.o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o -o build/lib.linux-x86_64-2.7/gitdb/_perf.so Running setup.py install for twitter.common.app Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth Running setup.py install for GitPython /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292: UserWarning: The version specified ('0.3.2 RC1') is an invalid version, this may not work as expected with newer versions of setuptools, pip, and PyPI. Please see PEP 440 for more details. details. % self.metadata.version Running setup.py install for pep8 Installing pep8 script to /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin Running setup.py install for pyflakes Installing pyflakes script to /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin Running setup.py install for twitter.checkstyle Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.checkstyle-0.1.0-py2.7-nspkg.pth Installing twitterstyle script to
Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1119 https://issues.apache.org/jira/browse/AURORA-1119 Repository: aurora Description --- Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2a9d36ab2c01960dc5384fc3ed90df4e11c0b12a src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ee329496adf051de717d42c60410c0469f7e90da Diff: https://reviews.apache.org/r/30913/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 30915: Increase findbugs heap size.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- I am no longer able to successfully run gradle as findbugs routinely OOMs. Raising the max heap size to 1g (it's hovering around 450MB before failing). Diffs - build.gradle 32302ec5b2b9546d5420836b700c07f43875e90d Diff: https://reviews.apache.org/r/30915/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30710: add mesos role feature
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/#review72113 --- Ship it! Master (61e6c35) 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. 12, 2015, 6:12 a.m., lozh...@ebay.com zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/ --- (Updated Feb. 12, 2015, 6:12 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1109 https://issues.apache.org/jira/browse/AURORA-1109 Repository: aurora Description --- ## Problems We are from eBay platform team. Previously, we used marathon to generate Jenkins master instance in dedicated vms and recieve resource offer from same dedicated vms. For the details, please refer to http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU Now, we found Aurora is more stable and powerful. We are moving from Marathon to Aurora. During the move, we found there is no mesos role in Aurora now. But we need use mesos role way to solve the problem in section Frameworks stopped receiving offers after a while of the given url. Here is a snippet of the problem description: *We noticed occurred after we used Marathon to create the initial set of CI masters. As those CI masters started registering themselves as frameworks, Marathon stopped receiving any offers from Mesos; essentially, no new CI masters could be launched. Let’s start with Marathon. In the DRF model, it was unfair to treat Marathon in the same bucket/role alongside hundreds of connected Jenkins frameworks. After launching all these Jenkins frameworks, Marathon had a large resource share and Mesos would aggressively offer resources to frameworks that were using little or no resources. Marathon was placed last in priority and got starved out.* *We decided to define a dedicated Mesos role for Marathon and to have all of the Mesos slaves that were reserved for Jenkins master instances support that Mesos role. Jenkins frameworks were left with the default role “*”.*This solved the problem – Mesos offered resources per role and hence Marathon never got starved out. A framework with a special role will get resource offers from both slaves supporting that special role and also from the default role “*”.* However, since we were using placement constraints, Marathon accepted resource offers only from slaves that supported both the role and the placement constraints.* ## Solution So we add role feature is the source code to solve the problem in same way: When accept a resource offer, Aurora will send back the needed resources to Mesos with the mesos role in resource offer. How to configure the Mesos role: 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler. We change the test cases according code change. Each changed test case is green Merge https://github.com/zhanglong2015/incubator-aurora Diffs - src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 1a158b4e0be94762ad0480e8ce74b19bacc90c97 src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b5a3140e3560f790d1db496dca3c2ee0dc96a195 src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java d0994203b5650f44ca2eb32e1e2aa61875163854 src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 5340d651b298ec8aa079e73d6d2f652fdf876293 src/main/java/org/apache/aurora/scheduler/mesos/OfferWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/mesos/ResourceWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java d6febb8998e05257cabe8d193cefa0b6c79f197e src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 5f08d00d39f016af9bc296e517ad49b66ab5a8de src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30710/diff/ Testing --- :buildSrc:compileJava UP-TO-DATE :buildSrc:compileGroovy UP-TO-DATE :buildSrc:processResources UP-TO-DATE :buildSrc:classes UP-TO-DATE
Re: Review Request 30710: add mesos role feature
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/ --- (Updated Feb. 12, 2015, 6:12 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Changes --- 1.Left the role unset in register framework message if no role specified in command line 2.For below case, will use the resource with role aurora first. If the resource with role aurora is not enough for task, then use resource with role * together. For example, a task need 5 CPU, then willl use cpus(aurora):4 and cpus(*):1. I am afraid the current approach is not going to work in a cluster with mesos slaves configured to offer multi-role resources. Consider an example where a slave is configured to offer both role-specific and general pool CPU: --resources=cpus(aurora):4;cpus(*):2 Bugs: AURORA-1109 https://issues.apache.org/jira/browse/AURORA-1109 Repository: aurora Description --- ## Problems We are from eBay platform team. Previously, we used marathon to generate Jenkins master instance in dedicated vms and recieve resource offer from same dedicated vms. For the details, please refer to http://www.ebaytechblog.com/2014/04/04/delivering-ebays-ci-solution-with-apache-mesos-part-i/#.VNQUuC6_SPU Now, we found Aurora is more stable and powerful. We are moving from Marathon to Aurora. During the move, we found there is no mesos role in Aurora now. But we need use mesos role way to solve the problem in section Frameworks stopped receiving offers after a while of the given url. Here is a snippet of the problem description: *We noticed occurred after we used Marathon to create the initial set of CI masters. As those CI masters started registering themselves as frameworks, Marathon stopped receiving any offers from Mesos; essentially, no new CI masters could be launched. Let’s start with Marathon. In the DRF model, it was unfair to treat Marathon in the same bucket/role alongside hundreds of connected Jenkins frameworks. After launching all these Jenkins frameworks, Marathon had a large resource share and Mesos would aggressively offer resources to frameworks that were using little or no resources. Marathon was placed last in priority and got starved out.* *We decided to define a dedicated Mesos role for Marathon and to have all of the Mesos slaves that were reserved for Jenkins master instances support that Mesos role. Jenkins frameworks were left with the default role “*”.*This solved the problem – Mesos offered resources per role and hence Marathon never got starved out. A framework with a special role will get resource offers from both slaves supporting that special role and also from the default role “*”.* However, since we were using placement constraints, Marathon accepted resource offers only from slaves that supported both the role and the placement constraints.* ## Solution So we add role feature is the source code to solve the problem in same way: When accept a resource offer, Aurora will send back the needed resources to Mesos with the mesos role in resource offer. How to configure the Mesos role: 1.Add cmd option --mesos_role=${Mesos role name} when start Aurora scheduler. We change the test cases according code change. Each changed test case is green Merge https://github.com/zhanglong2015/incubator-aurora Diffs (updated) - src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 1a158b4e0be94762ad0480e8ce74b19bacc90c97 src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b5a3140e3560f790d1db496dca3c2ee0dc96a195 src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java d0994203b5650f44ca2eb32e1e2aa61875163854 src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 5340d651b298ec8aa079e73d6d2f652fdf876293 src/main/java/org/apache/aurora/scheduler/mesos/OfferWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/mesos/ResourceWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java d6febb8998e05257cabe8d193cefa0b6c79f197e src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 5f08d00d39f016af9bc296e517ad49b66ab5a8de src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4 Diff: https://reviews.apache.org/r/30710/diff/ Testing --- :buildSrc:compileJava UP-TO-DATE :buildSrc:compileGroovy UP-TO-DATE :buildSrc:processResources
Re: Review Request 30915: Increase findbugs heap size.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/#review72103 --- Ship it! Master (61e6c35) 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. 12, 2015, 2:57 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/ --- (Updated Feb. 12, 2015, 2:57 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- I am no longer able to successfully run gradle as findbugs routinely OOMs. Raising the max heap size to 1g (it's hovering around 450MB before failing). Diffs - build.gradle 32302ec5b2b9546d5420836b700c07f43875e90d Diff: https://reviews.apache.org/r/30915/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/#review72104 --- Ship it! Master (61e6c35) 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. 12, 2015, 2:47 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/ --- (Updated Feb. 12, 2015, 2:47 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1119 https://issues.apache.org/jira/browse/AURORA-1119 Repository: aurora Description --- Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2a9d36ab2c01960dc5384fc3ed90df4e11c0b12a src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ee329496adf051de717d42c60410c0469f7e90da Diff: https://reviews.apache.org/r/30913/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko