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 2012/09/07 23:51:34 UTC
Re: Review Request: Short term fix for ignoring executor resources during
launch
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6863/
-----------------------------------------------------------
(Updated Sept. 7, 2012, 9:51 p.m.)
Review request for mesos, Benjamin Hindman, Brian Wickman, Jie Yu, and Ben Mahler.
Changes
-------
a sneak peak before i actually test this on (currently being installed vm) linux
Description
-------
This is a short term fix.
The basic idea is that we split the launcher->run() into setup() and launch(), and assign the executor process to cgroup after the launch()
Diffs (updated)
-----
src/launcher/launcher.hpp 89cfa8c
src/launcher/launcher.cpp 5267ac2
src/slave/cgroups_isolation_module.cpp 8a121e0
third_party/libprocess/include/stout/os.hpp df0f7ff
Diff: https://reviews.apache.org/r/6863/diff/
Testing
-------
make check succeeds on linux
but, the cgroups tests seemed to be disabled on my nest machine. are there any flags that i should use? i suspect the reason for the disabled tests on linux is because i run bootstrap on my mac and build the bootstrapped branch on nest.
Thanks,
Vinod Kone
Re: Review Request: Short term fix for ignoring executor resources during
launch
Posted by Vinod Kone <vi...@gmail.com>.
> On Sept. 9, 2012, 11:01 p.m., Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 84
> > <https://reviews.apache.org/r/6863/diff/2/?file=151318#file151318line84>
> >
> > This now kills the slave (for the cgroups module) in the event something weird is going on. I agree that not being able to chdir is a rather serious issue but I'd rather not see us killing the slave in this circumstance (i.e., return bool/int here).
good call, fixed.
> On Sept. 9, 2012, 11:01 p.m., Benjamin Hindman wrote:
> > src/launcher/launcher.cpp, line 87
> > <https://reviews.apache.org/r/6863/diff/2/?file=151318#file151318line87>
> >
> > My guess is this has some fatalerror issues as well? This one is loads of serious because a framework might have just given us a bad executor URI and we don't want to kill a slave for that.
done
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6863/#review11222
-----------------------------------------------------------
On Sept. 7, 2012, 9:51 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6863/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2012, 9:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brian Wickman, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> This is a short term fix.
>
> The basic idea is that we split the launcher->run() into setup() and launch(), and assign the executor process to cgroup after the launch()
>
>
> Diffs
> -----
>
> src/launcher/launcher.hpp 89cfa8c
> src/launcher/launcher.cpp 5267ac2
> src/slave/cgroups_isolation_module.cpp 8a121e0
> third_party/libprocess/include/stout/os.hpp df0f7ff
>
> Diff: https://reviews.apache.org/r/6863/diff/
>
>
> Testing
> -------
>
> make check succeeds on linux
>
> but, the cgroups tests seemed to be disabled on my nest machine. are there any flags that i should use? i suspect the reason for the disabled tests on linux is because i run bootstrap on my mac and build the bootstrapped branch on nest.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Short term fix for ignoring executor resources during
launch
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6863/#review11222
-----------------------------------------------------------
src/launcher/launcher.cpp
<https://reviews.apache.org/r/6863/#comment24153>
Not your bug, but please add a period at the end of comments you come across that don't have them. Thanks!
src/launcher/launcher.cpp
<https://reviews.apache.org/r/6863/#comment24154>
This now kills the slave (for the cgroups module) in the event something weird is going on. I agree that not being able to chdir is a rather serious issue but I'd rather not see us killing the slave in this circumstance (i.e., return bool/int here).
src/launcher/launcher.cpp
<https://reviews.apache.org/r/6863/#comment24157>
My guess is this has some fatalerror issues as well? This one is loads of serious because a framework might have just given us a bad executor URI and we don't want to kill a slave for that.
src/launcher/launcher.cpp
<https://reviews.apache.org/r/6863/#comment24155>
Ditto above.
src/launcher/launcher.cpp
<https://reviews.apache.org/r/6863/#comment24156>
Likewise, now this could kill the slave. Do me a favor and just kill initializeWorkingDirectory all together and inline this code into setup. Again, not your bug, but thanks!
- Benjamin Hindman
On Sept. 7, 2012, 9:51 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6863/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2012, 9:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brian Wickman, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> This is a short term fix.
>
> The basic idea is that we split the launcher->run() into setup() and launch(), and assign the executor process to cgroup after the launch()
>
>
> Diffs
> -----
>
> src/launcher/launcher.hpp 89cfa8c
> src/launcher/launcher.cpp 5267ac2
> src/slave/cgroups_isolation_module.cpp 8a121e0
> third_party/libprocess/include/stout/os.hpp df0f7ff
>
> Diff: https://reviews.apache.org/r/6863/diff/
>
>
> Testing
> -------
>
> make check succeeds on linux
>
> but, the cgroups tests seemed to be disabled on my nest machine. are there any flags that i should use? i suspect the reason for the disabled tests on linux is because i run bootstrap on my mac and build the bootstrapped branch on nest.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Short term fix for ignoring executor resources during
launch
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6863/#review11307
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Sept. 10, 2012, 9:39 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6863/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2012, 9:39 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brian Wickman, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> This is a short term fix.
>
> The basic idea is that we split the launcher->run() into setup() and launch(), and assign the executor process to cgroup after the launch()
>
>
> Diffs
> -----
>
> src/launcher/launcher.hpp 89cfa8c
> src/launcher/launcher.cpp 5267ac2
> src/slave/cgroups_isolation_module.cpp 8a121e0
> third_party/libprocess/include/stout/os.hpp 602db1f
>
> Diff: https://reviews.apache.org/r/6863/diff/
>
>
> Testing
> -------
>
> make check succeeds on linux
>
> but, the cgroups tests seemed to be disabled on my nest machine. are there any flags that i should use? i suspect the reason for the disabled tests on linux is because i run bootstrap on my mac and build the bootstrapped branch on nest.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Short term fix for ignoring executor resources during
launch
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6863/
-----------------------------------------------------------
(Updated Sept. 10, 2012, 9:39 p.m.)
Review request for mesos, Benjamin Hindman, Brian Wickman, Jie Yu, and Ben Mahler.
Changes
-------
Ben's comments.
diff'ed against apache/trunk
Description
-------
This is a short term fix.
The basic idea is that we split the launcher->run() into setup() and launch(), and assign the executor process to cgroup after the launch()
Diffs (updated)
-----
src/launcher/launcher.hpp 89cfa8c
src/launcher/launcher.cpp 5267ac2
src/slave/cgroups_isolation_module.cpp 8a121e0
third_party/libprocess/include/stout/os.hpp 602db1f
Diff: https://reviews.apache.org/r/6863/diff/
Testing
-------
make check succeeds on linux
but, the cgroups tests seemed to be disabled on my nest machine. are there any flags that i should use? i suspect the reason for the disabled tests on linux is because i run bootstrap on my mac and build the bootstrapped branch on nest.
Thanks,
Vinod Kone