You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/09/12 03:52:58 UTC

Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

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

Review request for mesos, Benjamin Hindman and Ben Mahler.


Bugs: MESOS-467 and MESOS-686
    https://issues.apache.org/jira/browse/MESOS-467
    https://issues.apache.org/jira/browse/MESOS-686


Repository: mesos-git


Description
-------

TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.

Also fixed tests that were incorrectly launching multiple executors.


Diffs
-----

  src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
  src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
  src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
  src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
  src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
  src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
  src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
  src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle

sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle


Thanks,

Vinod Kone


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

Posted by Vinod Kone <vi...@gmail.com>.

> On Sept. 20, 2013, 2:55 a.m., Ben Mahler wrote:
> > src/tests/isolator.hpp, lines 100-104
> > <https://reviews.apache.org/r/14097/diff/1/?file=351099#file351099line100>
> >
> >     Consider logging this explicitly when the framework id is different and the executor id is the same (to avoid potential confusion later).

logged the framework id. we don't store the framework ids, so we can't tell if it was previously launched by the same framework or different.


> On Sept. 20, 2013, 2:55 a.m., Ben Mahler wrote:
> > src/tests/master_tests.cpp, line 465
> > <https://reviews.apache.org/r/14097/diff/1/?file=351100#file351100line465>
> >
> >     Why the s/executorInfo.executor_id()/DEFAULT_EXECUTOR_ID/ change here?

good catch. fixed.


- Vinod


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


On Sept. 12, 2013, 1:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14097/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2013, 1:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-467 and MESOS-686
>     https://issues.apache.org/jira/browse/MESOS-467
>     https://issues.apache.org/jira/browse/MESOS-686
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.
> 
> Also fixed tests that were incorrectly launching multiple executors.
> 
> 
> Diffs
> -----
> 
>   src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
>   src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
>   src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
>   src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
>   src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
>   src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
>   src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
>   src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 
> 
> Diff: https://reviews.apache.org/r/14097/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14097/#review26291
-----------------------------------------------------------


Looks good, I just had a comment below to try to clean up some of the boilerplate code needed in the tests to set up the executors / testing isolator.


src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/14097/#comment51381>

    Seems like a lot of boilerplate to set up the TestingIsolator! Why not use CREATE_EXECUTOR_INFO up here and have it take a string for the executorId? This saves the need to explicitly construct and set the executor id.
    
    e.g.
    
    ExecutorInfo executor1 = CREATE_EXECUTOR_INFO("e1", "exit 1");
    
    Ditto elsewhere.



src/tests/isolator.hpp
<https://reviews.apache.org/r/14097/#comment51377>

    Consider logging this explicitly when the framework id is different and the executor id is the same (to avoid potential confusion later).



src/tests/master_tests.cpp
<https://reviews.apache.org/r/14097/#comment51378>

    Why the s/executorInfo.executor_id()/DEFAULT_EXECUTOR_ID/ change here?


- Ben Mahler


On Sept. 12, 2013, 1:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14097/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2013, 1:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-467 and MESOS-686
>     https://issues.apache.org/jira/browse/MESOS-467
>     https://issues.apache.org/jira/browse/MESOS-686
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.
> 
> Also fixed tests that were incorrectly launching multiple executors.
> 
> 
> Diffs
> -----
> 
>   src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
>   src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
>   src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
>   src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
>   src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
>   src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
>   src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
>   src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 
> 
> Diff: https://reviews.apache.org/r/14097/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14097/#review27143
-----------------------------------------------------------

Ship it!


Ship It!

- Ben Mahler


On Sept. 24, 2013, 7:22 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14097/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2013, 7:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-467 and MESOS-686
>     https://issues.apache.org/jira/browse/MESOS-467
>     https://issues.apache.org/jira/browse/MESOS-686
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.
> 
> Also fixed tests that were incorrectly launching multiple executors.
> 
> 
> Diffs
> -----
> 
>   src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
>   src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
>   src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
>   src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
>   src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
>   src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
>   src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
>   src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 
> 
> Diff: https://reviews.apache.org/r/14097/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

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

(Updated Oct. 18, 2013, 12:37 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

rebase. NNFR.


Bugs: MESOS-467 and MESOS-686
    https://issues.apache.org/jira/browse/MESOS-467
    https://issues.apache.org/jira/browse/MESOS-686


Repository: mesos-git


Description
-------

TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.

Also fixed tests that were incorrectly launching multiple executors.


Diffs (updated)
-----

  src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
  src/tests/allocator_tests.cpp 49fad2df7af08211c9306d0c56bffd4f8c253a15 
  src/tests/allocator_zookeeper_tests.cpp 664b77b5c4eb2d583645e120b6b18602377b7099 
  src/tests/fault_tolerance_tests.cpp 198161a17cb96d93d3b49c1b56bbcf288f0f1bed 
  src/tests/gc_tests.cpp 375f762c5a9537f87e094f66a933257f6eb267bd 
  src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
  src/tests/master_tests.cpp feea541c62eac4ff879ff78f6b143cc075ca4a09 
  src/tests/mesos.hpp 4358c8679907ce5050aafa6f552b2418f0ff12ba 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle

sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle


Thanks,

Vinod Kone


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

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


ping?

- Vinod Kone


On Sept. 24, 2013, 7:22 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14097/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2013, 7:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-467 and MESOS-686
>     https://issues.apache.org/jira/browse/MESOS-467
>     https://issues.apache.org/jira/browse/MESOS-686
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.
> 
> Also fixed tests that were incorrectly launching multiple executors.
> 
> 
> Diffs
> -----
> 
>   src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
>   src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
>   src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
>   src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
>   src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
>   src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
>   src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
>   src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 
> 
> Diff: https://reviews.apache.org/r/14097/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 14097: Fixed testing isolator and tests to properly handle multiple executors.

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

(Updated Sept. 24, 2013, 7:22 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

benm's.


Bugs: MESOS-467 and MESOS-686
    https://issues.apache.org/jira/browse/MESOS-467
    https://issues.apache.org/jira/browse/MESOS-686


Repository: mesos-git


Description
-------

TestingIsolator now rejects launching of multiple executors with same ExecutorID. This means 2 different frameworks cannot used the same executor id. While this is a restriction it is better than changing all tests to grab the framework id before creating testing isolator.

Also fixed tests that were incorrectly launching multiple executors.


Diffs (updated)
-----

  src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 
  src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 
  src/tests/allocator_zookeeper_tests.cpp 6e3214c15c14dc8ba82082738c172c8833cd0887 
  src/tests/fault_tolerance_tests.cpp 10e52c401476eb8416361de49b8e4061bb7ac4f3 
  src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f 
  src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 
  src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 
  src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle

sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle


Thanks,

Vinod Kone