Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-05 Thread Jie Yu 

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

Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

This patch added the support for systemd hierarchy in LinuxLauncher.
It created the same cgroup layout under the systemd hierarchy (if
systemd is enabled) as that in the freezer hierarchy.

This can give us a bunch of benefits:
1) systemd-cgls can list mesos container processes.
2) systemd-cgtop can show stats for mesos containers.
3) Avoid the pid migration issue described in MESOS-3352.

For example:

```
[jie@core-dev ~]$ systemd-cgls
|-1 /usr/lib/systemd/systemd --system --deserialize 20
|-mesos
|  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
|  |-31737 /usr/libexec/mesos/mesos-containerizer launch
|  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
|  |-mesos
| |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
|   |-31791 /usr/libexec/mesos/mesos-containerizer launch
|   |-31793 sleep 1000
```


Diffs
-

  src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
  src/slave/containerizer/mesos/linux_launcher.cpp 
554c598e2a7a53aede9d8761740d8efceb4a7e39 


Diff: https://reviews.apache.org/r/62800/diff/1/


Testing
---

sudo make check

I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
issue. Didn't observe it after applying the patch.


Thanks,

Jie Yu

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 62800 was successfully built and tested.

Reviews applied: `['62798', '62799', '62800']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62800

- Mesos Reviewbot Windows


On Oct. 6, 2017, 4:33 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 4:33 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 62800 was successfully built and tested.

Reviews applied: `['62798', '62799', '62800']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62800

- Mesos Reviewbot Windows


On Oct. 6, 2017, 4:33 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 4:33 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-06 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [62798, 62799, 62800]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Oct. 6, 2017, 6:33 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 6:33 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-06 Thread Jie Yu 

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

(Updated Oct. 6, 2017, 10:04 p.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description (updated)
---

This patch added the support for systemd hierarchy in LinuxLauncher.
It created the same cgroup layout under the systemd hierarchy (if
systemd is enabled) as that in the freezer hierarchy.

This can give us a bunch of benefits:
1) systemd-cgls can list mesos container processes.
2) systemd-cgtop can show stats for mesos containers.
3) Avoid the pid migration issue described in MESOS-3352.

For example:

```
|[jie@core-dev ~]$ systemd-cgls
|-1 /usr/lib/systemd/systemd --system --deserialize 20
|-mesos
|  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
| |-31737 /usr/libexec/mesos/mesos-containerizer launch
| |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
| |-mesos
||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
|   |-31791 /usr/libexec/mesos/mesos-containerizer launch
|   |-31793 sleep 1000
```


Diffs
-

  src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
  src/slave/containerizer/mesos/linux_launcher.cpp 
554c598e2a7a53aede9d8761740d8efceb4a7e39 


Diff: https://reviews.apache.org/r/62800/diff/1/


Testing
---

sudo make check

I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
issue. Didn't observe it after applying the patch.


Thanks,

Jie Yu

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-06 Thread James Peach 

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




src/linux/systemd.hpp
Lines 29 (patched)


Should be a comment here?



src/linux/systemd.hpp
Lines 30 (patched)


Do we even need this constant? It's used once and doesn't seem to add any 
value over just using `"systemd"`.



src/slave/containerizer/mesos/linux_launcher.cpp
Lines 189 (patched)


This is racy, but we can fix it by ensuring that `os::mkdir` checks for 
`EEXIST` even when it is non-recursive.



src/slave/containerizer/mesos/linux_launcher.cpp
Lines 402 (patched)


Would we really exit? I'm not sure we should expect users to be able to 
manually deal with that case.



src/slave/containerizer/mesos/linux_launcher.cpp
Lines 426 (patched)


Log the `cgroup.error()`?



src/slave/containerizer/mesos/linux_launcher.cpp
Line 369 (original), 440 (patched)


I dunno, maybe a `;` rather than a `.` since it is weird to not end with a 
`.` when we have full sentences :)


- James Peach


On Oct. 6, 2017, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 10:04 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> |[jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> | |-31737 /usr/libexec/mesos/mesos-containerizer launch
> | |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> | |-mesos
> ||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-07 Thread Jie Yu 


> On Oct. 7, 2017, 12:23 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp
> > Lines 189 (patched)
> > 
> >
> > This is racy, but we can fix it by ensuring that `os::mkdir` checks for 
> > `EEXIST` even when it is non-recursive.

This is under mesos cgroup root, i don't race is a big concern. I've been 
following what we did in cgroups isolator. I'd prefer consistency here.


- Jie


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


On Oct. 6, 2017, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 10:04 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> |[jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> | |-31737 /usr/libexec/mesos/mesos-containerizer launch
> | |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> | |-mesos
> ||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-07 Thread Jie Yu 


> On Oct. 7, 2017, 12:23 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp
> > Lines 402 (patched)
> > 
> >
> > Would we really exit? I'm not sure we should expect users to be able to 
> > manually deal with that case.

This is not a TODO from me:) I'd keep it here and probably follow up with a 
different patch.


- Jie


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


On Oct. 6, 2017, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 10:04 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> |[jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> | |-31737 /usr/libexec/mesos/mesos-containerizer launch
> | |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> | |-mesos
> ||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-07 Thread Jie Yu 

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

(Updated Feb. 7, 2018, 8:01 p.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

This patch added the support for systemd hierarchy in LinuxLauncher.
It created the same cgroup layout under the systemd hierarchy (if
systemd is enabled) as that in the freezer hierarchy.

This can give us a bunch of benefits:
1) systemd-cgls can list mesos container processes.
2) systemd-cgtop can show stats for mesos containers.
3) Avoid the pid migration issue described in MESOS-3352.

For example:

```
[jie@core-dev ~]$ systemd-cgls
|-1 /usr/lib/systemd/systemd --system --deserialize 20
|-mesos
|  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
|  |-31737 /usr/libexec/mesos/mesos-containerizer launch
|  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
|  |-mesos
| |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
|   |-31791 /usr/libexec/mesos/mesos-containerizer launch
|   |-31793 sleep 1000
```


Diffs (updated)
-

  src/slave/containerizer/mesos/linux_launcher.cpp 
c2e319894874e628beda6dc305462af0c274fd7b 


Diff: https://reviews.apache.org/r/62800/diff/2/

Changes: https://reviews.apache.org/r/62800/diff/1-2/


Testing
---

sudo make check

I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
issue. Didn't observe it after applying the patch.


Thanks,

Jie Yu

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-07 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 62800 was successfully built and tested.

Reviews applied: `['62798', '62799', '62800']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62800

- Mesos Reviewbot Windows


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e319894874e628beda6dc305462af0c274fd7b 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-08 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [62798, 62799, 62800]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e319894874e628beda6dc305462af0c274fd7b 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2018-02-09 Thread James Peach 

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


Ship it!




Ship It!

- James Peach


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e319894874e628beda6dc305462af0c274fd7b 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>