Re: Review Request 43582: Added flag to disable system support.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119236
---




src/slave/main.cpp (line 234)


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 system support.

[Joerg Schad]

> On Feb. 15, 2016, 6:35 p.m., Joerg Schad wrote:
> > src/slave/flags.hpp, line 97
> > 
> >
> > 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.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119234
---




src/linux/systemd.cpp (line 52)


does it make sense to have the same order here as above in the hpp?



src/slave/flags.hpp (line 97)


Should we document this in configure.md?



src/slave/flags.cpp (line 399)


What precisly do you mean by `Top level control systemd support`?



src/slave/main.cpp (line 243)


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.

[Benjamin Hindman]

---
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)


"... are enabled unless there is an explicit flag to disable these (see 
other flags)."



src/slave/flags.cpp (line 400)


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.

[Joris Van Remoortere]

---
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.

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43582/#review119229
---




src/slave/flags.cpp (lines 397 - 403)


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
> 
>

Review Request 43582: Added flag to disable system support.

[Joris Van Remoortere]

---
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