You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2017/05/24 21:32:09 UTC
Review Request 59541: Fixed flakiness in
MasterAllocatorTest.FrameworkExited.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59541/
-----------------------------------------------------------
Review request for mesos and Anand Mazumdar.
Bugs: MESOS-7552
https://issues.apache.org/jira/browse/MESOS-7552
Repository: mesos
Description
-------
This test did not pause the clock; hence, batch allocations could occur
at unpredictable times and cause the test to fail.
Fix this by pausing the clock and explicitly advancing it as needed to
trigger batch allocations or other events.
Diffs
-----
src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5
Diff: https://reviews.apache.org/r/59541/diff/1/
Testing
-------
`./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" --gtest_break_on_failure --gtest_repeat=5000` # Linux
Without this RR, the test fails within a few hundred iterations.
Thanks,
Neil Conway
Re: Review Request 59541: Fixed flakiness in
MasterAllocatorTest.FrameworkExited.
Posted by Neil Conway <ne...@gmail.com>.
> On May 25, 2017, 3:58 a.m., Anand Mazumdar wrote:
> > src/tests/master_allocator_tests.cpp
> > Lines 719-720 (patched)
> > <https://reviews.apache.org/r/59541/diff/1/?file=1732021#file1732021line736>
> >
> > hmm, not immediately clear to me what guarrantees that this method is invoked once before the end of the function?
> >
> > - `driver.stop()` would send the teardown message to the master and immediately trigger the latch making `driver.join()` return.
> > - If the function finishes before the message is processed by the master, `removeFramework()` might not be invoked.
Good catch -- I'm not sure offhand if that sequence is possible, but the intent here is just to reset the expectation to the default value, so `DoRepeatedly()` is probably better anyway.
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59541/#review176038
-----------------------------------------------------------
On May 24, 2017, 9:32 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59541/
> -----------------------------------------------------------
>
> (Updated May 24, 2017, 9:32 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7552
> https://issues.apache.org/jira/browse/MESOS-7552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test did not pause the clock; hence, batch allocations could occur
> at unpredictable times and cause the test to fail.
>
> Fix this by pausing the clock and explicitly advancing it as needed to
> trigger batch allocations or other events.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5
>
>
> Diff: https://reviews.apache.org/r/59541/diff/1/
>
>
> Testing
> -------
>
> `./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" --gtest_break_on_failure --gtest_repeat=5000` # Linux
>
> Without this RR, the test fails within a few hundred iterations.
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 59541: Fixed flakiness in
MasterAllocatorTest.FrameworkExited.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59541/#review176038
-----------------------------------------------------------
Looks good minus a small query.
src/tests/master_allocator_tests.cpp
Lines 719-720 (patched)
<https://reviews.apache.org/r/59541/#comment249369>
hmm, not immediately clear to me what guarrantees that this method is invoked once before the end of the function?
- `driver.stop()` would send the teardown message to the master and immediately trigger the latch making `driver.join()` return.
- If the function finishes before the message is processed by the master, `removeFramework()` might not be invoked.
- Anand Mazumdar
On May 24, 2017, 9:32 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59541/
> -----------------------------------------------------------
>
> (Updated May 24, 2017, 9:32 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7552
> https://issues.apache.org/jira/browse/MESOS-7552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test did not pause the clock; hence, batch allocations could occur
> at unpredictable times and cause the test to fail.
>
> Fix this by pausing the clock and explicitly advancing it as needed to
> trigger batch allocations or other events.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5
>
>
> Diff: https://reviews.apache.org/r/59541/diff/1/
>
>
> Testing
> -------
>
> `./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" --gtest_break_on_failure --gtest_repeat=5000` # Linux
>
> Without this RR, the test fails within a few hundred iterations.
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 59541: Fixed flakiness in
MasterAllocatorTest.FrameworkExited.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59541/#review176115
-----------------------------------------------------------
Ship it!
LGTM!
- Anand Mazumdar
On May 25, 2017, 6:36 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59541/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 6:36 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7552
> https://issues.apache.org/jira/browse/MESOS-7552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test did not pause the clock; hence, batch allocations could occur
> at unpredictable times and cause the test to fail.
>
> Fix this by pausing the clock and explicitly advancing it as needed to
> trigger batch allocations or other events.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5
>
>
> Diff: https://reviews.apache.org/r/59541/diff/2/
>
>
> Testing
> -------
>
> `./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" --gtest_break_on_failure --gtest_repeat=5000` # Linux
>
> Without this RR, the test fails within a few hundred iterations.
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 59541: Fixed flakiness in
MasterAllocatorTest.FrameworkExited.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59541/
-----------------------------------------------------------
(Updated May 25, 2017, 6:36 p.m.)
Review request for mesos and Anand Mazumdar.
Changes
-------
Address review comments.
Bugs: MESOS-7552
https://issues.apache.org/jira/browse/MESOS-7552
Repository: mesos
Description
-------
This test did not pause the clock; hence, batch allocations could occur
at unpredictable times and cause the test to fail.
Fix this by pausing the clock and explicitly advancing it as needed to
trigger batch allocations or other events.
Diffs (updated)
-----
src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5
Diff: https://reviews.apache.org/r/59541/diff/2/
Changes: https://reviews.apache.org/r/59541/diff/1-2/
Testing
-------
`./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" --gtest_break_on_failure --gtest_repeat=5000` # Linux
Without this RR, the test fails within a few hundred iterations.
Thanks,
Neil Conway