You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2016/02/15 18:37:52 UTC
Review Request 43582: Added flag to disable system support.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-4675
https://issues.apache.org/jira/browse/MESOS-4675
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
Diff: https://reviews.apache.org/r/43582/diff/
Testing
-------
run on systemd system with flag disabled.
Thanks,
Joris Van Remoortere
Re: Review Request 43582: Added flag to disable system support.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119236
-----------------------------------------------------------
src/slave/main.cpp (line 234)
<https://reviews.apache.org/r/43582/#comment180517>
Regarding the summary:
s/`to disable system support.`/`to disable systemd support`?
- Joerg Schad
On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 5:59 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added flag to disable system support.
>
>
> Diffs
> -----
>
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 43582: Added flag to disable systemd support.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119237
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43582]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 15, 2016, 6:55 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 6:55 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added flag to disable systemd support.
>
>
> Diffs
> -----
>
> docs/configuration.md eea985cc251d6edd8476ade174b10a58aebefab1
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 43582: Added flag to disable systemd support.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/
-----------------------------------------------------------
(Updated Feb. 15, 2016, 6:55 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
re-ordering flags.
Summary (updated)
-----------------
Added flag to disable systemd support.
Bugs: MESOS-4675
https://issues.apache.org/jira/browse/MESOS-4675
Repository: mesos
Description (updated)
-------
Added flag to disable systemd support.
Diffs (updated)
-----
docs/configuration.md eea985cc251d6edd8476ade174b10a58aebefab1
src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
Diff: https://reviews.apache.org/r/43582/diff/
Testing
-------
run on systemd system with flag disabled.
Thanks,
Joris Van Remoortere
Re: Review Request 43582: Added flag to disable system support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119231
-----------------------------------------------------------
Fix it, then Ship it!
src/linux/systemd.cpp (line 55)
<https://reviews.apache.org/r/43582/#comment180505>
"... are enabled unless there is an explicit flag to disable these (see other flags)."
src/slave/flags.cpp (line 400)
<https://reviews.apache.org/r/43582/#comment180506>
See comment above.
- Benjamin Hindman
On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 5:59 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added flag to disable system support.
>
>
> Diffs
> -----
>
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 43582: Added flag to disable system support.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On Feb. 15, 2016, 6:35 p.m., Joerg Schad wrote:
> > src/slave/flags.hpp, line 97
> > <https://reviews.apache.org/r/43582/diff/2/?file=1241721#file1241721line97>
> >
> > Should we document this in configure.md?
Or are we generating this automatically after https://reviews.apache.org/r/42939/? In that case could you point me to the script for the generation?
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119234
-----------------------------------------------------------
On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 5:59 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added flag to disable system support.
>
>
> Diffs
> -----
>
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 43582: Added flag to disable system support.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119234
-----------------------------------------------------------
src/linux/systemd.cpp (line 52)
<https://reviews.apache.org/r/43582/#comment180514>
does it make sense to have the same order here as above in the hpp?
src/slave/flags.hpp (line 97)
<https://reviews.apache.org/r/43582/#comment180512>
Should we document this in configure.md?
src/slave/flags.cpp (line 399)
<https://reviews.apache.org/r/43582/#comment180513>
What precisly do you mean by `Top level control systemd support`?
src/slave/main.cpp (line 243)
<https://reviews.apache.org/r/43582/#comment180515>
Isn't flags.systemd_enable_support always true at this point? Feel free to drop, but I personally feel `true` would be easier to read.
- Joerg Schad
On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 5:59 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added flag to disable system support.
>
>
> Diffs
> -----
>
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 43582: Added flag to disable system support.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/
-----------------------------------------------------------
(Updated Feb. 15, 2016, 5:59 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
More generic help strings.
Bugs: MESOS-4675
https://issues.apache.org/jira/browse/MESOS-4675
Repository: mesos
Description (updated)
-------
Added flag to disable system support.
Diffs (updated)
-----
src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
Diff: https://reviews.apache.org/r/43582/diff/
Testing
-------
run on systemd system with flag disabled.
Thanks,
Joris Van Remoortere
Re: Review Request 43582: Added flag to disable system support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119229
-----------------------------------------------------------
src/slave/flags.cpp (lines 397 - 403)
<https://reviews.apache.org/r/43582/#comment180503>
My $0.02: this flag makes it sound like we are "enabling systemd" not "enabling moving executors and associated processes into a systemd slice". IMHO this could mean two different things in the future, i.e., we want to enable systemd for one thing but not for moving executors? I'd call out the moving executors support explicitly in this flag, perhaps: `systemd_enable_executors_slice` or a better name if you decided previously to not call it an executor slice any more because it holds more than just executors.
- Benjamin Hindman
On Feb. 15, 2016, 5:37 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 5:37 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228
> src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d
> src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d
> src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7
> src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64
>
> Diff: https://reviews.apache.org/r/43582/diff/
>
>
> Testing
> -------
>
> run on systemd system with flag disabled.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>