Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread David McLaughlin

---
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.

2015-02-11 Thread David McLaughlin


 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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Joshua Cohen

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Bill Farner

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread David McLaughlin

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Joshua Cohen

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Joshua Cohen

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Bill Farner

---
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.

2015-02-11 Thread Bill Farner

---
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.

2015-02-11 Thread Stephan Erb

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Maxim Khutornenko


 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.

2015-02-11 Thread Aurora ReviewBot

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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

2015-02-11 Thread Maxim Khutornenko

---
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.

2015-02-11 Thread Maxim Khutornenko

---
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

2015-02-11 Thread Aurora ReviewBot

---
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

2015-02-11 Thread lozh...@ebay.com zhang

---
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.

2015-02-11 Thread Aurora ReviewBot

---
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

2015-02-11 Thread Aurora ReviewBot

---
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