You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/04/03 01:38:55 UTC

Review Request 32798: Add test to verify executor clean up in docker containerizer.

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

Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


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


Repository: mesos


Description
-------

Add test to verify executor clean up in docker containerizer.


Diffs
-----

  src/tests/docker_containerizer_tests.cpp fdd706a892ee1c8d55a406b3f956d99c076c623b 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 32798: Add test to verify executor clean up in docker containerizer.

Posted by Timothy Chen <tn...@apache.org>.


> On April 3, 2015, 3:38 p.m., Benjamin Hindman wrote:
> > src/tests/docker_containerizer_tests.cpp, lines 2625-2627
> > <https://reviews.apache.org/r/32798/diff/1/?file=914222#file914222line2625>
> >
> >     My only question here is how do you know the executor is properly killed and cleaned up? Is this because you know there aren't any more child processes? Is that something you want to check after you call Shutdown()? I.e., os::children(0).get().empty() or something like that?

Shutdown does indeed checks for remaining child processes, I've verified this by just running the test and get a exception on shutdown. I'll leave a comment in the end too.


- Timothy


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


On April 2, 2015, 11:38 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32798/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2583
>     https://issues.apache.org/jira/browse/MESOS-2583
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add test to verify executor clean up in docker containerizer.
> 
> 
> Diffs
> -----
> 
>   src/tests/docker_containerizer_tests.cpp fdd706a892ee1c8d55a406b3f956d99c076c623b 
> 
> Diff: https://reviews.apache.org/r/32798/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 32798: Add test to verify executor clean up in docker containerizer.

Posted by Benjamin Hindman <be...@berkeley.edu>.

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

Ship it!



src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/32798/#comment127812>

    My only question here is how do you know the executor is properly killed and cleaned up? Is this because you know there aren't any more child processes? Is that something you want to check after you call Shutdown()? I.e., os::children(0).get().empty() or something like that?


- Benjamin Hindman


On April 2, 2015, 11:38 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32798/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2583
>     https://issues.apache.org/jira/browse/MESOS-2583
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add test to verify executor clean up in docker containerizer.
> 
> 
> Diffs
> -----
> 
>   src/tests/docker_containerizer_tests.cpp fdd706a892ee1c8d55a406b3f956d99c076c623b 
> 
> Diff: https://reviews.apache.org/r/32798/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 32798: Add test to verify executor clean up in docker containerizer.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

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


Patch looks great!

Reviews applied: [32796, 32797, 32798]

All tests passed.

- Mesos ReviewBot


On April 2, 2015, 11:38 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32798/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2583
>     https://issues.apache.org/jira/browse/MESOS-2583
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add test to verify executor clean up in docker containerizer.
> 
> 
> Diffs
> -----
> 
>   src/tests/docker_containerizer_tests.cpp fdd706a892ee1c8d55a406b3f956d99c076c623b 
> 
> Diff: https://reviews.apache.org/r/32798/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>