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

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


Can a client set a limit for one parameter only (e.g., "cpus")?



src/master/validation.cpp
Lines 1583 (patched)


Consider using `Bytes::MEGABYTES` instead of `(1024 * 1024)`



src/master/validation.cpp
Lines 1991 (patched)


`shareCgroups` can be initialized with the value taken from the first task 
within a task group. This may simplify the `foreach` loop. Also, this would 
allow us to move `if (currentShareCgroups && !task.limits().empty` check 
outside the loop.


- Andrei Budnik


On Март 9, 2020, 3:25 п.п., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72216/
> ---
> 
> (Updated Март 9, 2020, 3:25 п.п.)
> 
> 
> Review request for mesos, Andrei Budnik and Qian Zhang.
> 
> 
> Bugs: MESOS-10045
> https://issues.apache.org/jira/browse/MESOS-10045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added master validation for task resource limits and shared cgroups.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 084f281eadd65cb8ae0a19b7b7797dc71ccebdd2 
> 
> 
> Diff: https://reviews.apache.org/r/72216/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 72190: Added a test `LaunchNestedShareCgroups`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [72121, 71965, 71966, 72122, 72189, 72190]

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

- Mesos Reviewbot


On March 4, 2020, 10:56 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72190/
> ---
> 
> (Updated March 4, 2020, 10:56 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `LaunchNestedShareCgroups`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c6f96e6b1bc0d9989ca87d23e112604820ac1d51 
> 
> 
> Diff: https://reviews.apache.org/r/72190/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Andrei Budnik]

> On Март 8, 2020, 2:29 п.п., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> > Lines 743-748 (original), 758-762 (patched)
> > 
> >
> > I think this is for the case: Agent is started with a cgroup subsystem 
> > specified (like `--isolation=cgroups/cpu`) and a default executor is 
> > launched to run a task group with `share_cgroups==true`, now agent is 
> > restarted with two cgroup subsystems (like 
> > `--isolation=cgroups/cpu,cgroups/mem`) and another task group with 
> > `share_cgroups==true` is launched by the same default executor. For the 
> > nested containers corresponding to the second task group, we should NOT 
> > assign their pids for the subsystem `cgroups/mem` since the default 
> > executor does not have cgroup created under it.
> > 
> > So I think we should not change the log message here.

After introducing the support for nested cgroups, a `cgroup` variable might 
refer to a nested container's cgroup rather than a root container's cgroup. If 
a nested cgroup is lost for some reason, we shouldn't claim that a parent 
cgroup doesn't have the cgroup hierarchy because it can actually exist. In 
fact, a nested cgroup might be missing by 2 reasons: a) parent cgroups is 
absent (due to the reason you've described) b) the parent cgroup is here, but a 
nested cgroup is absent. I decided to update the log message in order to make 
it more generic.


- Andrei


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


On Фев. 13, 2020, 5:19 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71965/
> ---
> 
> (Updated Фев. 13, 2020, 5:19 п.п.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Bugs: MESOS-10076
> https://issues.apache.org/jira/browse/MESOS-10076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for nested cgroups for nested containers.
> Nested cgroups are created only for a nested container with explicitly
> disabled `share_cgroups` flag. The cgroups isolator stores info about
> nested cgroups in the `infos` class variable, which is used to
> determine whether a nested container has its nested cgroup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
> 
> 
> Diff: https://reviews.apache.org/r/71965/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Andrei Budnik]

> On Янв. 13, 2020, 3:21 д.п., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> > Lines 779-785 (original), 797-803 (patched)
> > 
> >
> > This logic is correct for root container since we will return an 
> > `Unknown container` failure for it if `infos` does not contain it, but it 
> > seems not correct for the nested container whose `share_cgroup` is false, 
> > for such container, if `infos` does not contain it, we will always return a 
> > pending future but never return the `unknown container` failure, that might 
> > hide some errors in our code. I think we should also return an `Unknown 
> > container` failure for the nested container whose `share_cgroup` is false 
> > and somehow `infos` does not contain it. HDYT?
> 
> Andrei Budnik wrote:
> That's a good point!
> Unfortunately, `share_cgroup` flag is unknown for terminated containers. 
> A terminated nested container with `share_cgroup=false` is indistinguishable 
> from a running container with `share_cgroup=true`, because they both are not 
> presented in the `infos`.
> We can keep `Info` for all existing containers and add `share_cgroup` 
> field to it.
> 
> Andrei Budnik wrote:
> On second thought, we won't be able to create `Info` for nested 
> containers with shared cgroups during the recovery. They're invisible to the 
> cgroups isolator.
> 
> Andrei Budnik wrote:
> ```
> This logic is correct for root container since we will return an Unknown 
> container failure for it if infos does not contain it, but it seems not 
> correct for the nested container whose share_cgroup is false, for such 
> container, if infos does not contain it, we will always return a pending 
> future but never return the unknown container failure, that might hide some 
> errors in our code.
> ```
> 
> The previous version was prone to the problem you have described above: 
> we could return a pending future for a non-existent nested container. This 
> code change neither introduces new problems nor fixes the existing problem.
> 
> Qian Zhang wrote:
> Could you please add a `TODO` in this code to mention that ideally we 
> should return an unknown container failure for the nested container whose 
> `share_cgroups` is false but `infos` does not contain it?
> 
> Or how about we add a new parameter `const ContainerConfig& 
> containerConfig` to the `watch()` method of the isolator interface, and with 
> such parameter in `watch()` method we will clearly know if the container's 
> `share_cgroups` is true or false, if it is false and the container does not 
> exist in `infos`, we can just return an unknown container failure.

If a container shares cgroups with its parent, then the cgroups isolator should 
not be in charge of this container. So, returning "Unknown" seems totally fine 
for me, and I don't think we need a comment either.


> On Янв. 13, 2020, 3:21 д.п., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> > Lines 840-841 (patched)
> > 
> >
> > How do we know this is a nested container **with shared cgroups**?
> > 
> > And I have the same comment for the methods `usage`, `status` and 
> > `cleanup`.
> 
> Andrei Budnik wrote:
> > How do we know this is a nested container with shared cgroups?
> 
> `update()` and `usage()` case.
> Since there is no way to distinguish between a nested container with 
> shared cgroups and a non-existent container, we should return the error 
> message describing the situation: `Unknown container`. This message tells us 
> that the container is unknown to the cgroups isolator. It doesn't state that 
> this container does not exist, nor it states that this is a nested container 
> with shared cgroups.
> 
> `cleanup()` case.
> I removed the misleading comment. Also, I made it print VLOG message in 
> any case as it's more accurate.
> 
> `status()` case.
> If we are a nested container unknown to the isolator, we try to find the 
> status of its ancestor. This preserves the old behavior.
> 
> Qian Zhang wrote:
> > status() case.
> > If we are a nested container unknown to the isolator, we try to find 
> the status of its ancestor. This preserves the old behavior.
> 
> I think here we have the similar issue with `watch()`: For the nested 
> container whose `share_cgroups` is true but somehow it does not exist in 
> `infos`, ideally we should return an unknown container failure, but not just 
> return its parent container's status which means we may return wrong status 
> for a nested container.

Previously, we could return the status of a parent container for a nested 
container that had never been launched. Now, we're dealing with the following 
alternatives for a nested container: 1) it's unknown 2) 

Re: Review Request 72217: Added tests for resource limit and shared cgroup validation.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [72161, 72162, 71858, 71884, 71885, 71886, 71943, 71944, 
71950, 71951, 71952, 71953, 71955, 72215, 72216, 72217]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:16.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/jenkins/buildbot.sh 2>&1 | 
tee build_72217"]

Error:
..
KAGE_URL=\"\" -DPACKAGE=\"mesos\" -DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 
-DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 
-DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 
-DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 
-DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 
-DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 -DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 
-DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 -DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 
-DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 
-DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. 
-I../../../../3rdparty/stout  -I../../../../3rdparty/stout/include 
-I../boost-1.65.0 -I../elfio-3.2 -I../glog-0.4.0/src 
-I../googletest-release-1.8.0/googlemock/include 
-I../googletest-release-1.8.0/googletest/include 
-I../libarchive-3.3.2/libarchive -D__STDC_FORMAT_MACROS -I../picojson-1.3.0 
-I../protobuf-3.5.0/src -I../rapidjson-1.1.0/
 include  -I/usr/include/subversion-1 -I/usr/include/apr-1 
-I/usr/include/apr-1.0  -Wall -Wsign-compare -Wformat-security 
-fstack-protector-strong -fPIC -fPIE -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c -o stout_tests-ip_tests.o `test -f 'tests/ip_tests.cpp' || echo 
'../../../../3rdparty/stout/'`tests/ip_tests.cpp
g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.10.0\" -DPACKAGE_STRING=\"mesos\ 1.10.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../../3rdparty/stout  
-I../../../../3rdparty/stout/include -I../boost-1.65.0 -I../elfio-3.2 
-I../glog-0.4.0/src -I../googletest-release-1.8.0/googlemock/include 
-I../googletest-rel
 ease-1.8.0/googletest/include -I../libarchive-3.3.2/libarchive 
-D__STDC_FORMAT_MACROS -I../picojson-1.3.0 -I../protobuf-3.5.0/src 
-I../rapidjson-1.1.0/include  -I/usr/include/subversion-1 -I/usr/include/apr-1 
-I/usr/include/apr-1.0   -Wall -Wsign-compare -Wformat-security 
-fstack-protector-strong -fPIC -fPIE -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c -o stout_tests-json_tests.o `test -f 'tests/json_tests.cpp' || 
echo '../../../../3rdparty/stout/'`tests/json_tests.cpp
g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.10.0\" -DPACKAGE_STRING=\"mesos\ 1.10.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../../3rdparty/stout  
-I../../../../3rdparty/stout/include -I../boost-1.65.0 -I../elfio-3.2 
-I../glog-0.4.0/src -I../googletest-release-1.8.0/googlemock/include 
-I../googletest-rel
 ease-1.8.0/googletest/include -I../libarchive-3.3.2/libarchive 
-D__STDC_FORMAT_MACROS -I../picojson-1.3.0 -I../protobuf-3.5.0/src 
-I../rapidjson-1.1.0/include  -I/usr/include/subversion-1 -I/usr/include/apr-1 
-I/usr/include/apr-1.0   -Wall -Wsign-compare -Wformat-security 
-fstack-protector-strong -fPIC -fPIE -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c -o stout_tests-jsonify_tests.o `test -f 

Re: Review Request 72041: Updated default executor to call the `LaunchContainer` agent API.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [72161, 72162, 71858, 71884, 71885, 71886, 71943, 71944, 
71950, 71951, 71952, 71953, 71955, 71956, 72210, 71983, 72022, 72027, 72211, 
72040, 72041]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:16.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/jenkins/buildbot.sh 2>&1 | 
tee build_72041"]

Error:
..
GE_URL=\"\" -DPACKAGE=\"mesos\" -DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 
-DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 
-DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 
-DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 
-DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 
-DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 -DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 
-DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 -DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 
-DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 
-DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. 
-I../../../../3rdparty/stout  -I../../../../3rdparty/stout/include 
-I../boost-1.65.0 -I../elfio-3.2 -I../glog-0.4.0/src 
-I../googletest-release-1.8.0/googlemock/include 
-I../googletest-release-1.8.0/googletest/include 
-I../libarchive-3.3.2/libarchive -D__STDC_FORMAT_MACROS -I../picojson-1.3.0 
-I../protobuf-3.5.0/src -I../rapidjson-1.1.0/in
 clude  -I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0 
   -Wall -Wsign-compare -Wformat-security -fstack-protector-strong -fPIC 
-fPIE -g1 -O0 -Wno-unused-local-typedefs -std=c++11 -c -o 
stout_tests-ip_tests.o `test -f 'tests/ip_tests.cpp' || echo 
'../../../../3rdparty/stout/'`tests/ip_tests.cpp
g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.10.0\" -DPACKAGE_STRING=\"mesos\ 1.10.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../../3rdparty/stout  
-I../../../../3rdparty/stout/include -I../boost-1.65.0 -I../elfio-3.2 
-I../glog-0.4.0/src -I../googletest-release-1.8.0/googlemock/include 
-I../googletest-rel
 ease-1.8.0/googletest/include -I../libarchive-3.3.2/libarchive 
-D__STDC_FORMAT_MACROS -I../picojson-1.3.0 -I../protobuf-3.5.0/src 
-I../rapidjson-1.1.0/include  -I/usr/include/subversion-1 -I/usr/include/apr-1 
-I/usr/include/apr-1.0   -Wall -Wsign-compare -Wformat-security 
-fstack-protector-strong -fPIC -fPIE -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c -o stout_tests-json_tests.o `test -f 'tests/json_tests.cpp' || 
echo '../../../../3rdparty/stout/'`tests/json_tests.cpp
g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.10.0\" -DPACKAGE_STRING=\"mesos\ 1.10.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.10.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DENABLE_NVML=1 
-DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 
-DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 
-DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. -I../../../../3rdparty/stout  
-I../../../../3rdparty/stout/include -I../boost-1.65.0 -I../elfio-3.2 
-I../glog-0.4.0/src -I../googletest-release-1.8.0/googlemock/include 
-I../googletest-rel
 ease-1.8.0/googletest/include -I../libarchive-3.3.2/libarchive 
-D__STDC_FORMAT_MACROS -I../picojson-1.3.0 -I../protobuf-3.5.0/src 
-I../rapidjson-1.1.0/include  -I/usr/include/subversion-1 -I/usr/include/apr-1 
-I/usr/include/apr-1.0   -Wall -Wsign-compare -Wformat-security 
-fstack-protector-strong -fPIC -fPIE -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c -o stout_tests-jsonify_tests.o 

Review Request 72217: Added tests for resource limit and shared cgroup validation.

[Greg Mann]

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

Review request for mesos, Andrei Budnik and Qian Zhang.


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


Repository: mesos


Description
---

Added tests for resource limit and shared cgroup validation.


Diffs
-

  src/tests/master_validation_tests.cpp 
8d5e74e82a2f9093b69120c1dbba33c56e25966b 


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


Testing
---

`make check`


Thanks,

Greg Mann

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

[Greg Mann]

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

Review request for mesos, Andrei Budnik and Qian Zhang.


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


Repository: mesos


Description
---

Added master validation for task resource limits and shared cgroups.


Diffs
-

  src/master/validation.cpp 084f281eadd65cb8ae0a19b7b7797dc71ccebdd2 


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


Testing
---


Thanks,

Greg Mann

Review Request 72215: Added resource limits to one overload of 'createTask()' helper.

[Greg Mann]

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

Review request for mesos, Andrei Budnik and Qian Zhang.


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


Repository: mesos


Description
---

Added resource limits to one overload of 'createTask()' helper.


Diffs
-

  src/tests/mesos.hpp 17fcaa3eff28e8139e6770629a5dfbb7c29289ec 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Qian Zhang]

> On March 8, 2020, 10:29 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> > Line 411 (original), 412 (patched)
> > 
> >
> > s/with/from/
> 
> Greg Mann wrote:
> I think "with" is actually the preferred language here.

I think it should be parent container shares its cgroups with nested container, 
and nested container shares cgroups from parent container, right?


- Qian


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


On Feb. 14, 2020, 1:19 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71965/
> ---
> 
> (Updated Feb. 14, 2020, 1:19 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Bugs: MESOS-10076
> https://issues.apache.org/jira/browse/MESOS-10076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for nested cgroups for nested containers.
> Nested cgroups are created only for a nested container with explicitly
> enabled `share_cgroups` flag. The cgroups isolator stores info about
> nested cgroups in the `infos` class variable, which is used to
> determine whether a nested container has its nested cgroup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
> 
> 
> Diff: https://reviews.apache.org/r/71965/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71952: Set resource limits when updating executor container.

[Qian Zhang]

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

(Updated March 9, 2020, 10:08 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Replaced `Executor::resourceLimits` with `Slave::computeExecutorLimits()`.


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


Repository: mesos


Description
---

Set resource limits when updating executor container.


Diffs (updated)
-

  src/slave/slave.hpp 03279db082bdba23dbfeb2d93081a908e609aec2 
  src/slave/slave.cpp a914de4bae5630126ae26b9e7dee8c8784ce1ee0 


Diff: https://reviews.apache.org/r/71952/diff/7/

Changes: https://reviews.apache.org/r/71952/diff/6-7/


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 71951: Added resource limits into the `Task` protobuf message.

[Qian Zhang]

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

(Updated March 9, 2020, 10:05 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Updated `protobuf::createTask()`.


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


Repository: mesos


Description
---

Added resource limits into the `Task` protobuf message.


Diffs (updated)
-

  include/mesos/mesos.proto 40c45de371b12d84aeebcc91847336c4968592f2 
  include/mesos/v1/mesos.proto 638763612ccb302f052eb30caf661984087314e1 
  src/common/protobuf_utils.cpp b3057be7460100201066a354a1f621d100546233 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 71858: 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 March 9, 2020, 10:04 p.m.)


Review request for mesos, Andrei Budnik and Greg Mann.


Changes
---

Refactored the code to compute executor resource limits into 
`Slave::computeExecutorLimits()`.


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


Repository: mesos


Description
---

Set resource limits when launching executor container.


Diffs (updated)
-

  src/slave/slave.hpp 03279db082bdba23dbfeb2d93081a908e609aec2 
  src/slave/slave.cpp a914de4bae5630126ae26b9e7dee8c8784ce1ee0 


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

Changes: https://reviews.apache.org/r/71858/diff/10-11/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Greg Mann]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp
Line 149 (original), 149 (patched)


I just tried to apply this patch on master and got a conflict, looks like 
it needs a rebase.


- Greg Mann


On Feb. 13, 2020, 5:19 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71965/
> ---
> 
> (Updated Feb. 13, 2020, 5:19 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Bugs: MESOS-10076
> https://issues.apache.org/jira/browse/MESOS-10076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for nested cgroups for nested containers.
> Nested cgroups are created only for a nested container with explicitly
> enabled `share_cgroups` flag. The cgroups isolator stores info about
> nested cgroups in the `infos` class variable, which is used to
> determine whether a nested container has its nested cgroup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
> 
> 
> Diff: https://reviews.apache.org/r/71965/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Greg Mann]

> On March 8, 2020, 2:29 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> > Line 411 (original), 412 (patched)
> > 
> >
> > s/with/from/

I think "with" is actually the preferred language here.


- Greg


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


On Feb. 13, 2020, 5:19 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71965/
> ---
> 
> (Updated Feb. 13, 2020, 5:19 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Bugs: MESOS-10076
> https://issues.apache.org/jira/browse/MESOS-10076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for nested cgroups for nested containers.
> Nested cgroups are created only for a nested container with explicitly
> enabled `share_cgroups` flag. The cgroups isolator stores info about
> nested cgroups in the `infos` class variable, which is used to
> determine whether a nested container has its nested cgroup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
> 
> 
> Diff: https://reviews.apache.org/r/71965/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71965: Cgroups isolator: added support for nested cgroups during launch.

[Greg Mann]

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



In the description: should be "explicitly disabled `share_cgroups` flag"


src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Line 411 (original), 412 (patched)


I think "with" is actually the preferred language here.


- Greg Mann


On Feb. 13, 2020, 5:19 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71965/
> ---
> 
> (Updated Feb. 13, 2020, 5:19 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Qian Zhang.
> 
> 
> Bugs: MESOS-10076
> https://issues.apache.org/jira/browse/MESOS-10076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for nested cgroups for nested containers.
> Nested cgroups are created only for a nested container with explicitly
> enabled `share_cgroups` flag. The cgroups isolator stores info about
> nested cgroups in the `infos` class variable, which is used to
> determine whether a nested container has its nested cgroup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
> 
> 
> Diff: https://reviews.apache.org/r/71965/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>