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/07/25 23:11:31 UTC
Review Request: FD_CLOEXEC the pipes in slave
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs
-----
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9479
-----------------------------------------------------------
This seems to be conflict with these two pending reviews:
https://reviews.apache.org/r/5576/
https://reviews.apache.org/r/5621/
- Jie Yu
On July 25, 2012, 9:51 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 25, 2012, 9:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9480
-----------------------------------------------------------
Ship it!
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/6146/#comment20297>
Noting a core lib thing only: c++ allow Try<Void> would be nice here - bool is distracting since its unused.
- John Sirois
On July 25, 2012, 9:51 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 25, 2012, 9:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Vinod Kone <vi...@gmail.com>.
> On July 30, 2012, 4:19 p.m., Benjamin Hindman wrote:
> > src/exec/exec.cpp, line 58
> > <https://reviews.apache.org/r/6146/diff/3/?file=130173#file130173line58>
> >
> > { on newline.
done
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9594
-----------------------------------------------------------
On July 28, 2012, 12:46 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 28, 2012, 12:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/exec/exec.cpp d60c28d
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9594
-----------------------------------------------------------
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20462>
{ on newline.
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20463>
This isn't needed.
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20465>
Just override 'initialize' (put it in protected, and don't forget virtual keyword) and put the body of this function in it.
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20467>
Kill space before semi-colon.
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20466>
This is more than 80 characters: s/&ShutdownProcess::kill/&Self::kill/
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20468>
s/p "/p"/
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20469>
s/p "/p"/
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20470>
No need for this, just spawn the process and have it get garbaged collected automagically below.
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20471>
Just do:
spawn(new ShutdownProcess(), true);
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20472>
Same thing here: spawn(new ShutdownProcess(), true);
src/exec/exec.cpp
<https://reviews.apache.org/r/6146/#comment20473>
Kill.
src/slave/process_based_isolation_module.cpp
<https://reviews.apache.org/r/6146/#comment20474>
Not 80 characters.
src/slave/process_based_isolation_module.cpp
<https://reviews.apache.org/r/6146/#comment20475>
Ditto.
src/webui/webui.cpp
<https://reviews.apache.org/r/6146/#comment20476>
Ditto.
src/webui/webui.cpp
<https://reviews.apache.org/r/6146/#comment20477>
Ditto.
- Benjamin Hindman
On July 28, 2012, 12:46 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 28, 2012, 12:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/exec/exec.cpp d60c28d
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9609
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On July 30, 2012, 4:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 30, 2012, 4:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/exec/exec.cpp d60c28d
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave should shutdown properly on exit
Posted by Vinod Kone <vi...@gmail.com>.
> On July 30, 2012, 10:15 p.m., Jie Yu wrote:
> > third_party/libprocess/include/stout/os.hpp, line 112
> > <https://reviews.apache.org/r/6146/diff/4/?file=130805#file130805line112>
> >
> > check indent.
done
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9612
-----------------------------------------------------------
On July 30, 2012, 9:47 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 30, 2012, 9:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/exec/exec.cpp d60c28d
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave should shutdown properly on exit
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9612
-----------------------------------------------------------
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/6146/#comment20503>
check indent.
- Jie Yu
On July 30, 2012, 9:47 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 30, 2012, 9:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/exec/exec.cpp d60c28d
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Slave should shutdown properly on exit
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
(Updated July 30, 2012, 11:14 p.m.)
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Changes
-------
jie's comments.
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs (updated)
-----
src/exec/exec.cpp d60c28d
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: Slave should shutdown properly on exit
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
(Updated July 30, 2012, 9:47 p.m.)
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Changes
-------
changed title to properly reflect this review
Summary (updated)
-----------------
Slave should shutdown properly on exit
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs
-----
src/exec/exec.cpp d60c28d
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
(Updated July 30, 2012, 4:54 p.m.)
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Changes
-------
ben's comments
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs (updated)
-----
src/exec/exec.cpp d60c28d
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
(Updated July 28, 2012, 12:46 a.m.)
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Changes
-------
Forcefully shutting down executor, incase the driver is blocked on shutdown(), for whatever reason.
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs (updated)
-----
src/exec/exec.cpp d60c28d
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/
-----------------------------------------------------------
(Updated July 25, 2012, 9:51 p.m.)
Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
Changes
-------
s/doig/setting/g
Description
-------
See summary
This addresses bug MESOS-244.
https://issues.apache.org/jira/browse/MESOS-244
Diffs (updated)
-----
src/slave/process_based_isolation_module.cpp b54f91c
src/webui/webui.cpp 9453002
third_party/libprocess/include/stout/os.hpp 1186d24
Diff: https://reviews.apache.org/r/6146/diff/
Testing
-------
Need more specific tests.
But 'make check' succeeds.
Thanks,
Vinod Kone
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Vinod Kone <vi...@gmail.com>.
> On July 25, 2012, 9:13 p.m., Benjamin Hindman wrote:
> > src/slave/process_based_isolation_module.cpp, line 117
> > <https://reviews.apache.org/r/6146/diff/1/?file=129090#file129090line117>
> >
> > s/doig/setting/
done
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9475
-----------------------------------------------------------
On July 25, 2012, 9:11 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 25, 2012, 9:11 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: FD_CLOEXEC the pipes in slave
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6146/#review9475
-----------------------------------------------------------
Ship it!
src/slave/process_based_isolation_module.cpp
<https://reviews.apache.org/r/6146/#comment20292>
s/doig/setting/
- Benjamin Hindman
On July 25, 2012, 9:11 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6146/
> -----------------------------------------------------------
>
> (Updated July 25, 2012, 9:11 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
>
>
> Description
> -------
>
> See summary
>
>
> This addresses bug MESOS-244.
> https://issues.apache.org/jira/browse/MESOS-244
>
>
> Diffs
> -----
>
> src/slave/process_based_isolation_module.cpp b54f91c
> src/webui/webui.cpp 9453002
> third_party/libprocess/include/stout/os.hpp 1186d24
>
> Diff: https://reviews.apache.org/r/6146/diff/
>
>
> Testing
> -------
>
> Need more specific tests.
>
> But 'make check' succeeds.
>
>
> Thanks,
>
> Vinod Kone
>
>