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 2015/09/22 19:29:22 UTC

Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Sept. 23, 2015, 8:52 a.m., Timothy Chen wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 117
> > <https://reviews.apache.org/r/38635/diff/2/?file=1081893#file1081893line117>
> >
> >     How about we do what libprocess does where we initialize on each system:: call, and you don't have to initialize explicitly

The one in libprocess doesn't take arguments. If we didn't initialize in one place then we'd have to carry the flags around everywhere or have global access to the flags.


> On Sept. 23, 2015, 8:52 a.m., Timothy Chen wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 128
> > <https://reviews.apache.org/r/38635/diff/2/?file=1081893#file1081893line128>
> >
> >     This seems pretty systemd specific, why not move this into the systemd.cpp?

This is a unit file specific to what we are doing in the linux launcher. If we want to change some behavior in terms of the executors slice we'll want to do it here.
All the generic helper functions are in a separate systemd namespace though :-)


- Joris


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


On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100171
-----------------------------------------------------------



src/linux/systemd.cpp (line 161)
<https://reviews.apache.org/r/38635/#comment157334>

    Why not use os::write?



src/slave/containerizer/linux_launcher.hpp (line 28)
<https://reviews.apache.org/r/38635/#comment157332>

    What do you mean by detect these?



src/slave/containerizer/linux_launcher.cpp (line 117)
<https://reviews.apache.org/r/38635/#comment157333>

    How about we do what libprocess does where we initialize on each system:: call, and you don't have to initialize explicitly



src/slave/containerizer/linux_launcher.cpp (line 128)
<https://reviews.apache.org/r/38635/#comment157335>

    This seems pretty systemd specific, why not move this into the systemd.cpp?



src/slave/flags.cpp (line 313)
<https://reviews.apache.org/r/38635/#comment157331>

    systemd system run time


- Timothy Chen


On Sept. 23, 2015, 1:48 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100290
-----------------------------------------------------------


Just some feedback on doxygen usage; trying to avoid comment bloat and redundancy.

Looking over our doxygen comment style, it looks like we may want to altogether avoid the use of @param, @return in favor just converting our existing `//` paragraph style comments into `/** */` comments so that they get picked up by doxygen. That probably would have been a better first step for doxygen usage in retrospect IMHO.


src/linux/systemd.hpp (lines 52 - 65)
<https://reviews.apache.org/r/38635/#comment157448>

    Ditto w.r.t. redundant documentation across top and @return lines.



src/linux/systemd.hpp (lines 79 - 83)
<https://reviews.apache.org/r/38635/#comment157447>

    These lines look redundant, can you remove the @return?



src/linux/systemd.hpp (line 93)
<https://reviews.apache.org/r/38635/#comment157446>

    Do we really need to say this? This is standard for Try.



src/linux/systemd.hpp (lines 98 - 106)
<https://reviews.apache.org/r/38635/#comment157445>

    This kind of comment only tells me 1 thing about the method, namely that it the way it starts the slice is through "systemctl start <name>". Everything else I can infer from the function signature. Is it worth bloating the source with 9 lines of doxygen like this? Or can we do it in 1 line?
    
    ```
    /**
      Starts the slice with the given name (via 'systemctl start <name>').
     */
    ```
    
    Also, we've been using two asterisks on the first line for it to be considered doxygen.


- Ben Mahler


On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/
-----------------------------------------------------------

(Updated Sept. 24, 2015, 3:52 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
  src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
  src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/
-----------------------------------------------------------

(Updated Sept. 24, 2015, 3:41 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
  src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
  src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100332
-----------------------------------------------------------

Ship it!



src/linux/systemd.hpp (lines 32 - 33)
<https://reviews.apache.org/r/38635/#comment157517>

    How about a systemd::Flags object?



src/linux/systemd.hpp (line 60)
<https://reviews.apache.org/r/38635/#comment157516>

    Why not just have this be `systemd::hierarchy`?



src/linux/systemd.cpp (line 44)
<https://reviews.apache.org/r/38635/#comment157518>

    How about we make this a Try<Nothing>, then we can return the error below, and do the EXIT() at the call site in LinuxLauncher where we can add to the message '... considering using --flag1 --flag2 to initialize systemd'.



src/slave/containerizer/linux_launcher.cpp (line 125)
<https://reviews.apache.org/r/38635/#comment157519>

    ... to over-ride the settings for the slice that we provide when we create the 'Unit' below.



src/slave/containerizer/linux_launcher.cpp (line 137)
<https://reviews.apache.org/r/38635/#comment157520>

    Let's just return 'Failure' here (and all the ones below) rather than aborting.



src/slave/containerizer/linux_launcher.cpp (line 144)
<https://reviews.apache.org/r/38635/#comment157521>

    s|service|slice/service| maybe?


- Benjamin Hindman


On Sept. 24, 2015, 12:12 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
>   src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/
-----------------------------------------------------------

(Updated Sept. 24, 2015, 12:12 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
  src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
  src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Jie Yu <yu...@gmail.com>.

> On Sept. 23, 2015, 10:31 p.m., Kapil Arya wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 133
> > <https://reviews.apache.org/r/38635/diff/3/?file=1083336#file1083336line133>
> >
> >     Do we want to ABORT, or return an Error here (and below)? Not sure, if we have a policy that I missed.
> 
> Joris Van Remoortere wrote:
>     We've chosen to abort / exit under these paths because we don't want to silently (or only with a warning) run in a bad state.

+1 on return Error.

We won't run into a bad state because MesosContainerizer::create will fail and slave will abort anyway. We usually return Error in those initialization functions.


- Jie


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


On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Sept. 23, 2015, 10:31 p.m., Kapil Arya wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 133
> > <https://reviews.apache.org/r/38635/diff/3/?file=1083336#file1083336line133>
> >
> >     Do we want to ABORT, or return an Error here (and below)? Not sure, if we have a policy that I missed.

We've chosen to abort / exit under these paths because we don't want to silently (or only with a warning) run in a bad state.


- Joris


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


On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100306
-----------------------------------------------------------



src/slave/containerizer/linux_launcher.cpp (line 133)
<https://reviews.apache.org/r/38635/#comment157480>

    Do we want to ABORT, or return an Error here (and below)? Not sure, if we have a policy that I missed.



src/slave/flags.cpp (line 311)
<https://reviews.apache.org/r/38635/#comment157478>

    Newline?


- Kapil Arya


On Sept. 23, 2015, 4:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100317
-----------------------------------------------------------



src/linux/systemd.hpp (line 34)
<https://reviews.apache.org/r/38635/#comment157495>

    That's a weired dependency. Can you pass in systemd_runtime_directory directly:
    
    ```
    void initialize(const string& runtimeDirectory);
    ```



src/linux/systemd.cpp (lines 167 - 180)
<https://reviews.apache.org/r/38635/#comment157500>

    Use os::write?



src/slave/containerizer/linux_launcher.hpp (lines 28 - 30)
<https://reviews.apache.org/r/38635/#comment157501>

    We try to avoid non-POD global variables. You can use:
    
    ```
    static const char SYSTEMD_MESOS_EXECUTORS_SLICE[] = "mesos_executors.slice";
    ```



src/slave/containerizer/linux_launcher.cpp (lines 123 - 124)
<https://reviews.apache.org/r/38635/#comment157498>

    The follow is less jagged.
    
    ```
    const Path path(path::join(
        systemd::runtimeDirectory(),
        SYSTEMD_MESOS_EXECUTORS_SLICE));
    ```



src/slave/containerizer/linux_launcher.cpp (lines 151 - 154)
<https://reviews.apache.org/r/38635/#comment157499>

    Style nits:
    
    ```
    Try<bool> exists = cgroups::exists(
        systemd::nameHierarchy(),
        SYSTEMD_MESOS_EXECUTORS_SLICE);
    ```


- Jie Yu


On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/
-----------------------------------------------------------

(Updated Sept. 23, 2015, 8:34 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
-------

rebased


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/
-----------------------------------------------------------

(Updated Sept. 23, 2015, 1:48 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
-------

Refactored based on BenH's comments.


Bugs: MESOS-3425
    https://issues.apache.org/jira/browse/MESOS-3425


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 

Diff: https://reviews.apache.org/r/38635/diff/


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Sept. 23, 2015, 12:48 a.m., Niklas Nielsen wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 197
> > <https://reviews.apache.org/r/38635/diff/1/?file=1081421#file1081421line197>
> >
> >     Wouldn't the idomatic way be to use stout's file abstractions?

Which ones are you referring to specifically? This is the pattern used in the cgroups files.
I did abstract some of this away into a systemd namespace. Please see my upcoming reviews.


- Joris


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


On Sept. 22, 2015, 5:29 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 5:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38635/#review100109
-----------------------------------------------------------



src/slave/containerizer/linux_launcher.cpp (line 197)
<https://reviews.apache.org/r/38635/#comment157225>

    Wouldn't the idomatic way be to use stout's file abstractions?


- Niklas Nielsen


On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
>     https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>