You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Thomas Marshall <tw...@gmail.com> on 2013/03/12 22:46:55 UTC
Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Description
-------
In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
This addresses bug MESOS-386.
https://issues.apache.org/jira/browse/MESOS-386
Diffs
-----
src/tests/allocator_tests.cpp b953cd1
Diff: https://reviews.apache.org/r/9887/diff/
Testing
-------
make check
Thanks,
Thomas Marshall
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Thomas Marshall <tw...@gmail.com>.
> On March 12, 2013, 10:33 p.m., Vinod Kone wrote:
> > On a related note, maybe in another review, it would be great to have more comments (on top of or inside) each allocator test regarding what it is doing/expected.
https://reviews.apache.org/r/9407/
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/#review17766
-----------------------------------------------------------
On March 12, 2013, 9:46 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9887/
> -----------------------------------------------------------
>
> (Updated March 12, 2013, 9:46 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
>
>
> This addresses bug MESOS-386.
> https://issues.apache.org/jira/browse/MESOS-386
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp b953cd1
>
> Diff: https://reviews.apache.org/r/9887/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/#review17766
-----------------------------------------------------------
Ship it!
On a related note, maybe in another review, it would be great to have more comments (on top of or inside) each allocator test regarding what it is doing/expected.
- Vinod Kone
On March 12, 2013, 9:46 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9887/
> -----------------------------------------------------------
>
> (Updated March 12, 2013, 9:46 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
>
>
> This addresses bug MESOS-386.
> https://issues.apache.org/jira/browse/MESOS-386
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp b953cd1
>
> Diff: https://reviews.apache.org/r/9887/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/
-----------------------------------------------------------
(Updated March 20, 2013, 5:40 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
additional testing
Description
-------
In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
This addresses bug MESOS-386.
https://issues.apache.org/jira/browse/MESOS-386
Diffs
-----
src/tests/allocator_tests.cpp 4ae8bfd
Diff: https://reviews.apache.org/r/9887/diff/
Testing (updated)
-------
make check
./bin/mesos-tests.sh --gtest_filter=*TaskFinished* --gtest_repeat=300 --gtest_break_on_failure
Thanks,
Thomas Marshall
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/#review18135
-----------------------------------------------------------
Ship it!
Ditto on using --gtest_repeat here to check for flakiness.
- Ben Mahler
On March 19, 2013, 9:49 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9887/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 9:49 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
>
>
> This addresses bug MESOS-386.
> https://issues.apache.org/jira/browse/MESOS-386
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp 4ae8bfd
>
> Diff: https://reviews.apache.org/r/9887/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/
-----------------------------------------------------------
(Updated March 19, 2013, 9:49 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Upon further review, I realized that its not actually guaranteed that resourcesChanged will be called three times, no matter how long we wait, depending on how the slave is implemented, so since we don't actually care how many times it gets called in this test anyways (what's important is that the task gets launched and killed, which we have expectations for) its better to leave the test flexible and not specify the number of times we expect resourcesChanged to be called.
Description
-------
In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
This addresses bug MESOS-386.
https://issues.apache.org/jira/browse/MESOS-386
Diffs (updated)
-----
src/tests/allocator_tests.cpp 4ae8bfd
Diff: https://reviews.apache.org/r/9887/diff/
Testing
-------
make check
Thanks,
Thomas Marshall
Re: Review Request: Fixes the flaky AllocatorTests.TaskFinished unit test
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9887/#review17764
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On March 12, 2013, 9:46 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9887/
> -----------------------------------------------------------
>
> (Updated March 12, 2013, 9:46 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> In the TaskFinished unit test, TestingIsolationModule::resourcesChanged should be called three times - once each for the two tasks that are launched, and once for the task that is killed. The way the test was written before, we were only expecting 2 calls, and since the call resulting from the task being killed usually didn't happen before the test was shut down this worked most but not all of the time, causing occasional flakiness. This patch changes it to expect 3 calls and adds a trigger so that we wait until the third call happens before shutting down the test.
>
>
> This addresses bug MESOS-386.
> https://issues.apache.org/jira/browse/MESOS-386
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp b953cd1
>
> Diff: https://reviews.apache.org/r/9887/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>