Re: Review Request 71931: Added the `share_cgroups` field into `LinuxInfo`.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Dec. 20, 2019, 12:38 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71931/
> ---
> 
> (Updated Dec. 20, 2019, 12:38 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10075
> https://issues.apache.org/jira/browse/MESOS-10075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the `share_cgroups` field into `LinuxInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71931/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71934: Bumped site's rack to rack-1.16.12.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71934]

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

- Mesos Reviewbot


On Dec. 20, 2019, 11:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71934/
> ---
> 
> (Updated Dec. 20, 2019, 11:51 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses CVE-2019-16782 which should not affect us.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock 87d825c4e4056c33e0702b3c429a48b01cc1b035 
> 
> 
> Diff: https://reviews.apache.org/r/71934/diff/1/
> 
> 
> Testing
> ---
> 
> Confirmed that the website still builds.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71855: Added resource limits into `TaskInfo`.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Dec. 20, 2019, 12:38 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71855/
> ---
> 
> (Updated Dec. 20, 2019, 12:38 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10043
> https://issues.apache.org/jira/browse/MESOS-10043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource limits into `TaskInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71855/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 71934: Bumped site's rack to rack-1.16.12.

[Benjamin Bannier]

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

Review request for mesos, Benno Evers and Vinod Kone.


Repository: mesos


Description
---

This addresses CVE-2019-16782 which should not affect us.


Diffs
-

  site/Gemfile.lock 87d825c4e4056c33e0702b3c429a48b01cc1b035 


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


Testing
---

Confirmed that the website still builds.


Thanks,

Benjamin Bannier

Re: Review Request 71886: WIP: Set container's `cpu.cfs_quota_us` to its CPU resource limit.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71855, 71931, 71856, 71858, 71884, 71885, 71886]

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

- Mesos Reviewbot


On Dec. 6, 2019, 8:04 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71886/
> ---
> 
> (Updated Dec. 6, 2019, 8:04 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Set container's `cpu.cfs_quota_us` to its CPU resource limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 960bd141430387e076a8fab1948d07719613ed90 
> 
> 
> Diff: https://reviews.apache.org/r/71886/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

[Qian Zhang]

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

(Updated Dec. 20, 2019, 8:44 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

WIP: Set resource limits when launching executor container.


Diffs (updated)
-

  src/slave/slave.hpp 77b5bc0082c6bb73fbd48a2ebe812629921645cb 
  src/slave/slave.cpp 3839a120446339fea8aa857f431a2dba28ed4002 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 71855: Added resource limits into `TaskInfo`.

[Qian Zhang]

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

(Updated Dec. 20, 2019, 8:38 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Addressed review comments.


Summary (updated)
-

Added resource limits into `TaskInfo`.


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


Repository: mesos


Description (updated)
---

Added resource limits into `TaskInfo`.


Diffs (updated)
-

  include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
  include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 


Diff: https://reviews.apache.org/r/71855/diff/3/

Changes: https://reviews.apache.org/r/71855/diff/2-3/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 71931: Added the `share_cgroups` field into `LinuxInfo`.

[Qian Zhang]

> On Dec. 20, 2019, 5:02 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 3362 (patched)
> > 
> >
> > Do you think we should do `share_cgroups` instead of `shared_cgroups`, 
> > to be consistent with the name of the `share_pid_namespace` field?

Agree.


- Qian


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


On Dec. 20, 2019, 8:38 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71931/
> ---
> 
> (Updated Dec. 20, 2019, 8:38 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10075
> https://issues.apache.org/jira/browse/MESOS-10075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the `share_cgroups` field into `LinuxInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71931/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71855: Added resource limits into `TaskInfo`.

[Qian Zhang]

> On Dec. 20, 2019, 5 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 800-801 (patched)
> > 
> >
> > What do you think about adding another argument to `launchExecutor()` 
> > for the executor container limits, rather than adding this field? Since 
> > this message is widely-used in public-facing APIs, I would prefer to avoid 
> > adding a strictly internal parameter if possible.

Agree.


- Qian


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


On Dec. 20, 2019, 8:38 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71855/
> ---
> 
> (Updated Dec. 20, 2019, 8:38 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10043
> https://issues.apache.org/jira/browse/MESOS-10043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource limits into `TaskInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71855/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71931: Added the `share_cgroups` field into `LinuxInfo`.

[Qian Zhang]

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

(Updated Dec. 20, 2019, 8:38 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Addressed review comments.


Summary (updated)
-

Added the `share_cgroups` field into `LinuxInfo`.


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


Repository: mesos


Description (updated)
---

Added the `share_cgroups` field into `LinuxInfo`.


Diffs (updated)
-

  include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
  include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 71856: Added resource limits into `ContainerConfig`.

[Greg Mann]

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


Fix it, then Ship it!





include/mesos/slave/containerizer.proto
Lines 144 (patched)


s/Resources/Resource/


- Greg Mann


On Dec. 20, 2019, 2:11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71856/
> ---
> 
> (Updated Dec. 20, 2019, 2:11 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10046
> https://issues.apache.org/jira/browse/MESOS-10046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource limits into `ContainerConfig`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 8b1a268b5ddf1b791c47219355143da8dbd03789 
> 
> 
> Diff: https://reviews.apache.org/r/71856/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71931: Added the `shared_cgroups` field into `LinuxInfo`.

[Greg Mann]

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




include/mesos/mesos.proto
Lines 3362 (patched)


Do you think we should do `share_cgroups` instead of `shared_cgroups`, to 
be consistent with the name of the `share_pid_namespace` field?


- Greg Mann


On Dec. 20, 2019, 2:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71931/
> ---
> 
> (Updated Dec. 20, 2019, 2:10 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10075
> https://issues.apache.org/jira/browse/MESOS-10075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the `shared_cgroups` field into `LinuxInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71931/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71855: Added resource limits into `TaskInfo` and `ExecutorInfo`.

[Greg Mann]

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




include/mesos/mesos.proto
Lines 800-801 (patched)


What do you think about adding another argument to `launchExecutor()` for 
the executor container limits, rather than adding this field? Since this 
message is widely-used in public-facing APIs, I would prefer to avoid adding a 
strictly internal parameter if possible.


- Greg Mann


On Dec. 20, 2019, 2:11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71855/
> ---
> 
> (Updated Dec. 20, 2019, 2:11 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10043
> https://issues.apache.org/jira/browse/MESOS-10043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource limits into `TaskInfo` and `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d77ddd8021eb7cc3919f3151594dd38606c15c5 
>   include/mesos/v1/mesos.proto 7abb6ba60a1b1861338c47717a49c72a5dffa68c 
> 
> 
> Diff: https://reviews.apache.org/r/71855/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>