You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2015/11/17 21:18:57 UTC

Review Request 40396: Quota: Added a test for offer rescinding.

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

Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.


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


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
-------

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Nov. 25, 2015, 11:35 p.m., Joris Van Remoortere wrote:
> > Added
> > ```
> > EXPECT_CALL(sched2, resourceOffers(&framework2, _))
> >   .Times(0);
> > EXPECT_CALL(sched3, resourceOffers(&framework3, _))
> >   .Times(0);
> > ```
> > to resolve The assertion issue opened.
> > 
> > Tested with gtest_shuffle, repeat

I'm afraid this may render the test flaky. At some point, after quota request reaches the allocator and offers are rescinded, an allocation for `framework2` and `framework3` may happen. In this case, we race with the allocation timeout. This is the reason why I haven't introduced this expectations. I think we can fix it by either carefully stopping the clock or intercepting `ResourceOffers` message. What do you think?


- Alexander


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


On Nov. 24, 2015, 4:29 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2015, 4:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3985
>     https://issues.apache.org/jira/browse/MESOS-3985
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp 330e591f81c7ece7f401042ad159bd6b55881a84 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40396/#review108074
-----------------------------------------------------------

Ship it!


Added
```
EXPECT_CALL(sched2, resourceOffers(&framework2, _))
  .Times(0);
EXPECT_CALL(sched3, resourceOffers(&framework3, _))
  .Times(0);
```
to resolve The assertion issue opened.

Tested with gtest_shuffle, repeat

- Joris Van Remoortere


On Nov. 24, 2015, 4:29 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2015, 4:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3985
>     https://issues.apache.org/jira/browse/MESOS-3985
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp 330e591f81c7ece7f401042ad159bd6b55881a84 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40396/
-----------------------------------------------------------

(Updated Nov. 24, 2015, 4:29 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/master_quota_tests.cpp 330e591f81c7ece7f401042ad159bd6b55881a84 

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


Testing
-------

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40396/
-----------------------------------------------------------

(Updated Nov. 23, 2015, 10:45 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.


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


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
-------

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Nov. 23, 2015, 4:09 a.m., Guangya Liu wrote:
> > src/tests/master_quota_tests.cpp, line 542
> > <https://reviews.apache.org/r/40396/diff/2/?file=1134003#file1134003line542>
> >
> >     Add ASSERT here to make sure no offers?

Could you please elaborate what exactly do you propose?


- Alexander


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


On Nov. 23, 2015, 10:45 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 10:45 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3985
>     https://issues.apache.org/jira/browse/MESOS-3985
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 40396: Quota: Added a test for offer rescinding.

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



src/tests/master_quota_tests.cpp (line 539)
<https://reviews.apache.org/r/40396/#comment166770>

    Add ASSERT here to make sure no offers?


- Guangya Liu


On 十一月 20, 2015, 8:21 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated 十一月 20, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
>     https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40396/
-----------------------------------------------------------

(Updated Nov. 20, 2015, 8:21 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.


Changes
-------

Fixed a race.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
-------

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov


Re: Review Request 40396: Quota: Added a test for offer rescinding.

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


Patch looks great!

Reviews applied: [39211, 39018, 39102, 36913, 38059, 39285, 38110, 40342, 40351, 38956, 40396]

All tests passed.

- Mesos ReviewBot


On Nov. 17, 2015, 8:18 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2015, 8:18 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
>     https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 40396: Quota: Added a test for offer rescinding.

Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40396/#review107310
-----------------------------------------------------------



src/tests/master_quota_tests.cpp (line 454)
<https://reviews.apache.org/r/40396/#comment166368>

    This should be before StartSlave() as otherwise the test might be flaky (in cases where the slave has been added before the Expect_Call)



src/tests/master_quota_tests.cpp (lines 465 - 466)
<https://reviews.apache.org/r/40396/#comment166369>

    same here



src/tests/master_quota_tests.cpp (line 478)
<https://reviews.apache.org/r/40396/#comment166370>

    same here


- Joerg Schad


On Nov. 17, 2015, 8:18 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2015, 8:18 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
>     https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>