Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-20 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review220034 --- Ship it! Ship It! - Qian Zhang On March 20, 2020, 2:42

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-20 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 20, 2020, 6:42 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 20, 2020, 3:48 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 2:40 p.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 2073-2085 (patched) > > > > > > What if the framework launches multiple executors with the same > > executor ID on multiple

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 19, 2020, 5:53 p.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 2:40 p.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 2073-2085 (patched) > > > > > > What if the framework launches multiple executors with the same > > executor ID on multiple

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 10:47 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 2072 (patched) > > > > > > Is it possible that `shareCgroups.isSome()` is false at this point? I > > think as long as the

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review220008 --- src/master/validation.cpp Lines 2015-2017 (patched)

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 10:47 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 2073 (patched) > > > > > > I think we should not check all tasks of the framework, instead we > > should only check the

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 10:47 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 2072 (patched) > > > > > > Is it possible that `shareCgroups.isSome()` is false at this point? I > > think as long as the

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 19, 2020, 1:14 p.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Greg Mann
> On March 19, 2020, 10:47 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 1839-1845 (patched) > > > > > > So we have this validation only for the tasks within a task group but > > not for the

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review21 --- src/master/validation.cpp Lines 1839-1841 (patched)

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 19, 2020, 2:10 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-18 Thread Greg Mann
> On March 15, 2020, 9:04 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 1574-1577 (patched) > > > > > > Why do we want to return error if `taskCpus` is none? The purpose of > > this

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-16 Thread Qian Zhang
> On March 15, 2020, 5:04 p.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 1554-1561 (patched) > > > > > > What about the task which does not set `share_cgroups` (or even not set > > `ContainerInfo`

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-15 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review219933 --- src/master/validation.cpp Lines 1554-1561 (patched)

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-11 Thread Greg Mann
> On March 10, 2020, 1:06 p.m., Andrei Budnik wrote: > > src/master/validation.cpp > > Lines 1567 (patched) > > > > > > `if (limits.size() > cpuCount + memCount)` might be slightly easier to > > understand. Good

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-11 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 11, 2020, 7:10 p.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review219868 --- Fix it, then Ship it! Ship It! src/master/validation.cpp

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
> On March 9, 2020, 9:35 p.m., Andrei Budnik wrote: > > src/master/validation.cpp > > Lines 1991 (patched) > > > > > > `shareCgroups` can be initialized with the value taken from the first > > task within a task

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
> On March 9, 2020, 9:35 p.m., Andrei Budnik wrote: > > src/master/validation.cpp > > Lines 1567 (patched) > > > > > > Can a client set a limit for one parameter only (e.g., "cpus")? Yes they can; AFAICT this

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 10, 2020, 9:42 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 10, 2020, 9:31 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 10, 2020, 8:56 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/ --- (Updated March 10, 2020, 8:56 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 72216: Added master validation for task resource limits and shared cgroups.

2020-03-09 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review219855 --- src/master/validation.cpp Lines 1567 (patched)