You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joshua Cohen <jc...@twitter.com> on 2014/09/05 23:30:44 UTC

Review Request 25398: Make the offer hold jitter window configurable.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/
-----------------------------------------------------------

Review request for Aurora and Bill Farner.


Bugs: AURORA-313
    https://issues.apache.org/jira/browse/AURORA-313


Repository: aurora


Description
-------

Make the offer hold jitter window configurable.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 

Diff: https://reviews.apache.org/r/25398/diff/


Testing
-------

gradle -Pq build


Thanks,

Joshua Cohen

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.

> On Sept. 8, 2014, 8:41 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java, line 71
> > <https://reviews.apache.org/r/25398/diff/2/?file=682734#file682734line71>
> >
> >     How about a assertRandomJitterReturnDelay(0, 0) test?

Added.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52630
-----------------------------------------------------------


On Sept. 8, 2014, 9:09 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 9:09 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52630
-----------------------------------------------------------

Ship it!



src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java
<https://reviews.apache.org/r/25398/#comment91484>

    How about a assertRandomJitterReturnDelay(0, 0) test?


- Maxim Khutornenko


On Sept. 8, 2014, 5:55 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 5:55 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Bill Farner <wf...@apache.org>.

> On Sept. 10, 2014, 10:15 p.m., Bill Farner wrote:
> > Ship It!

Thanks! This is now on master:

```
commit cc70136
Author: Joshua Cohen <jc...@twopensource.com>
Date:   Wed Sep 10 15:38:56 2014 -0700

    Make the offer hold jitter window configurable.
    
    Bugs closed: AURORA-313
    
    Reviewed at https://reviews.apache.org/r/25398/
```


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52958
-----------------------------------------------------------


On Sept. 9, 2014, 4:54 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 4:54 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52958
-----------------------------------------------------------

Ship it!


Ship It!

- Bill Farner


On Sept. 9, 2014, 4:54 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 4:54 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/
-----------------------------------------------------------

(Updated Sept. 9, 2014, 4:54 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
-------

Use mock random for all tests.


Bugs: AURORA-313
    https://issues.apache.org/jira/browse/AURORA-313


Repository: aurora


Description
-------

Make the offer hold jitter window configurable.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
  src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/25398/diff/


Testing
-------

gradle -Pq build


Thanks,

Joshua Cohen

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twopensource.com>.

> On Sept. 9, 2014, 4:45 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java, line 53
> > <https://reviews.apache.org/r/25398/diff/3/?file=682822#file682822line53>
> >
> >     Consider using the mock throughout.  You get a little extra bit of verification that it is not invoked.

Updated.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52735
-----------------------------------------------------------


On Sept. 9, 2014, 4:54 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 4:54 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52735
-----------------------------------------------------------



src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java
<https://reviews.apache.org/r/25398/#comment91720>

    Consider using the mock throughout.  You get a little extra bit of verification that it is not invoked.


- Bill Farner


On Sept. 8, 2014, 9:09 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 9:09 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/
-----------------------------------------------------------

(Updated Sept. 8, 2014, 9:09 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
-------

Added a 0,0 test.


Bugs: AURORA-313
    https://issues.apache.org/jira/browse/AURORA-313


Repository: aurora


Description
-------

Make the offer hold jitter window configurable.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
  src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/25398/diff/


Testing
-------

gradle -Pq build


Thanks,

Joshua Cohen

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/
-----------------------------------------------------------

(Updated Sept. 8, 2014, 5:55 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
-------

+maxim as reviewer.


Bugs: AURORA-313
    https://issues.apache.org/jira/browse/AURORA-313


Repository: aurora


Description
-------

Make the offer hold jitter window configurable.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
  src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/25398/diff/


Testing
-------

gradle -Pq build


Thanks,

Joshua Cohen

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52613
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/25398/#comment91463>

    Should these be @Positive instead (i.e. do we want to allow for a zero hold time/jitter window)?


- Joshua Cohen


On Sept. 8, 2014, 5:54 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 5:54 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
>   src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/
-----------------------------------------------------------

(Updated Sept. 8, 2014, 5:54 p.m.)


Review request for Aurora and Bill Farner.


Changes
-------

Refactor to add tests.


Bugs: AURORA-313
    https://issues.apache.org/jira/browse/AURORA-313


Repository: aurora


Description
-------

Make the offer hold jitter window configurable.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
  src/main/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelay.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/RandomJitterReturnDelayTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/25398/diff/


Testing
-------

gradle -Pq build


Thanks,

Joshua Cohen

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Joshua Cohen <jc...@twitter.com>.

> On Sept. 5, 2014, 9:44 p.m., Bill Farner wrote:
> > Mind exposing RandomJitterReturnDelay and adding a unit test?  You'll need a way to plumb in your own Random as well.

Yeah, I mulled that over, but was wary of exposing it. Happy to to do so though.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52514
-----------------------------------------------------------


On Sept. 5, 2014, 9:30 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2014, 9:30 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 25398: Make the offer hold jitter window configurable.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25398/#review52514
-----------------------------------------------------------


Mind exposing RandomJitterReturnDelay and adding a unit test?  You'll need a way to plumb in your own Random as well.

- Bill Farner


On Sept. 5, 2014, 9:30 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25398/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2014, 9:30 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-313
>     https://issues.apache.org/jira/browse/AURORA-313
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the offer hold jitter window configurable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b 
> 
> Diff: https://reviews.apache.org/r/25398/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>