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/03/01 02:37:32 UTC

Review Request: Fixed allocator, gc and master tests

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

Review request for mesos, Benjamin Hindman and Ben Mahler.


Description
-------

See summary.


Diffs
-----

  src/master/drf_sorter.cpp 33a8ec8ba0c5d4189719feb0f4b001a91910bd35 
  src/tests/allocator_tests.cpp ad3de212c6ce71d0f3dc14d6198a899a00a35bc8 
  src/tests/gc_tests.cpp 90b3a26ded877cff6eb6e29bf8139950ce237950 
  src/tests/master_tests.cpp 104610cef3fdaaea75271657c42c4fd2b6b61618 
  src/tests/utils.hpp b648b631d8c11c26e6adfa3f5b16012044557e25 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request: Fixed allocator, gc and master tests

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

Ship it!


Ship It!

- Ben Mahler


On March 1, 2013, 2:23 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9690/
> -----------------------------------------------------------
> 
> (Updated March 1, 2013, 2:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> Changes:
> 
> --> TestingIsolationModule now always sends executorStarted and executorTerminated messages after launching and killing executors respectively. This is because some of the tests either 1) expect an executorTerminated() to be received after killing an executor or 2) expect executorStarted() before getting executorExited() (e.g., resource monitor). Also, since executorStarted() is no longer a no-op it makes sense for testing isolation module to send it, to satisfy slave's expectation.
> 
> --> Changed allocator tests to use testing isolation module instead of process based isolation module, because at least one of the tests (ResourcesUnused) was not doing proper teardown of the executor, resulting in errors. In general, I don't think tests should spawn executor processes unless they are testing real executor semantics. This makes the tests clean and concise.
> 
> --> Fixed DRFAllocatorProcess test by adding 'disk:0' to slave resources, because not specifying 'disk' defaults to non-zero disk (MesosTest sets it to 1024) being allocated by the slave. Non-zero disk resource, breaks the assumption in the test on which framework gets the which offers (e.g. framework1 gets slave3's offer instead of framework2).
> 
> 
> Diffs
> -----
> 
>   src/master/drf_sorter.cpp 33a8ec8ba0c5d4189719feb0f4b001a91910bd35 
>   src/tests/allocator_tests.cpp ad3de212c6ce71d0f3dc14d6198a899a00a35bc8 
>   src/tests/gc_tests.cpp 90b3a26ded877cff6eb6e29bf8139950ce237950 
>   src/tests/master_tests.cpp 104610cef3fdaaea75271657c42c4fd2b6b61618 
>   src/tests/utils.hpp b648b631d8c11c26e6adfa3f5b16012044557e25 
> 
> Diff: https://reviews.apache.org/r/9690/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request: Fixed allocator, gc and master tests

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

(Updated March 1, 2013, 2:23 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

updated description.


Description (updated)
-------

Changes:

--> TestingIsolationModule now always sends executorStarted and executorTerminated messages after launching and killing executors respectively. This is because some of the tests either 1) expect an executorTerminated() to be received after killing an executor or 2) expect executorStarted() before getting executorExited() (e.g., resource monitor). Also, since executorStarted() is no longer a no-op it makes sense for testing isolation module to send it, to satisfy slave's expectation.

--> Changed allocator tests to use testing isolation module instead of process based isolation module, because at least one of the tests (ResourcesUnused) was not doing proper teardown of the executor, resulting in errors. In general, I don't think tests should spawn executor processes unless they are testing real executor semantics. This makes the tests clean and concise.

--> Fixed DRFAllocatorProcess test by adding 'disk:0' to slave resources, because not specifying 'disk' defaults to non-zero disk (MesosTest sets it to 1024) being allocated by the slave. Non-zero disk resource, breaks the assumption in the test on which framework gets the which offers (e.g. framework1 gets slave3's offer instead of framework2).


Diffs
-----

  src/master/drf_sorter.cpp 33a8ec8ba0c5d4189719feb0f4b001a91910bd35 
  src/tests/allocator_tests.cpp ad3de212c6ce71d0f3dc14d6198a899a00a35bc8 
  src/tests/gc_tests.cpp 90b3a26ded877cff6eb6e29bf8139950ce237950 
  src/tests/master_tests.cpp 104610cef3fdaaea75271657c42c4fd2b6b61618 
  src/tests/utils.hpp b648b631d8c11c26e6adfa3f5b16012044557e25 

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


Testing
-------

make check


Thanks,

Vinod Kone