You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2015/09/04 14:06:47 UTC

Review Request 38126: Add UT for QuiesceOffers

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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod Kone.


Bugs: MESOS-3037
    https://issues.apache.org/jira/browse/MESOS-3037


Repository: mesos


Description
-------

Add UT for QuiesceOffers


Diffs
-----

  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 38126: Add UT for QuiesceOffers

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38126/#review97750
-----------------------------------------------------------


Patch looks great!

Reviews applied: [37532, 37873, 38119, 38120, 38121, 38124, 38126]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
>     https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -----
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 38126: Add UT for QuiesceOffers

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38126/
-----------------------------------------------------------

(Updated 九月 16, 2015, 5:44 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod Kone.


Bugs: MESOS-3037
    https://issues.apache.org/jira/browse/MESOS-3037


Repository: mesos


Description
-------

Add UT for QuiesceOffers


Diffs
-----

  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 38126: Add UT for QuiesceOffers

Posted by Guangya Liu <gy...@gmail.com>.

> On 九月 15, 2015, 7:43 p.m., Vinod Kone wrote:
> > you should've merged this test review with the dependent review. i really like tests to be present in the same patch as the code change, to give me confidence that the code changes are correct.
> > 
> > also, no tests for the scheduler driver?

This is now merged to https://reviews.apache.org/r/38124/diff/2/ 

I'm now working on another patch to add some tests for schduler driver. I see that the reviveOffers also do not have a test, will try to see if the test can cover both cases: revive and quiesce offer.


> On 九月 15, 2015, 7:43 p.m., Vinod Kone wrote:
> > src/tests/scheduler_tests.cpp, lines 1004-1005
> > <https://reviews.apache.org/r/38126/diff/1/?file=1064033#file1064033line1004>
> >
> >     // On revival scheduler should get another offer with the same amount of resources.
> >     
> >     also, i'm a bit confused on how/why the scheduler gets another offer (for the same resources) considering the framework is holding on to the original offer?

I updated the logic a bit, the sequence is as this: subscribe->get offer->decline offer with a filter of 60min->enable quiesce offer-> wait 100min, no offer->revive the offer->get offer, please refer to https://reviews.apache.org/r/38124/diff/2/ for detail. Thanks.


- Guangya


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


On 九月 16, 2015, 5:44 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> -----------------------------------------------------------
> 
> (Updated 九月 16, 2015, 5:44 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
>     https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -----
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 38126: Add UT for QuiesceOffers

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38126/#review99078
-----------------------------------------------------------


you should've merged this test review with the dependent review. i really like tests to be present in the same patch as the code change, to give me confidence that the code changes are correct.

also, no tests for the scheduler driver?


src/tests/scheduler_tests.cpp (lines 1004 - 1005)
<https://reviews.apache.org/r/38126/#comment155911>

    // On revival scheduler should get another offer with the same amount of resources.
    
    also, i'm a bit confused on how/why the scheduler gets another offer (for the same resources) considering the framework is holding on to the original offer?


- Vinod Kone


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
>     https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -----
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>