You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2018/07/30 17:50:27 UTC
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/
-----------------------------------------------------------
(Updated July 30, 2018, 10:50 a.m.)
Review request for mesos, Gilbert Song and Jason Lai.
Changes
-------
Squash both diffs.
Summary (updated)
-----------------
Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.
Bugs: MESOS-8038
https://issues.apache.org/jira/browse/MESOS-8038
Repository: mesos
Description (updated)
-------
The new agent flag can be used to reconfigure how long a container
destroy is allowed to take on Mesos containerizer.
The default is also increased to 5 min based on suggestion from Gilbert
because certain containers could have deep system calls which may not
finish within previous 1 min timeout.
Diffs (updated)
-----
src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
Diff: https://reviews.apache.org/r/68088/diff/2/
Changes: https://reviews.apache.org/r/68088/diff/1-2/
Testing
-------
`make` and `./bin/mesos-slave.sh --help`
Thanks,
Zhitao Li
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Aug. 1, 2018, 8:40 p.m., James Peach wrote:
> > I don't have a good sense of whether this is going to fix the underlying problem, but making it a config option is definitely a benefit and the code looks fine.
I'm also puzzled about how making something configurable will fix this problem. My understanding is that one way MESOS-8038 occurs is via destroy timeout leading to a destroy failure, making the timeout configurable doesn't fix the failure handling?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/#review206755
-----------------------------------------------------------
On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> -----------------------------------------------------------
>
> (Updated July 30, 2018, 5:50 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and James Peach.
>
>
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
>
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
> src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
> src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
> src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
>
>
> Diff: https://reviews.apache.org/r/68088/diff/2/
>
>
> Testing
> -------
>
> `make` and `./bin/mesos-slave.sh --help`
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/#review206755
-----------------------------------------------------------
Fix it, then Ship it!
I don't have a good sense of whether this is going to fix the underlying problem, but making it a config option is definitely a benefit and the code looks fine.
src/slave/flags.cpp
Lines 541 (patched)
<https://reviews.apache.org/r/68088/#comment289844>
Please also document this in `docs/configuration/agent.md`.
- James Peach
On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> -----------------------------------------------------------
>
> (Updated July 30, 2018, 5:50 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jason Lai.
>
>
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
>
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
> src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
> src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
> src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
>
>
> Diff: https://reviews.apache.org/r/68088/diff/2/
>
>
> Testing
> -------
>
> `make` and `./bin/mesos-slave.sh --help`
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by Jason Lai <ja...@jasonlai.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/#review206604
-----------------------------------------------------------
Ship it!
Ship It!
- Jason Lai
On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> -----------------------------------------------------------
>
> (Updated July 30, 2018, 5:50 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jason Lai.
>
>
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
>
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
> src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
> src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
> src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
>
>
> Diff: https://reviews.apache.org/r/68088/diff/2/
>
>
> Testing
> -------
>
> `make` and `./bin/mesos-slave.sh --help`
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/#review206608
-----------------------------------------------------------
PASS: Mesos patch 68088 was successfully built and tested.
Reviews applied: `['68088']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2002/mesos-review-68088
- Mesos Reviewbot Windows
On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> -----------------------------------------------------------
>
> (Updated July 30, 2018, 5:50 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jason Lai.
>
>
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
>
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
> src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
> src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
> src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
>
>
> Diff: https://reviews.apache.org/r/68088/diff/2/
>
>
> Testing
> -------
>
> `make` and `./bin/mesos-slave.sh --help`
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/#review207052
-----------------------------------------------------------
Fix it, then Ship it!
Please modify the agent.md
src/slave/flags.cpp
Lines 541 (patched)
<https://reviews.apache.org/r/68088/#comment290237>
Hey Zhitao, this patch looks good to me. I was thinking about what is the better default semantic: hanging forever VS extend the timeout. I think for now, Let's do not change the default behavior and set the default value as 60 seconds. You guys could configure the flag to be longer (I would suggest forever).
- Gilbert Song
On July 30, 2018, 10:50 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> -----------------------------------------------------------
>
> (Updated July 30, 2018, 10:50 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and James Peach.
>
>
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
>
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed
> src/slave/containerizer/mesos/linux_launcher.cpp 3bddcece7028745cec6623ac33dbfcaced629629
> src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
> src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
>
>
> Diff: https://reviews.apache.org/r/68088/diff/2/
>
>
> Testing
> -------
>
> `make` and `./bin/mesos-slave.sh --help`
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with
new agent flag.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68088/
-----------------------------------------------------------
(Updated Aug. 10, 2018, 10:36 a.m.)
Review request for mesos, Gilbert Song, Jason Lai, and James Peach.
Bugs: MESOS-8038
https://issues.apache.org/jira/browse/MESOS-8038
Repository: mesos
Description (updated)
-------
The new agent flag can be used to reconfigure how long a container
destroy is allowed to take on Mesos containerizer.
Diffs (updated)
-----
src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp e016d6dcff15bf49b003c9f3ca94477ce313f7b6
src/slave/containerizer/mesos/linux_launcher.cpp cd677cc652dc53ea3bf9a715b415c3e5c48c1f89
src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
Diff: https://reviews.apache.org/r/68088/diff/3/
Changes: https://reviews.apache.org/r/68088/diff/2-3/
Testing
-------
`make` and `./bin/mesos-slave.sh --help`
Thanks,
Zhitao Li