You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/06/09 06:15:51 UTC
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/
-----------------------------------------------------------
(Updated June 8, 2014, 9:15 p.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Rebased.
Repository: mesos-git
Description
-------
Use a timeout to discard cgroups::destroy.
Diffs (updated)
-----
src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
Diff: https://reviews.apache.org/r/22064/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45947
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On June 16, 2014, 9:58 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 9:58 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp eb8933f4e37e09e52123b03890b6df952ceb6ec8
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 73b926ffb682b96c9ef3c6b4e712e8058b2a4f91
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45944
-----------------------------------------------------------
Bad patch!
Reviews applied: [20817, 20818, 22064]
Failed command: ./support/mesos-style.py
Error:
Checking 475 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/slave/containerizer/linux_launcher.cpp:281: Redundant blank line at the end of a code block should be deleted. [whitespace/blank_line] [3]
Total errors found: 1
- Mesos ReviewBot
On June 16, 2014, 9:58 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 9:58 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp eb8933f4e37e09e52123b03890b6df952ceb6ec8
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 73b926ffb682b96c9ef3c6b4e712e8058b2a4f91
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/
-----------------------------------------------------------
(Updated June 16, 2014, 2:58 p.m.)
Review request for mesos and Jie Yu.
Repository: mesos-git
Description
-------
Use a timeout to discard cgroups::destroy.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
src/slave/containerizer/isolators/cgroups/cpushare.cpp eb8933f4e37e09e52123b03890b6df952ceb6ec8
src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
src/slave/containerizer/isolators/cgroups/mem.cpp 73b926ffb682b96c9ef3c6b4e712e8058b2a4f91
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
Diff: https://reviews.apache.org/r/22064/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45553
-----------------------------------------------------------
Bad patch!
Reviews applied: [22064]
Failed command: git apply --index 22064.patch
Error:
error: patch failed: src/linux/cgroups.hpp:40
error: src/linux/cgroups.hpp: patch does not apply
error: patch failed: src/linux/cgroups.cpp:1670
error: src/linux/cgroups.cpp: patch does not apply
error: patch failed: src/slave/containerizer/isolators/cgroups/cpushare.cpp:439
error: src/slave/containerizer/isolators/cgroups/cpushare.cpp: patch does not apply
error: patch failed: src/slave/containerizer/linux_launcher.cpp:271
error: src/slave/containerizer/linux_launcher.cpp: patch does not apply
- Mesos ReviewBot
On June 11, 2014, 11:31 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 11:31 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/
-----------------------------------------------------------
(Updated June 11, 2014, 11:31 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
i'll let @jie shepherd this -- vinod.
Repository: mesos-git
Description
-------
Use a timeout to discard cgroups::destroy.
Diffs
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
Diff: https://reviews.apache.org/r/22064/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Ian Downes <ia...@gmail.com>.
> On June 11, 2014, 3:39 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, lines 128-131
> > <https://reviews.apache.org/r/22064/diff/3/?file=607780#file607780line128>
> >
> > In fact, we just discovered an issue which will occur when an isolator cleans up an orphaned container before processes inside it are killed. (In our case, it causes tcp connection being leaked.)
> >
> > So I think we should do a block wait here and set a timeout. If we cannot finish it before timeout, we probably want to return error?
This will be fixed separately: https://issues.apache.org/jira/browse/MESOS-1488
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45440
-----------------------------------------------------------
On June 11, 2014, 4:31 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 4:31 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45440
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/cpushare.cpp
<https://reviews.apache.org/r/22064/#comment80305>
This should be 'onAny'?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/22064/#comment80304>
In fact, we just discovered an issue which will occur when an isolator cleans up an orphaned container before processes inside it are killed. (In our case, it causes tcp connection being leaked.)
So I think we should do a block wait here and set a timeout. If we cannot finish it before timeout, we probably want to return error?
- Jie Yu
On June 11, 2014, 10:21 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 10:21 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
> src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/
-----------------------------------------------------------
(Updated June 11, 2014, 3:21 p.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Reworked destroy timeout as per Jie's comment.
Repository: mesos-git
Description
-------
Use a timeout to discard cgroups::destroy.
Diffs (updated)
-----
src/linux/cgroups.hpp 21d87a0783c2edd653d28fa89c59773200ae647e
src/linux/cgroups.cpp 142ac437d6d53b678ef284bda46444e1615ff0d1
src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
Diff: https://reviews.apache.org/r/22064/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45396
-----------------------------------------------------------
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/22064/#comment80231>
Ditto BenM's comments. I think having a destroyTimedout in the library is a bit strange. I guess the reason is because you have a lot of duplication here because the action after destroy timedout is the same.
If that's the case, I would rather add an overload for cgroups::destory which takes a timeout value, and invoke the existing non-timeout version of the cgroups::destroy. (Although we try to avoid having timeout in library as much as possible).
What do you think?
- Jie Yu
On June 11, 2014, 5:27 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 5:27 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/
-----------------------------------------------------------
(Updated June 11, 2014, 10:27 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Added depends on.
Repository: mesos-git
Description
-------
Use a timeout to discard cgroups::destroy.
Diffs
-----
src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
Diff: https://reviews.apache.org/r/22064/diff/
Testing
-------
Thanks,
Ian Downes
Re: Review Request 22064: Use a timeout to discard cgroups::destroy.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22064/#review45057
-----------------------------------------------------------
Bad patch!
Reviews applied: [22064]
Failed command: git apply --index 22064.patch
Error:
error: patch failed: src/slave/containerizer/isolators/cgroups/cpushare.cpp:439
error: src/slave/containerizer/isolators/cgroups/cpushare.cpp: patch does not apply
error: patch failed: src/slave/containerizer/linux_launcher.cpp:271
error: src/slave/containerizer/linux_launcher.cpp: patch does not apply
- Mesos ReviewBot
On June 9, 2014, 4:15 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22064/
> -----------------------------------------------------------
>
> (Updated June 9, 2014, 4:15 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use a timeout to discard cgroups::destroy.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 909ea8802b3746b73aae8d62c8e49f259c471fd5
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3d253af51677dcb4dc48dc9e01bdc2ba80847da9
> src/slave/containerizer/isolators/cgroups/mem.hpp 362ebcfa2e16701b225deea0fbeb92e4a56d51aa
> src/slave/containerizer/isolators/cgroups/mem.cpp 60013d4e840f6b1f131b796b95916d1978b37c70
> src/slave/containerizer/linux_launcher.cpp c17724b138de9d64856fb85db019e52043fbc7af
>
> Diff: https://reviews.apache.org/r/22064/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>